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

bpf: Fix name for example map #10768

Merged

Conversation

joestringer
Copy link
Member

These map names were the same as the ones in Cilium, which is explicitly
what we don't want, as that can cause local testing to interfere with
real Cilium agent instances. This was picked up by running Cilium then
running test/bpf/verifier-test.sh This was picked up by running Cilium
then running test/bpf/verifier-test.sh, which fails due to the maps
already existing on the filesystem.

@joestringer joestringer added pending-review kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact. labels Mar 30, 2020
@joestringer joestringer requested review from brb and a team March 30, 2020 22:33
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.8.0 Mar 30, 2020
@joestringer
Copy link
Member Author

test-focus RuntimeVerifier

@coveralls
Copy link

coveralls commented Mar 30, 2020

Coverage Status

Coverage increased (+0.03%) to 45.498% when pulling 787e7f7 on joestringer:submit/fix-verifier-test-map-name into 05f6bbc on cilium:master.

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@tklauser tklauser left a comment

Choose a reason for hiding this comment

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

Should we do the same for LB4_REVERSE_NAT_SK_MAP and LB6_REVERSE_NAT_SK_MAP?

These map names were the same as the ones in Cilium, which is explicitly
what we *don't* want, as that can cause local testing to interfere with
real Cilium agent instances. This was picked up by running Cilium then
running test/bpf/verifier-test.sh This was picked up by running Cilium
then running test/bpf/verifier-test.sh, which fails due to the maps
already existing on the filesystem.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer force-pushed the submit/fix-verifier-test-map-name branch from 0f552b8 to 787e7f7 Compare March 31, 2020 17:14
@joestringer
Copy link
Member Author

@tklauser I updated those two maps as well. They weren't caught because we compile "maximum complexity" for older kernels and test with that, but not with features dependent on newer kernels (see also #10712 )

@joestringer
Copy link
Member Author

test-me-please

1 similar comment
@aanm
Copy link
Member

aanm commented Apr 1, 2020

test-me-please

@joestringer
Copy link
Member Author

Tests with kernel passed which shows the changes are good. Regular Ginkgo run seems to be flaking on known policy issue.

@joestringer joestringer merged commit c08e72a into cilium:master Apr 1, 2020
1.8.0 automation moved this from In progress to Merged Apr 1, 2020
@joestringer joestringer deleted the submit/fix-verifier-test-map-name branch April 1, 2020 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.8.0
  
Merged
Development

Successfully merging this pull request may close these issues.

None yet

6 participants