Skip to content

v0.47: vendor: bump runc to 1.1.9#3383

Merged
bobbypage merged 1 commit intogoogle:release-v0.47from
haircommander:runc-1.1.9-0.47
Sep 14, 2023
Merged

v0.47: vendor: bump runc to 1.1.9#3383
bobbypage merged 1 commit intogoogle:release-v0.47from
haircommander:runc-1.1.9-0.47

Conversation

@haircommander
Copy link
Copy Markdown
Contributor

to fix an issue with cgroupv2 memory accounting not being accurate

to fix an issue with cgroupv2 memory accounting not being accurate

Signed-off-by: Peter Hunt <pehunt@redhat.com>
@k8s-ci-robot
Copy link
Copy Markdown
Collaborator

Hi @haircommander. Thanks for your PR.

I'm waiting for a google member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@haircommander haircommander changed the title vendor: bump runc to 1.1.9 v0.47: vendor: bump runc to 1.1.9 Aug 17, 2023
@iwankgb
Copy link
Copy Markdown
Collaborator

iwankgb commented Aug 20, 2023

I'm afraid we have to stick to 1.7 until Kubernetes updates its dependency version.

Copy link
Copy Markdown
Collaborator

@iwankgb iwankgb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pending update in Kubernetes.

@haircommander
Copy link
Copy Markdown
Contributor Author

I don't think I understand why. does cadvisor depend on kubernetes go packages at all? I would think we'd update in cadvisor, then pull cadvisor and run changes into kube together. Otherwise, we'd do a runc bump in kube, one here, and then one for cadvisor, increasing the total amount of PRs needed

@haircommander
Copy link
Copy Markdown
Contributor Author

WDYT @iwankgb ?

@iwankgb
Copy link
Copy Markdown
Collaborator

iwankgb commented Aug 23, 2023

Kubernetes depends on cAdvisor and I"m hesitant to introduce changes that would affect their go.mod.

@iwankgb
Copy link
Copy Markdown
Collaborator

iwankgb commented Aug 23, 2023

This is why i decided to block the PR: i should have been clearer explaining my reasoning.

@haircommander
Copy link
Copy Markdown
Contributor Author

ah I see, but I am trying to bump it there too kubernetes/kubernetes#119909 I figured this would be a good first step

@haircommander
Copy link
Copy Markdown
Contributor Author

@iwankgb @bobbypage WDYT?

@bobbypage
Copy link
Copy Markdown
Contributor

bobbypage commented Aug 28, 2023

Thanks @haircommander for the PR! Why do we need backport this PR to v0.47? Can we just merge this in master and use the next cut version of cAdvisor in the next k8s?

@haircommander
Copy link
Copy Markdown
Contributor Author

haircommander commented Aug 28, 2023

I believe it's worth bumping the older versions to get kubernetes/kubernetes#118916 fixes in the k8s release branches to more accurately describe the memory usage on v2. 1.1.9 has the fix opencontainers/runc#3933 which should fix this without code changes in k8s or cadvisor

@haircommander
Copy link
Copy Markdown
Contributor Author

WDYT @bobbypage

@haircommander
Copy link
Copy Markdown
Contributor Author

@iwankgb @bobbypage PTAL

@bobbypage
Copy link
Copy Markdown
Contributor

Thanks @haircommander. Where we plan to merge the fix for kubernetes/kubernetes#118916? If it's only master branch, then we don't need to backport. Or do we plan to merge it to both 1.27 + 1.28 and master? cAdvisor v0.47 is used in k/k in v1.27 and v1.28, so if we backport it here, we should ensure make the corresponding changes in k/k in v1.27 and v1.28.

@haircommander
Copy link
Copy Markdown
Contributor Author

yeah I'd like to pull this back to 1.27 and 1.28

@bobbypage
Copy link
Copy Markdown
Contributor

sg, let's merge this than, and I will followup to create a release.

@bobbypage bobbypage merged commit 1a19abb into google:release-v0.47 Sep 14, 2023
@ritazh
Copy link
Copy Markdown

ritazh commented Sep 15, 2023

sg, let's merge this than, and I will followup to create a release.

Can we also get this into k8s 1.25 and 1.26? Thank you!

@RDPzero
Copy link
Copy Markdown

RDPzero commented Sep 26, 2023

I would appreciate to see this on k8s, since it solves a problem I'm facing for a long time.

Are you going to cut a patch release from 0.47 @bobbypage ?

@aritraghosh
Copy link
Copy Markdown

@bobbypage When is the next release scheduled for, We would love to have it to merge kubernetes/kubernetes#119909

@haircommander

@aritraghosh
Copy link
Copy Markdown

@haircommander Can this change also be cherry picked to 1.26, 1.27 and 1.28? This change significantly reduces the memory reported on cgroup v2 to match v1

cgroupv2

@ritazh
Copy link
Copy Markdown

ritazh commented Nov 23, 2023

sg, let's merge this than, and I will followup to create a release.

@bobbypage thanks for merging the PR! Can you please share when the next release is scheduled for so we can back port this change to kubernetes? Thanks in advance!

xref: kubernetes/kubernetes#121637 (comment)

@bobbypage
Copy link
Copy Markdown
Contributor

bobbypage commented Nov 28, 2023

Thanks @ritazh for the ping.

Before we backport this change, I would like to ensure we have the full clarity on the fix. Specifically there are still some open questions in opencontainers/runc#3933 (comment) if this is the right fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants