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

Add support for a drop-in kubelet configuration directory #3983

Open
8 of 12 tasks
haircommander opened this issue May 4, 2023 · 103 comments
Open
8 of 12 tasks

Add support for a drop-in kubelet configuration directory #3983

haircommander opened this issue May 4, 2023 · 103 comments
Assignees
Labels
lead-opted-in Denotes that an issue has been opted in to a release sig/node Categorizes an issue or PR as relevant to SIG Node. stage/beta Denotes an issue tracking an enhancement targeted for Beta status
Milestone

Comments

@haircommander
Copy link
Contributor

haircommander commented May 4, 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 May 4, 2023
@haircommander
Copy link
Contributor Author

/sig node

@k8s-ci-robot k8s-ci-robot added 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 May 4, 2023
@SergeyKanzhelev
Copy link
Member

/milestone v1.28

Adding to the milestone for tracking based on discussions at sig node meeting.

@k8s-ci-robot k8s-ci-robot added this to the v1.28 milestone May 5, 2023
@ffromani
Copy link
Contributor

ffromani commented May 9, 2023

+1!!! (non binding)
@haircommander I'd love to help here to get this in kubelet

@SergeyKanzhelev
Copy link
Member

/stage alpha

@k8s-ci-robot k8s-ci-robot added the stage/alpha Denotes an issue tracking an enhancement targeted for Alpha status label Jun 6, 2023
@SergeyKanzhelev
Copy link
Member

/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 Jun 8, 2023
@ruheenaansari34
Copy link

Hello @haircommander 👋, 1.28 Enhancements team here.

Just checking in as we approach enhancements freeze on 1:00 UTC on Friday 16th June 2023.

This enhancement is targeting stage alpha for 1.28 (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.28
  • KEP readme has a updated detailed test plan section filled out
  • KEP readme has up to date graduation criteria
  • KEP has a production readiness review that has been completed and merged into k/enhancements.

For this KEP, we would need to take care of:

  • Merging KEP update PR
  • The KEP number seems to be incorrect ie., should this be 3983 instead of 3893?

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

@yuqi-zhang
Copy link
Contributor

Hi @ruheenaansari34 , I've updated the ID, thanks for pointing that out!

Is there anything else needed at the moment other than getting consensus to merge the enhancement?

@ruheenaansari34
Copy link

@yuqi-zhang Please take care of the following before the Enhancement freeze to meet the requirements:

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

@yuqi-zhang
Copy link
Contributor

Thanks! I've marked it as implementable after discussion with @haircommander . We will work to get it merged

@Atharva-Shinde
Copy link
Contributor

Thanks @yuqi-zhang :)
With all the KEP requirements in place and merged into k/enhancements, this enhancement is all good for the upcoming enhancements freeze 🚀.
The status of this enhancement is marked as tracked. Please keep the issue description up-to-date with appropriate stages as well.

@Atharva-Shinde Atharva-Shinde moved this from At Risk to Tracked in 1.28 Enhancements Tracking Jun 16, 2023
@Rishit-dagli
Copy link
Member

Hello @haircommander 👋, 1.28 Docs Lead here.

Does this enhancement work planned for 1.28 require any new docs or modification to existing docs?

If so, please follows the steps here to open a PR against dev-1.28 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday 20th July 2023.

Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release.

Thank you!

@Rishit-dagli
Copy link
Member

Hey @haircommander , could you please create a docs PR even if it is a draft PR with no content yet against dev-1.28 branch in the k/website repo. The deadline to create this draft PR is Thursday 20th July 2023.

@haircommander
Copy link
Contributor Author

yes I will take care of it, thanks @Rishit-dagli !

@haircommander
Copy link
Contributor Author

done! kubernetes/website#42013

@ruheenaansari34
Copy link

Hey again @haircommander 👋
Just checking in as we approach Code freeze at 01:00 UTC Friday, 19th July 2023 .

I don't see any code (k/k) update PR(s) in the issue description so if there are any k/k related PR(s) that we should be tracking for this KEP please link them in the issue description above.

As always, we are here to help if any questions come up. Thanks!

@Atharva-Shinde
Copy link
Contributor

Hey @haircommander 👋 Enhancements Lead here,
With kubernetes/kubernetes#119390 merged as per the issue description I updated above, this enhancement is now tracked for v1.28 Code Freeze. Thanks!

@Rishit-dagli
Copy link
Member

Hello @haircommander wave: please take a look at Documenting for a release - PR Ready for Review to get your docs PR ready for review before Tuesday 25th July 2023. Thank you!

Ref: kubernetes/website#42013

@haircommander
Copy link
Contributor Author

thank you! I have moved it out of draft

@katcosgrove
Copy link
Contributor

