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

validator: unbreak sysctl net.* validation #1149

Merged
merged 2 commits into from
Oct 26, 2016
Merged

validator: unbreak sysctl net.* validation #1149

merged 2 commits into from
Oct 26, 2016

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Oct 26, 2016

When changing this validation, the code actually allowing the validation
to pass was removed. This meant that any net.* sysctl would always fail
to validate.

Fixes: bc84f83 ("fix moby/moby#27484")
Reported-by: Justin Cormack justin.cormack@docker.com
Signed-off-by: Aleksa Sarai asarai@suse.de

Fixes #1138

/cc @justincormack Does this fix the issue you mentioned?

When changing this validation, the code actually allowing the validation
to pass was removed. This meant that any net.* sysctl would always fail
to validate.

Fixes: bc84f83 ("fix moby/moby#27484")
Reported-by: Justin Cormack <justin.cormack@docker.com>
Signed-off-by: Aleksa Sarai <asarai@suse.de>
@cyphar
Copy link
Member Author

cyphar commented Oct 26, 2016

I've also added a test -- it turns out we weren't testing that sysctl validation ever succeeded.

Previously we only tested failures, which causes us to miss issues where
setting sysctls would *always* fail.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
@gaocegege
Copy link
Contributor

Ah, sorry about it. I was too careless.

LGTM

@cyphar
Copy link
Member Author

cyphar commented Oct 26, 2016

Don't sweat it. 😸

/cc @opencontainers/runc-maintainers

@hqhq
Copy link
Contributor

hqhq commented Oct 26, 2016

LGTM

Approved with PullApprove

1 similar comment
@crosbymichael
Copy link
Member

crosbymichael commented Oct 26, 2016

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit 6328410 into opencontainers:master Oct 26, 2016
@cyphar cyphar deleted the fix-sysctl-validation branch October 26, 2016 16:34
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.

Docker run with --sysctl accepts with --net=host and option values other than 0 and 1
5 participants