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

Improve k0s-managed containerd config detection #4117

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

twz123
Copy link
Member

@twz123 twz123 commented Feb 22, 2024

Description

The previous detection method was to read the whole file and search each line for the magic string "# k0s_managed=true". This seems excessive and unexpected, as it will catch any substring, even if it is not at the beginning of the file, the start of a line, or even inside a comment. It would even have caught this as part of a TOML string value.

Change this behavior by expecting the first line of the file to be exactly this magic string. All files generated by k0s will have exactly that first line. Also return any previously swallowed errors back to the caller.

Add more test cases to ensure that the generated default config is recognized as being managed by k0s, and that a file not starting with the magic line is recognized as not being managed by k0s.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

How Has This Been Tested?

  • Manual test
  • Auto test added

Checklist:

  • My code follows the style guidelines of this project
  • My commit messages are signed-off
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@twz123 twz123 requested a review from a team as a code owner February 22, 2024 14:48
@twz123 twz123 requested review from ncopa and makhov February 22, 2024 14:48
@twz123 twz123 force-pushed the containerd-config-detection branch from 4a32e44 to b210ab4 Compare February 22, 2024 14:53
@twz123 twz123 marked this pull request as draft February 23, 2024 16:29
Copy link
Contributor

github-actions bot commented Mar 1, 2024

This pull request has merge conflicts that need to be resolved.

@twz123 twz123 force-pushed the containerd-config-detection branch from b210ab4 to d9abb59 Compare March 12, 2024 12:32
@twz123
Copy link
Member Author

twz123 commented Mar 27, 2024

Tests can be fixed after #4160 has landed.

@twz123 twz123 force-pushed the containerd-config-detection branch from d9abb59 to 1f285bd Compare March 28, 2024 14:31
The previous detection method was to read the whole file and search each
line for the magic string "# k0s_managed=true". This seems excessive
and unexpected, as it will catch any substring, even if it is not at the
beginning of the file, the start of a line, or even inside a comment. It
would even have caught this as part of a TOML string value.

Change this behavior by expecting the first line of the file to be
exactly this magic string. All files generated by k0s will have exactly
that first line. Also return any previously swallowed errors back to
the caller.

Add more test cases to ensure that the generated default config is
recognized as being managed by k0s, and that a file not starting with
the magic line is recognized as not being managed by k0s.

Signed-off-by: Tom Wieczorek <[email protected]>
@twz123 twz123 force-pushed the containerd-config-detection branch from 1f285bd to 0780516 Compare April 2, 2024 06:59
@twz123 twz123 marked this pull request as ready for review April 2, 2024 12:56
@twz123 twz123 merged commit 4c0b3bd into k0sproject:main Apr 22, 2024
74 checks passed
@twz123 twz123 deleted the containerd-config-detection branch April 22, 2024 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants