-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Fix indentation issue in Cilium values file and ensure booleans are lowercase #12280
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
Fix indentation issue in Cilium values file and ensure booleans are lowercase #12280
Conversation
Hi @spantaleev. 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 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-sigs/prow repository. |
dbd3a3b
to
4157595
Compare
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.
Duplicate with #12244 |
/ok-to-test But I like this part of the change better. |
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.
Some comments.
…owercase This patch fixes the indentation in the `encryption` section. Previously configuration like this: ```yml cilium_encryption_enabled: true cilium_encryption_type: wireguard ``` Would template to a `values.yaml` file with indentation that looks like this: ```yml encryption: enabled: True type: wireguard nodeEncryption: False ``` instead of this: ```yml encryption: enabled: true type: wireguard nodeEncryption: false ``` This syntax issue causes an error during Cilium installation. This patch also makes all boolean values in this template file go through the `to_json` filter. Since values like `True` and `False` are not compliant with the YAML v1.2 spec, avoiding them is preferable. `to_json` may be used for all other values in this template to ensure we end up with a valid YAML document in all cases (even when various strings include special characters), but this was left for another (future) patch.
4157595
to
b10de2f
Compare
#12244 only (potentially) fixes stuff around the All the other It seems like using With the other |
Please edit the release note, this is a patch for the Cilium installation. |
I've updated the Does this PR introduce a user-facing change?: value in the PR description above. |
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.
Thank you!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cyclinder, spantaleev, tico88612 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 |
/lgtm |
/cherry-pick release-2.28 |
@tico88612: once the present PR merges, I will cherry-pick it on top of In response to this:
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. |
@tico88612: new pull request created: #12283 In response to this:
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. |
/cherry-pick release-2.28 |
@tico88612: new pull request could not be created: failed to create pull request against kubernetes-sigs/kubespray#release-2.28 from head k8s-infra-cherrypick-robot:cherry-pick-12280-to-release-2.28: status code 422 not one of [201], body: {"message":"Validation Failed","errors":[{"resource":"PullRequest","code":"custom","message":"A pull request already exists for k8s-infra-cherrypick-robot:cherry-pick-12280-to-release-2.28."}],"documentation_url":"https://docs.github.com/rest/pulls/pulls#create-a-pull-request","status":"422"} In response to this:
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. |
This patch fixes the indentation in the
encryption
section. Previously configuration like this:Would template to a
values.yaml
file with indentation that looks like this:instead of this:
This syntax issue causes an error during Cilium installation.
This patch also makes all boolean values in this template file to go through the
to_json
filter. Since values likeTrue
andFalse
are not compliant with the YAML v1.2 spec, avoiding them is preferable.to_json
may be used for all other values in this template to ensure we end up with a valid YAML document in all cases (even when various strings include special characters), but this was left for another future patch.What type of PR is this?
/kind bug
What this PR does / why we need it:
#12101 (part of Kubespray v2.28.0) completely reworked the Cilium installation method, so people who were enabling encryption for Cilium (via
cilium_encryption_enabled: true
..) would encounter errors during thecilium install
command executed by the Kubespray playbook.Which issue(s) this PR fixes:
No open issues about this yet, as far as I see.
Special notes for your reviewer:
None
Does this PR introduce a user-facing change?: