Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support image pull per runtime class #4216

Open
2 of 4 tasks
kiashok opened this issue Sep 15, 2023 · 31 comments
Open
2 of 4 tasks

Support image pull per runtime class #4216

kiashok opened this issue Sep 15, 2023 · 31 comments
Assignees
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/windows Categorizes an issue or PR as relevant to SIG Windows.

Comments

@kiashok
Copy link
Contributor

kiashok commented Sep 15, 2023

Enhancement Description

Please keep this description up to date. This will help the Enhancement Team to track the evolution of the enhancement efficiently.

@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Sep 15, 2023
@kiashok
Copy link
Contributor Author

kiashok commented Sep 15, 2023

cc @jsturtevant @mikebrow @marosset @haircommander @SergeyKanzhelev @aravindhp @dmcgowan @kevpar @dcantah

@aravindhp
Copy link
Contributor

/sig windows
/sig node

@k8s-ci-robot k8s-ci-robot added sig/windows Categorizes an issue or PR as relevant to SIG Windows. sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Sep 15, 2023
@jsturtevant
Copy link
Contributor

/milestone v1.29

@k8s-ci-robot k8s-ci-robot added this to the v1.29 milestone Sep 18, 2023
@npolshakova
Copy link

npolshakova commented Sep 22, 2023

Hello @kiashok, 1.29 Enhancements team here! Is this enhancement targeting 1.29? If it is, can you follow the instructions here to opt in the enhancements and make sure the lead-opted-in label is set so it can get added to the tracking board? Thanks!

@aravindhp
Copy link
Contributor

/label lead-opted-in

@k8s-ci-robot k8s-ci-robot added the lead-opted-in Denotes that an issue has been opted in to a release label Sep 22, 2023
@sanchita-07
Copy link
Member

sanchita-07 commented Sep 27, 2023

Hello @kiashok 👋, Enhancements team here.

Just checking in as we approach enhancements freeze on Friday, 6th October 2023.

This enhancement is targeting for stage alpha for 1.29 (correct me, if otherwise)

Here's where this enhancement currently stands:

  • KEP readme using the latest template has been merged into the k/enhancements repo.
  • KEP status is marked as implementable for latest-milestone: 1.29.
  • KEP readme has up-to-date graduation criteria
  • KEP has a production readiness review that has been completed and merged into k/enhancements. (For more information on the PRR process, check here).

For this KEP, we would just need to add/update the following:

  • The latest readme template has more items in production readiness review questionnaire that need to be addressed.
  • The status should be marked as implementable in the kep.yaml file and the latest-milestone should be set to v1.29.
  • Ensure that the PR including the production readiness review has been reviewed and merged into k/enhancements.

The status of this enhancement is marked as at risk for enhancement freeze. Please keep the issue description up-to-date with appropriate stages as well. Thank you!

@kiashok
Copy link
Contributor Author

kiashok commented Sep 27, 2023

Hello @kiashok 👋, Enhancements team here.

Just checking in as we approach enhancements freeze on Friday, 6th October 2023.

This enhancement is targeting for stage alpha for 1.29 (correct me, if otherwise)

Here's where this enhancement currently stands:

  • KEP readme using the latest template has been merged into the k/enhancements repo.
  • KEP status is marked as implemented for latest-milestone: 1.29.
  • KEP readme has up-to-date graduation criteria
  • KEP has a production readiness review that has been completed and merged into k/enhancements. (For more information on the PRR process, check here).

For this KEP, we would just need to add/update the following:

  • The latest readme template has more items in production readiness review questionnaire that need to be addressed.
  • The status should be marked as implementable in the kep.yaml file and the latest-milestone should be set to v1.29.
  • Ensure that the PR including the production readiness review has been reviewed and merged into k/enhancements.

The status of this enhancement is marked as at risk for enhancement freeze. Please keep the issue description up-to-date with appropriate stages as well. Thank you!

Hi @sanchita-07 , thanks a lot for the note. I have updated the KEP PR with your suggestions and I also requested for PRR review from the slack channel. Please let me know if there is anything more I need to do. Thanks!

@kiashok
Copy link
Contributor Author

kiashok commented Sep 27, 2023

lead-opted-in

Thanks @npolshakova . @marosset reviewed the KEP today and set this label.

@sanchita-07
Copy link
Member

Hi @sanchita-07 , thanks a lot for the note. I have updated the KEP PR with your suggestions and I also requested for PRR review from the slack channel. Please let me know if there is anything more I need to do. Thanks!

We just need the PR #4217 to be merged, then we are good to go to mark the status as tracked for enhancement freeze. 😃

@kiashok
Copy link
Contributor Author

kiashok commented Oct 3, 2023

Hi @sanchita-07 , thanks a lot for the note. I have updated the KEP PR with your suggestions and I also requested for PRR review from the slack channel. Please let me know if there is anything more I need to do. Thanks!

We just need the PR #4217 to be merged, then we are good to go to mark the status as tracked for enhancement freeze. 😃

@sanchita-07 could you please help assign a PRR reviewer? Thanks.

@sanchita-07
Copy link
Member

It looks like @johnbelamaric has been assigned and looking over it.

@npolshakova
Copy link

Hello @kiashok 👋, Enhancement Lead here. This enhancement is tracked for enhancement freeze, but there is some additional feedback on the KEP PR that was already merged: #4217 (review). Please address the feedback and open a follow-up PR as soon as you can. Thanks!

@kiashok
Copy link
Contributor Author

kiashok commented Oct 6, 2023

Hello @kiashok 👋, Enhancement Lead here. This enhancement is tracked for enhancement freeze, but there is some additional feedback on the KEP PR that was already merged: #4217 (review). Please address the feedback and open a follow-up PR as soon as you can. Thanks!

Hi @npolshakova , I am addressing the comments now. The comments are only around restructuring some sentences and have no impact on the design that the KEP is talking about. It should not affect the KEP for 1.29

cc @jsturtevant @aravindhp @mrunalp @marosset @johnbelamaric @mikebrow

@kiashok
Copy link
Contributor Author

kiashok commented Oct 6, 2023

Hello @kiashok 👋, Enhancement Lead here. This enhancement is tracked for enhancement freeze, but there is some additional feedback on the KEP PR that was already merged: #4217 (review). Please address the feedback and open a follow-up PR as soon as you can. Thanks!

Hi @npolshakova , I am addressing the comments now. The comments are only around restructuring some sentences and have no impact on the design that the KEP is talking about. It should not affect the KEP for 1.29

cc @jsturtevant @aravindhp @mrunalp @marosset @johnbelamaric @mikebrow

FYI: opened PR #4283 to address the changes

@katcosgrove
Copy link

Hey there @kiashok! 👋, v1.29 Docs Lead here.
Does this enhancement work planned for v1.29 require any new docs or modification to existing docs?
If so, please follows the steps here to open a PR against dev-1.29 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday, 19 October 2023.
Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release.
Thank you!

@kiashok
Copy link
Contributor Author

kiashok commented Oct 10, 2023

Hey there @kiashok! 👋, v1.29 Docs Lead here. Does this enhancement work planned for v1.29 require any new docs or modification to existing docs? If so, please follows the steps here to open a PR against dev-1.29 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday, 19 October 2023. Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release. Thank you!

I'll take a look at this tomorrow and share an update. Thanks! Also what is the code freeze date for alpha changes of the KEP?

@katcosgrove
Copy link

Thank you! Code freeze is 18:00 PDT on 31 October. The deadline for Docs PRs ready to review is 14 November, and ready to merge 28 November.

@katcosgrove
Copy link

Hi again @kiashok! The deadline to open a placeholder PR against k/website for required documentation is this Thursday, 19 October. Could you please update me on the status of docs for this enhancement? Thank you!

@kiashok
Copy link
Contributor Author

kiashok commented Oct 17, 2023

Hi again @kiashok! The deadline to open a placeholder PR against k/website for required documentation is this Thursday, 19 October. Could you please update me on the status of docs for this enhancement? Thank you!

@katcosgrove Created a placeholder PR here - kubernetes/website#43541

@sanchita-07
Copy link
Member

Hey again @kiashok 👋, 1.29 Enhancements team here.

Just checking in as we approach code freeze at 01:00 UTC Wednesday 1st November 2023:

Here's where this enhancement currently stands:

  • All PRs to the Kubernetes repo that are related to your enhancement are linked in the above issue description (for tracking purposes).
  • All PR/s are ready to be merged (they have approved and lgtm labels applied) by the code freeze deadline. This includes tests.

For this enhancement, it looks like the following PR was merged before code freeze:

Also, please let me know if there are other PRs in k/k we should be tracking for this KEP.

With the merged and linked in the issue description, this KEP is tracked for code freeze for v1.29. 🚀
As always, we are here to help if any questions come up ✌. Thanks :)

@kiashok
Copy link
Contributor Author

kiashok commented Oct 30, 2023

Waiting for this PR kubernetes/kubernetes#121456 to be signed off for 1.29 . It introduces feature gate and kubelet changes needed for this KEP

@a-mccarthy
Copy link

Hi @kiashok, 👋 from the v1.29 Release Team-Communications! We would like to check if you have any plans to publish a blog for this KEP regarding new features, removals, and deprecations for this release.

If so, you need to open a PR placeholder in the website repository.
The deadline will be on Tuesday 14th November 2023 (after the Docs deadline PR ready for review)

Here is the 1.29 calendar

@kiashok
Copy link
Contributor Author

kiashok commented Oct 30, 2023

Hi @kiashok, 👋 from the v1.29 Release Team-Communications! We would like to check if you have any plans to publish a blog for this KEP regarding new features, removals, and deprecations for this release.

If so, you need to open a PR placeholder in the website repository. The deadline will be on Tuesday 14th November 2023 (after the Docs deadline PR ready for review)

Here is the 1.29 calendar

@a-mccarthy I already opened one two weeks ago: #4216 (comment)

@kiashok
Copy link
Contributor Author

kiashok commented Oct 31, 2023

Follow up discussion to be had in sig-node for beta version: kubernetes/kubernetes#121658

@a-mccarthy
Copy link

@a-mccarthy I already opened one two weeks ago: #4216 (comment)

@kiashok, that looks like a PR for docs changes. If you would like to also write a blog for this change, please create a separate placeholder PR by November 14th. Thanks!

@kiashok
Copy link
Contributor Author

kiashok commented Nov 14, 2023

@a-mccarthy I already opened one two weeks ago: #4216 (comment)

@kiashok, that looks like a PR for docs changes. If you would like to also write a blog for this change, please create a separate placeholder PR by November 14th. Thanks!

kubernetes/website#43923 @a-mccarthy . I will have the actual changes go out early tomorrow

@salehsedghpour
Copy link
Contributor

/remove-label lead-opted-in

@k8s-ci-robot k8s-ci-robot removed the lead-opted-in Denotes that an issue has been opted in to a release label Jan 6, 2024
@salehsedghpour
Copy link
Contributor

salehsedghpour commented Jan 16, 2024

Hello 👋 1.30 Enhancements Lead here,

I'm closing milestone 1.29 now,
If you wish to progress this enhancement in v1.30, please follow the instructions here to opt in the enhancement and make sure the lead-opted-in label is set so it can get added to the tracking board and finally add /milestone v1.30. Thanks!

/milestone clear

@k8s-ci-robot k8s-ci-robot removed this from the v1.29 milestone Jan 16, 2024
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 15, 2024
@ruiwen-zhao
Copy link
Contributor

containerd is adding the capability of pulling images with different snapshotters, based on the pod's runtime handler. So if kubelet requests containerd to pull image per runtime class, then containerd might pull them with different snapshotters.

And since each snapshotter keeps its own imageFs usage, this will cause trouble for kubelet's tracking of imageFs usage. More spefically, kubelet currently only reads the imagefs usage of containerd's global snapshotter ([1] [2]), so if we pull a lot of images thru a non-global snapshotter, the disk usage will be ignored.

I cannot think of any easy way to address this, mostly because kubelet's cri_stats_provider doesn't know which pods are using which snapshotter. Should we add this in to the scope of this KEP, to make sure kubelet either track imageFs usage per runtime class, or kubelet combine the imageFs of all snapshotters.

[1] https://github.com/containerd/containerd/blob/bbac058cf31359d7ca7b2a98f48bce086b817dcf/internal/cri/server/images/imagefs_info.go#L54

[2] https://github.com/kubernetes/kubernetes/blob/26923b91e8cbd2d409e5d177ddd509429b76cb35/pkg/kubelet/stats/cri_stats_provider.go#L401

@kiashok
Copy link
Contributor Author

kiashok commented Apr 22, 2024

containerd is adding the capability of pulling images with different snapshotters, based on the pod's runtime handler. So if kubelet requests containerd to pull image per runtime class, then containerd might pull them with different snapshotters.

And since each snapshotter keeps its own imageFs usage, this will cause trouble for kubelet's tracking of imageFs usage. More spefically, kubelet currently only reads the imagefs usage of containerd's global snapshotter ([1] [2]), so if we pull a lot of images thru a non-global snapshotter, the disk usage will be ignored.

I cannot think of any easy way to address this, mostly because kubelet's cri_stats_provider doesn't know which pods are using which snapshotter. Should we add this in to the scope of this KEP, to make sure kubelet either track imageFs usage per runtime class, or kubelet combine the imageFs of all snapshotters.

[1] https://github.com/containerd/containerd/blob/bbac058cf31359d7ca7b2a98f48bce086b817dcf/internal/cri/server/images/imagefs_info.go#L54

[2] https://github.com/kubernetes/kubernetes/blob/26923b91e8cbd2d409e5d177ddd509429b76cb35/pkg/kubelet/stats/cri_stats_provider.go#L401

@ruiwen-zhao I think this is a problem that exists even today, so I don't think we need to block this KEP on that. We could try and address it for GA of this KEP/feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/windows Categorizes an issue or PR as relevant to SIG Windows.
Projects
Status: Tracked for Code Freeze
Status: In Progress
Development

No branches or pull requests