cluster-api
cluster-api copied to clipboard
kubernetes-sigs
✨ Add bootCommands to cloud-init file generation
What this PR does / why we need it: Adds the ability to provide bootcmd commands to cloud-init via the KubeadmConfig or KubeadmConfigTemplates custom resources. (Same for KubeadmControlPlane)
One thing that I know that does not work at the time of creating this PR is validating/mutating webhooks for KubeadmConfig and KubeadmConfigTemplates. Some pointers to where to change that would be appreciated.
Which issue(s) this PR fixes Fixes #
Welcome @davidumea!
It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.
You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.
You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.
You may want to refer to our testing guide if you run into trouble with your tests not passing.
If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!
Thank you, and welcome to Kubernetes. :smiley:
Hi @davidumea. Thanks for your PR.
I'm waiting for a kubernetes-sigs 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.
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.
Are there version constraints around bootcmd? Does it already exist in cloud-init for a long time?
What happens if we render it but the cloud-init version in the machine does not support it? Does it make cloud-init fail as a whole or is it just ignored?
Edit: looks like it is already there for a long time (from the start?) at 0.7.7 https://cloudinit.readthedocs.io/en/0.7.7/topics/examples.html
Are there version constraints around
bootcmd? Does it already exist in cloud-init for a long time?Edit: looks like it is already there for a long time (from the start?) at
0.7.7https://cloudinit.readthedocs.io/en/0.7.7/topics/examples.html
Exactly I don't think that should be an issue as bootcmd has been part of cloud-init for a long time.
What happens if we render it but the cloud-init version in the machine does not support it? Does it make cloud-init fail as a whole or is it just ignored?
During my testing, if the bootcmd module fails e.g. due to invalid commands, cloud-init still continues with the following modules. I haven't been able to test running this with a cloud-init version that doesn't support bootcmd.
@chrischdi: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:
/test pull-cluster-api-build-main/test pull-cluster-api-e2e-blocking-main/test pull-cluster-api-e2e-conformance-ci-latest-main/test pull-cluster-api-e2e-conformance-main/test pull-cluster-api-e2e-latestk8s-main/test pull-cluster-api-e2e-main/test pull-cluster-api-e2e-mink8s-main/test pull-cluster-api-e2e-upgrade-1-31-1-32-main/test pull-cluster-api-test-main/test pull-cluster-api-test-mink8s-main/test pull-cluster-api-verify-main
The following commands are available to trigger optional jobs:
/test pull-cluster-api-apidiff-main
Use /test all to run the following jobs that were automatically triggered:
pull-cluster-api-apidiff-mainpull-cluster-api-build-mainpull-cluster-api-e2e-blocking-mainpull-cluster-api-test-mainpull-cluster-api-verify-main
In response to this:
/test help
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.
/retest
Looks like a flake in creating the docker container and overlapping ports
@fabriziopandini Can you take another look? I added some new tests and addressed the comments
LGTM label has been added.
LGTM label has been added.
@chrischdi Is there anything I can do for this to progress?
Thanks for asking, I'm sure this is in the backlog of some maintainers, at least this should also bring it up again in notifications if it got lost ;-)
Just a heads up. Code freeze for 1.10 is on 18th March. So we maybe have 1,5 weeks left if this should make it into v1.10.
Seems like there has been some updates to the linter on main, will rebase and fix
@davidumea One very last comment. Feel free to squash and ping me afterwards and I'll merge
@davidumea One very last comment. Feel free to squash and ping me afterwards and I'll merge
Sure, I added MinLength/MaxLength for bootcommands, rebased on main again and squashed.
When running make test locally the TestNewClusterClient test failed for me but it also failed when running it on main, so I figured it was unrelated to the changes in this PR.
Agree, that seems unrelated. (tests are green on my machine)
Thank you very much for working on this. Sorry it took a while
/lgtm /approve
LGTM label has been added.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: sbueringer
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [sbueringer]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment