Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Check for duplicated sysctl keys #730

Merged
merged 1 commit into from
May 4, 2022

Conversation

mmirecki
Copy link
Contributor

The tuning-cni config currently allows to specif duplicated sysctls, also with conflicting values. Duplicated sysctl entries are usually not used intentionally, but are rather the result of an error in configuration.
This PR adds a check for duplicated sysctl's.

@squeed
Copy link
Member

squeed commented Apr 20, 2022

@mmirecki looks good! Can you fix DCO and go fmt? Then I can merge when CI goes green.

@mmirecki mmirecki force-pushed the tuning_duplicate_check branch from c09a7df to 72db601 Compare April 21, 2022 08:08
@mmirecki mmirecki force-pushed the tuning_duplicate_check branch from 72db601 to 7accdeb Compare April 21, 2022 13:02
@squeed
Copy link
Member

squeed commented Apr 26, 2022

lgtm, will merge tomorrow

@mmirecki mmirecki force-pushed the tuning_duplicate_check branch from 7accdeb to 50ca338 Compare April 27, 2022 14:10
@dcbw
Copy link
Member

dcbw commented Apr 27, 2022

LGTM after the dupe check gets moved to parseConf() instead of just in cmdAdd().

@squeed
Copy link
Member

squeed commented Apr 27, 2022

@mmirecki @dcbw observes we should do this when parsing the config for CHECK too, right?

Signed-off-by: mmirecki <mmirecki@redhat.com>
@mmirecki mmirecki force-pushed the tuning_duplicate_check branch from 50ca338 to 7c452c7 Compare April 27, 2022 20:13
@dcbw
Copy link
Member

dcbw commented May 4, 2022

/lgtm

@dcbw dcbw merged commit 9c59728 into containernetworking:main May 4, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants