Skip to content

Conversation

walnuts1018
Copy link
Contributor

Modified the condition for adding the tolerations key to ensure it correctly adds the key even when .Values.operator.tolerations does not exist, by including an additional OR condition that checks for the presence of .Values.agentNotReadyTaintKey.

Fixes: #40937

… for cilium-operator deployment

The current Helm Template adds `tolerations` to the cilium-operator deployment by combining:
  - The contents of `.Values.operator.tolerations`
  - A `operator: Exists` toleration with `.Values.agentNotReadyTaintKey` as the key

However, the `tolerations` key is only added when `.Values.operator.tolerations` exists, so attempting to set `.Values.operator.tolerations` to an empty value results in an error:

```
Error: YAML parse error on cilium/templates/cilium-operator/deployment.yaml: error converting YAML to JSON: yaml: line 110: did not find expected key
```

This patch modifies the condition for adding the `tolerations` key to include an additional OR condition checking for the existence of `.Values.agentNotReadyTaintKey`, thereby resolving this error.

Fixes: cilium#40937
Signed-off-by: walnuts1018 <r.juglans.1018@gmail.com>
@walnuts1018 walnuts1018 requested review from a team as code owners August 5, 2025 07:33
@walnuts1018 walnuts1018 requested review from joamaki and brlbil August 5, 2025 07:33
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 5, 2025
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Aug 5, 2025
@walnuts1018 walnuts1018 marked this pull request as draft August 5, 2025 07:33
@walnuts1018 walnuts1018 marked this pull request as ready for review August 5, 2025 07:49
@joestringer joestringer requested a review from parlakisik August 5, 2025 21:00
@parlakisik
Copy link
Contributor

/test

@parlakisik
Copy link
Contributor

/lgtm

@walnuts1018 walnuts1018 changed the title Corrected logic for adding tolerations key in helm template for cilium-operator deployment Correcte logic for adding tolerations key in helm template for cilium-operator deployment Aug 6, 2025
@walnuts1018 walnuts1018 changed the title Correcte logic for adding tolerations key in helm template for cilium-operator deployment Corrected logic for adding tolerations key in helm template for cilium-operator deployment Aug 6, 2025
@parlakisik
Copy link
Contributor

@walnuts1018 can you checked failed tests ?

@walnuts1018
Copy link
Contributor Author

I'll check it

@maintainer-s-little-helper
Copy link

Commit af2e1db does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Aug 6, 2025
Signed-off-by: walnuts1018 <r.juglans.1018@gmail.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Aug 6, 2025
Copy link
Contributor

@joamaki joamaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joamaki joamaki added the release-note/misc This PR makes changes that have no direct user impact. label Aug 6, 2025
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Aug 6, 2025
@brlbil
Copy link
Contributor

brlbil commented Aug 6, 2025

/test

Copy link
Contributor

@brlbil brlbil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@parlakisik
Copy link
Contributor

@walnuts1018 the conformance gateway api test is failing .
Can you check the issue

@aanm aanm enabled auto-merge August 11, 2025 08:33
@aanm
Copy link
Member

aanm commented Aug 11, 2025

@walnuts1018 the conformance gateway api test is failing . Can you check the issue

it's a flake.

@aanm
Copy link
Member

aanm commented Aug 11, 2025

/test

@aanm aanm added this pull request to the merge queue Aug 11, 2025
Merged via the queue into cilium:main with commit 1a61ff9 Aug 11, 2025
68 of 79 checks passed
@parlakisik
Copy link
Contributor

This should be backported for 1.18 too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/community-contribution This was a contribution made by a community member. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm: Empty .Values.operator.tolerations results in invalid YAML
5 participants