Hey y'all! I'm swinging by for the v1.28 Docs team. Today is the docs deadline. Is there anything we can help you with to get this merged?

@haircommander
Copy link
Contributor Author

sorry I am updating it now, I will get this done soon.

@shecodesmagic shecodesmagic moved this from At risk for enhancements freeze to Tracked for enhancements freeze in 1.32 Enhancements Tracking Oct 11, 2024
@pacoxu
Copy link
Member

pacoxu commented Oct 12, 2024

As this KEP does not have a Feature gate, it used an env KUBELET_CONFIG_DROPIN_DIR_ALPHA to guard the feature.

alpha: "v1.28"
beta: "v1.31"
updated the beta version to 1.31; but Jordan's PR kubernetes/kubernetes#127421 was merged in 1.32. Is this a typo?

@kannon92
Copy link
Contributor

@pacoxu no, that is not a typo. Jordan merged a bug fix but this was promoted to beta in 1.31.

@pacoxu
Copy link
Member

pacoxu commented Oct 14, 2024

@pacoxu no, that is not a typo. Jordan merged a bug fix but this was promoted to beta in 1.31.

I mean that it should be already beta in v1.30 as the alpha ENV was removed in v1.30.

@SergeyKanzhelev
Copy link
Member

yes, it stays in beta for one more release with the idea to pre-enable the drop-in directory and ensure that future version bump of config api will not break this

@mbianchidev
Copy link
Member

Hey @haircommander,👋 from the v1.32 Communications Team!

We'd love for you to consider writing a feature blog about your enhancement.
Some reasons why you might want to write a blog for this feature include (but are not limited to) if this introduces breaking changes, is important to our users, or has been in progress for a long time and it is graduating.

To opt-in, let us know by opening a Feature Blog placeholder PR against the website repository by 30th Oct 2024. For more information about writing a blog see the blog contribution guidelines.

Note: In your placeholder PR, use XX characters for the blog date in the front matter and file name. We will work with you on updating the PR with the publication date once we finalize the blog schedule.

@hacktivist123
Copy link

Hello @haircommander 👋 1.32 Docs Shadow here.

Does this enhancement work planned for 1.32 require any new docs or modifications to existing docs?
If so, please follow the steps here to open a PR against the dev-1.32 branch in the k/website repo. This PR can be just a placeholder at this time and must be created before Thursday, October 24th 2024 18:00 PDT.

Also, take a look at Documenting for a release to get yourself familiarize with the docs requirement for the release.
Thank you!

@hacktivist123
Copy link

Hello @haircommander 👋 1.32 Docs Shadow here.

This is just a reminder to open a placeholder PR against dev-1.32 branch in the k/website repo for this (steps available here) for this KEP if it requires new or modifications to existing docs:

The deadline for this is Thursday, Oct 24 at 18:00 PDT.
Thanks! 🚀

@haircommander
Copy link
Contributor Author

we're retargeting beta here so I don't think there should be a docs update. is that correct @sohankunkerkar ?

@SergeyKanzhelev
Copy link
Member

we're retargeting beta here so I don't think there should be a docs update. is that correct @sohankunkerkar ?

My understanding was that we will be doing this: "Set the default value of --config-dir to /etc/kubernetes/kubelet.conf.d. Users can disable this field by setting it to an empty string."

If so, some docs update will be needed!

@haircommander
Copy link
Contributor Author

good point, done here kubernetes/website#48482

@liggitt
Copy link
Member

liggitt commented Oct 21, 2024

My understanding was that we will be doing this: "Set the default value of --config-dir to /etc/kubernetes/kubelet.conf.d. Users can disable this field by setting it to an empty string."

Does that error if /etc/kubernetes/kubelet.conf.d does not exist? If so, won't that break ~all existing installations that haven't created that directory? That doesn't seem reasonable.

@haircommander
Copy link
Contributor Author

I would expect the kubelet attempt to create the directory in that case

@liggitt
Copy link
Member

liggitt commented Oct 22, 2024

I would expect the kubelet attempt to create the directory in that case

hmm... I wouldn't... I'd expect config locations on disk to be read-only from the kubelet's perspective

@ffromani
Copy link
Contributor

ffromani commented Oct 22, 2024

The following may be silly, but still: golang's os.ReadDir return a clear specific error in case the directory being open doesn't exist. Maybe we can catch it with os.IsNotExist and carry along only on this path?
(disclosure: don't remember what the KEP prescribes in this case)

@SergeyKanzhelev
Copy link
Member

@sohankunkerkar it will be great if you can summarize the behavior of misconfiguration/misconfiguration.

Thinking about it - maybe the typical installation will be that both config file and config dir will be specified as an arguments for the systemd unit so they will be (mostly) readonly and not expected to be changed. In this case, making the default to be a predefined folder doesn't make too much sense.

Also if default will be configured - make sure to update e2e_node framework to change the folder for every test run. So there is no leftovers from the previous test making the next test fail.

@brandond
Copy link

Maybe the typical installation will be that both config file and config dir will be specified as an arguments for the systemd unit so they will be (mostly) readonly and not expected to be changed

Are we talking defaults for kubeadm-based distros now? Or just the default behavior for the kubelet binary?

@SergeyKanzhelev
Copy link
Member

@haircommander @sohankunkerkar what do you say we remove this defaulting? It feels that the common usage pattern will be to specify this folder in systemd unit config anyways. So having a default folder is more troubles and surprises for end users than a benefit.

@mbianchidev
Copy link
Member

mbianchidev commented Oct 28, 2024

Hey @haircommander,👋 from the v1.32 Communications Team!

We'd love for you to consider writing a feature blog about your enhancement. Some reasons why you might want to write a blog for this feature include (but are not limited to) if this introduces breaking changes, is important to our users, or has been in progress for a long time and it is graduating.

To opt-in, let us know by opening a Feature Blog placeholder PR against the website repository by 30th Oct 2024. For more information about writing a blog see the blog contribution guidelines.

Note: In your placeholder PR, use XX characters for the blog date in the front matter and file name. We will work with you on updating the PR with the publication date once we finalize the blog schedule.

Hey there!
A reminder since the blog opt-in deadline is close :)

@shecodesmagic
Copy link

Hello @haircommander 👋, Enhancements team here (again 😁 )

With all the implementation(code related) PRs merged as per the issue description:

This enhancement is now marked as tracked for code freeze for the v1.32 Code Freeze!

Please note that KEPs targeting stable need to have the status field marked as implemented in the kep.yaml file after code PRs are merged and the feature gates are removed.

@shecodesmagic shecodesmagic moved this from Tracked for enhancements freeze to Tracked for code freeze in 1.32 Enhancements Tracking Oct 30, 2024
@sohankunkerkar
Copy link
Member

@haircommander @sohankunkerkar what do you say we remove this defaulting? It feels that the common usage pattern will be to specify this folder in systemd unit config anyways. So having a default folder is more troubles and surprises for end users than a benefit.

@SergeyKanzhelev are you asking to revert to the same state as the v1.31 release

@SergeyKanzhelev
Copy link
Member

@haircommander @sohankunkerkar what do you say we remove this defaulting? It feels that the common usage pattern will be to specify this folder in systemd unit config anyways. So having a default folder is more troubles and surprises for end users than a benefit.

@SergeyKanzhelev are you asking to revert to the same state as the v1.31 release

Was it implemented already in 1.32? I suggest we remove the line "Set the default value of --config-dir to /etc/kubernetes/kubelet.conf.d. Users can disable this field by setting it to an empty string." from beta graduation criteria.

@sohankunkerkar
Copy link
Member

@haircommander @sohankunkerkar what do you say we remove this defaulting? It feels that the common usage pattern will be to specify this folder in systemd unit config anyways. So having a default folder is more troubles and surprises for end users than a benefit.

@SergeyKanzhelev are you asking to revert to the same state as the v1.31 release

Was it implemented already in 1.32? I suggest we remove the line "Set the default value of --config-dir to /etc/kubernetes/kubelet.conf.d. Users can disable this field by setting it to an empty string." from beta graduation criteria.

Right, that was the change added in #4889. So essentially, the ask is to revert that change, which means sticking to the requirements mentioned in v1.31.

@SergeyKanzhelev
Copy link
Member

Yes, we will default to "not configured" and expect many k8s deployments will preconfigure their drop-in folders to the same or similar location.

@haircommander
Copy link
Contributor Author

I think eventually we'll want a default but we can not have that for beta. if the default directory doesn't exist, we can just skip (similar to what we would do if the directory is empty). How does that sound @SergeyKanzhelev ?

@SergeyKanzhelev
Copy link
Member

Why would we need a default? We ask to specify the config file explicitly, why not require to do the same for the drop in folder? And for the end-user, it will NOT need to be set as the typical installation will include the default folder pre-configured in systemd unit file

@haircommander
Copy link
Contributor Author

totally, for some reason I had the impression there was a default set for --config-dir in kubelet. Agreed there should be no default.

@sohankunkerkar
Copy link
Member

@SergeyKanzhelev closing the loop: #4966

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lead-opted-in Denotes that an issue has been opted in to a release sig/node Categorizes an issue or PR as relevant to SIG Node. stage/beta Denotes an issue tracking an enhancement targeted for Beta status
Projects
Status: Tracked
Status: Tracked for Code Freeze
Status: Tracked for Doc Freeze
Status: Tracked for code freeze
Status: Tracked
Development

No branches or pull requests