Skip to content
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 quoting in nodeinit temporary cilium config #30399

Merged
merged 1 commit into from
Jan 25, 2024

Conversation

tlcowling
Copy link
Contributor

The Cilium nodeinit startup script lays down a temporary CNI config in order to be able to restart a version of containerd that doesn't allow a missing CNI config.

This commit fixes an issue with missing double quotes in the creation of the temporary config that causes an error in containerd which in turn causes Kubernetes nodes to become NotReady

I also considered heredoc or escaping the quote characters but settled on single quoting as I think its the most readable one line solution without needing to deal with the indentation issue with heredoc

Fix nodeinit issue causing NotReady state in Kubernetes nodes when laying down an incorrect CNI config

The Cilium nodeinit startup script lays down a temporary CNI config in order to
be able to restart a version of containerd that doesn't allow a missing CNI
config.

This commit fixes an issue with missing double quotes in the temporary config
which causes an error in containerd and leads to NotReady Kubernetes nodes

I also considered heredoc or escaping the quote characters but settled on single
quoting as I think its the most readable one line solution without needing to
deal with the indentation issue with heredoc

Signed-off-by: Tom Cowling <952241+tlcowling@users.noreply.github.com>
@tlcowling tlcowling requested review from a team as code owners January 24, 2024 09:43
@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 Jan 24, 2024
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jan 24, 2024
Copy link
Member

@gandro gandro left a comment

Choose a reason for hiding this comment

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

Thank you! I do wonder why we did not catch this earlier

@gandro gandro added release-note/bug This PR fixes an issue in a previous release of Cilium. needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jan 24, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in v1.15.0-rc.1 Jan 24, 2024
@gandro
Copy link
Member

gandro commented Jan 24, 2024

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 25, 2024
@gandro gandro added this pull request to the merge queue Jan 25, 2024
Merged via the queue into cilium:main with commit 87d948e Jan 25, 2024
63 checks passed
@joamaki joamaki mentioned this pull request Jan 30, 2024
7 tasks
@joamaki joamaki added backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. and removed needs-backport/1.13 This PR / issue needs backporting to the v1.13 branch labels Jan 30, 2024
@joamaki joamaki mentioned this pull request Jan 30, 2024
28 tasks
@joamaki joamaki added backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. and removed needs-backport/1.15 This PR / issue needs backporting to the v1.15 branch labels Jan 30, 2024
@github-actions github-actions bot added backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. and removed backport-pending/1.13 The backport for Cilium 1.13.x for this PR is in progress. labels Jan 31, 2024
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from main to Backport pending to v1.15 in v1.15.0-rc.1 Jan 31, 2024
@joamaki joamaki mentioned this pull request Jan 31, 2024
17 tasks
@joamaki joamaki added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Jan 31, 2024
@aanm aanm added this to Backport pending to v1.15 in 1.15.1 Jan 31, 2024
@aanm aanm removed this from Backport pending to vv1.15.0-rc in v1.15.0-rc.1 Jan 31, 2024
@aanm aanm removed this from Backport pending to v1.15 in 1.15.1 Jan 31, 2024
@aanm aanm added backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. and removed backport-pending/1.15 The backport for Cilium 1.15.x for this PR is in progress. labels Jan 31, 2024
@aanm aanm added this to Backport done to v1.15 in v1.15.0-rc.1 Jan 31, 2024
@github-actions github-actions bot added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.13 The backport for Cilium 1.13.x for this PR is done. backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. backport-done/1.15 The backport for Cilium 1.15.x for this PR is done. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
v1.15.0-rc.1
Backport done to v1.15
Development

Successfully merging this pull request may close these issues.

None yet

4 participants