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

Support policy with EndpointSelector missing #1987

Merged
merged 1 commit into from
Nov 9, 2017
Merged

Conversation

raybejjani
Copy link
Contributor

Summary of changes:
If we import a CNP without an endpointSelector cilium-agent crashes.

@raybejjani raybejjani added the area/CI Continuous Integration testing issue or flake label Nov 7, 2017
@raybejjani raybejjani requested review from a team as code owners November 7, 2017 15:26
@raybejjani raybejjani added wip and removed area/CI Continuous Integration testing issue or flake labels Nov 7, 2017
log "importing CNP missing endpointSelector"
k8s_apply_policy kube-system create "${bad_policy_path}"

log "waiting 2 seconds for the policy to get to cilium-agent"
Copy link
Member

Choose a reason for hiding this comment

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

k8s_apply_policy will wait for the policy to be applied

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it does! our tests also told me to not use sleep ;) so that was handy.

@raybejjani raybejjani force-pushed the endpointselector-missing branch 4 times, most recently from 57c886c to d1919e4 Compare November 7, 2017 19:53
Copy link
Member

@tgraf tgraf left a comment

Choose a reason for hiding this comment

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

Does the runtime test work locally?

@@ -97,6 +97,14 @@ cat <<EOF | cilium -D policy import -
}]
EOF

log "import policy with without an endpointSelector"
Copy link
Member

Choose a reason for hiding this comment

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

s/with//

@raybejjani raybejjani force-pushed the endpointselector-missing branch 10 times, most recently from 2e29f81 to 96a8d1e Compare November 9, 2017 12:39
@raybejjani raybejjani requested a review from tgraf November 9, 2017 13:59
@raybejjani
Copy link
Contributor Author

OK. finally works. I tested to make sure the test failed without the fix.

The agent would crash when a bad policy was imported via k8s. This is
now rejected, and we have tests to make sure it doesn't happend again.
Signed-off-by: Ray Bejjani <[email protected]>
@tgraf tgraf merged commit 0a79507 into master Nov 9, 2017
@tgraf tgraf deleted the endpointselector-missing branch November 9, 2017 18:23
@tgraf tgraf added the release-note/bug This PR fixes an issue in a previous release of Cilium. label Nov 9, 2017
@tgraf tgraf changed the title Endpointselector missing Support policy with EndpointSelector missing Nov 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants