kubernetes-sigs
🌱 Add NamingStrategy to MachineDeployment
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes part of #10577
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all
/hold on https://github.com/kubernetes-sigs/cluster-api/pull/11123 so we make sure feedback there is incorporated here
cc @sbueringer @fabriziopandini @chrischdi This one is ready for review, please check.
I have verified that it is working as expected with a real end-to-end test. Thanks for working on this @adilGhaffarDev !
I don't know when I'll get around to it. Not much time but a lot of work left until code freeze / PTO.
In general I'm fine with keeping the flag for another minor release
If the PR is not merged in the meantime, I'll get back to this once I'm back from PTO
@fabriziopandini @chrischdi sorry for the delay, I was on vacation, and I was not able to address all the comments. But now all comments are addressed please take a look.
LGTM label has been added.
The last findings I could find. As of that lgtm
rebased the PR please re lgtm
LGTM label has been added.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: chrischdi
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [chrischdi]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Thanks everyone for working on this, really appreciate it. I know it took a while and some patience :) (realistically mostly due to maintainer bandwith limitations)
@sm4ll-3gg : couldn't you use v1.10 to migrate away from use-deprecated-infra-machine-naming=true?
I think we should not introduce new API fields in a patch version (edit: if possible).
@sm4ll-3gg : couldn't you use v1.10 to migrate away from
use-deprecated-infra-machine-naming=true?I think we should not introduce new API fields in a patch version.
Unfortunately no, I described the issue in the Slack: https://kubernetes.slack.com/archives/C8TSNPY4T/p1746453230357789
Not sure about introducing new fields in a patch version, but the same feature was added for KCP as part of 1.9.x patch at least (https://github.com/kubernetes-sigs/cluster-api/pull/11123). Just one case I'm aware of)
Some data on what happened:
- KCP NamingStrategy implementation in #11123
- Part of v1.9.0
- MD NamingStrategy implementation (this PR)
- Part of v1.10.0
- Removal of the flag in #11679
- Part of v1.10.0
I discussed it with @sbueringer and its more straight forward to cherry-pick this back to v1.9 then reverting the flag removal for v1.10.
/cherry-pick release-1.9
@chrischdi: new pull request created: #12181
In response to this:
Some data on what happened:
- KCP NamingStrategy implementation in #11123
- Part of v1.9.0
- MD NamingStrategy implementation (this PR)
- Part of v1.10.0
- Removal of the flag in #11679
- Part of v1.10.0
I discussed it with @sbueringer and its more straight forward to cherry-pick this back to v1.9 then reverting the flag removal for v1.10.
/cherry-pick release-1.9
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-sigs/prow repository.
I discussed it with @sbueringer and its more straight forward to cherry-pick this back to v1.9 then reverting the flag removal for v1.10.
@chrischdi thank you for the quick help. JFYI, if i'm not wrong, you don't have to revert flag removal from 1.10. Provided that namingStrategy for MD is released in 1.9.x, users will be able to migrate smoothly from deprecated naming and discard the flag using 1.9, and only after that upgrade to 1.10.
@sm4ll-3gg Yes, that's correct. We were just thinking if we should revert the flag removal or backport. But now that we backported we don't have to revert the flag as you are saying.