[v0.3] Add ContainerFilesystems to ImageFsInfoResponse #407
Conversation
|
@xinfengliu Could you take a look at this? |
core/imagefs_linux.go
Outdated
| @@ -63,6 +63,15 @@ func (ds *dockerService) imageFsInfo() (*runtimeapi.ImageFsInfoResponse, error) | |||
| } | |||
| logrus.Debugf("Total used bytes by docker images: %v", totalImageSize) | |||
|
|
|||
| var totalRWLayerSize uint64 | |||
| for cid, _ := range ds.containerStatsCache.stats { | |||
There was a problem hiding this comment.
There's a RW lock for accessing containerStatsCache, maybe a better way is to add a new method for containerStatsCache in core/stats.go?
|
@kinarashah Thank you for the pull request. Could you confirm if the PR will break older versions of Also, We generally create PR against the master branch first, then back-port it to other release branches. |
|
@xinfengliu Thank you for the quick review and context, I'll test it out with older versions and open a PR against master branch today. |
|
@xinfengliu Updated the PR. Here's the PR for the Also, confirmed that this doesn't break earlier kubernetes versions because the feature flag is disabled by default. Feature flag is enabled by default for the first time with v1.31 and is still in beta. |
|
Thank you @kinarashah . I have approved both PRs. |
|
@nwneisen Unit test fixed and |
|
@nwneisen Thanks for restarting the workflow. I noticed two actions are failing, but they don’t appear to be related to this PR. Could you check if any additional changes are needed from my side? |
|
@kinarashah They do not look like they are related failures. I will merge with them failing |
|
@kinarashah and @nwneisen |
|
@xinfengliu sorry for the delay, I'll update the PR for master branch today. |
Problem
Kubelet complaining about missing image stats in logs:
Issue
#408
Solution
In version v1.31, the feature gate KubeletSeparateDiskGC is enabled, and the ImageFsInfoResponse now includes container filesystem information. kubernetes/kubernetes#120914
Leveraging the current rwLayerSize for each container to determine the total writable layer size.
Note: I attempted to bump Kubernetes up to v1.31, but that requires significant changes for
k8s.io/cri-api/v1alpha2. So I decided to stick with v1.29 instead which is the minimum version ofk8s.io/cri-apithat introducesContainerFilesystems. (Looks like this part is already being covered here: #333)