-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fixes for Flatcar cloud init #272
Conversation
Hi @LittleFox94. Thanks for your PR. I'm waiting for a kubermatic member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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/test-infra repository. |
4955579
to
8100cd4
Compare
So the |
yep, coreos/flatcar cloud-config does not support it: https://github.com/flatcar/coreos-cloudinit/blob/flatcar-master/Documentation/cloud-config.md |
Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
This time done in ExecStartPre of kubelet service. Kubelet will not start without overcommit_memory enabled. Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
5a47053
to
b775507
Compare
Only thing I'm not sure about rn is how compatible the changed cloud-init template is with other non-coreos/flatcar operating systems, they probably don't have support for coreos.units .. if they are ok with this key being present, we could probably just add+run the setup.service unit via both file+runcmd and coreos.units what do you think? |
@LittleFox94 Let me take a look at this PR today. My initial idea was to introduce a separate Provisioning Utility for this case since units are only supported in coreos-cloud-init not the standard one. |
/ok-to-test |
@LittleFox94 we can proceed with this PR. We'll discuss internally how we want to treat units in cloud-init but that shouldn't block your PR. |
@ahmedwaleedmalik sorry for the delay, what do you need from me then - only the Draft status to be removed or anything else? |
No worries, I've removed the draft status. This looks good to be merged now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
LGTM label has been added. Git tree hash: 192c9b8357087b003cbbcec7ff038b2023d87ecd
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmedwaleedmalik, LittleFox94 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
many thanks :) |
The output redirection added in kubermatic#272 is not interpreted by systemd, which instead just passes it as arguments to echo. Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
The output redirection added in #272 is not interpreted by systemd, which instead just passes it as arguments to echo. Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
What this PR does / why we need it:
This adds support for OperatingSystemProfileSpec.Units in cloud-init and fixes the default OSP for flatcar with cloud-init by starting the setup by using a unit instead of generic file and re-adding the configuration in /proc removed in 01829fc.
What type of PR is this?
/kind bug
Special notes for your reviewer:
I think no changelog is required, but not sure hence leaving that empty instead of writing NONE.
Does this PR introduce a user-facing change? Then add your Release Note here:
Documentation: