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.

Sign up for GitHub

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

Jump to bottom

install: add mountPropagation directive to bpf-maps volume in cilium DS #18438

Merged
merged 1 commit into from
Jan 12, 2022
Merged

install: add mountPropagation directive to bpf-maps volume in cilium DS #18438

merged 1 commit into from
Jan 12, 2022

Conversation

jibi
Copy link
Member

@jibi jibi commented Jan 11, 2022

The original backport was missing the "mountPropagation: Bidirectional"
directive for the bpf-maps volume, causing the bpffs to not get mounted
in the host.

Fixes: d221704 ("install/kubernetes: use bidirectional mounts to mount bpf fs")

@jibi jibi added kind/backports This PR provides functionality previously merged into master. backport/1.10 labels Jan 11, 2022
@jibi jibi requested a review from aanm January 11, 2022 09:46
@jibi jibi requested a review from a team as a code owner January 11, 2022 09:46
pchaigno
pchaigno approved these changes Jan 11, 2022
View reviewed changes
Copy link
Member

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Nice catch!

Looking at how this happened, it seems it was just an oversight and there isn't much I think we can do to avoid it happening again. We did follow the process thoroughly when making that backport, so neither tests nor reviews were skipped.

@jibi
Copy link
Member Author

jibi commented Jan 11, 2022

Nice catch!

Looking at how this happened, it seems it was just an oversight and there isn't much I think we can do to avoid it happening again. We did follow the process thoroughly when making that backport, so neither tests nor reviews were skipped.

Yep, I think this specific backport was just particularly tricky as master had diverged quite a bit from v1.10 wrt the helm templates

@jibi
Copy link
Member Author

jibi commented Jan 11, 2022 •
edited by maintainer-s-little-helper bot

/test-backport-1.10

Job 'Cilium-PR-K8s-GKE' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sChaosTest Connectivity demo application Endpoint can still connect while Cilium is not running

Failure Output

FAIL: Found 1 k8s-app=cilium logs matching list of errors that must be investigated:

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create a new GitHub issue to track it.

@jibi
Copy link
Member Author

jibi commented Jan 11, 2022 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-K8s-GKE

👍 created #18439

@jibi jibi mentioned this pull request Jan 11, 2022
Merged
@jibi jibi closed this Jan 11, 2022
@jibi jibi reopened this Jan 11, 2022
@jibi jibi force-pushed the pr/jibi/fix-v1.10-16656-backport branch from e5fcdf2 to beee219 Compare January 11, 2022 17:46
@pchaigno
Copy link
Member

@jibi You will need to rebase before starting the end-to-end tests, due to the recent CI breakage.

@pchaigno pchaigno added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jan 11, 2022
@jibi
install: add mountPropagation directive to bpf-maps volume in cilium DS
f733e14 
The original backport was missing the "mountPropagation: Bidirectional"
directive for the bpf-maps volume, causing the bpffs to not get mounted
in the host.

Fixes: d221704 ("install/kubernetes: use bidirectional mounts to mount bpf fs")
Signed-off-by: Gilberto Bertin <gilberto@isovalent.com>
@jibi jibi force-pushed the pr/jibi/fix-v1.10-16656-backport branch from beee219 to f733e14 Compare January 12, 2022 08:51
@jibi jibi removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Jan 12, 2022
@jibi
Copy link
Member Author

jibi commented Jan 12, 2022 •
edited by maintainer-s-little-helper bot

/test-backport-1.10

Job 'Cilium-PR-K8s-GKE' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sChaosTest Connectivity demo application Endpoint can still connect while Cilium is not running

Failure Output

FAIL: Found 1 k8s-app=cilium logs matching list of errors that must be investigated:

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create a new GitHub issue to track it.

@pchaigno
Copy link
Member

pchaigno commented Jan 12, 2022 •
edited

Restarting a couple CI jobs as they failed due to a full disk:
test-1.16-netnext
test-runtime
test-1.21-4.9

The GKE CI job failed with known test breakage #18285.

@aanm aanm added the kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. label Jan 12, 2022
@jibi
Copy link
Member Author

jibi commented Jan 12, 2022

GKE failed with a known issue, l4lb didn't start but it would fail anyway as the fix for that test hasn't been backported yet, marking as ready to merge

@jibi jibi added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jan 12, 2022
@joestringer
Copy link
Member

net-next run failed provisioning, but the likelihood that mountpropagation setting changes only cause a problem on that build and none of the other test runs seems pretty low: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.23-kernel-net-next/187/

Merging.

@joestringer joestringer merged commit 52a0400 into v1.10 Jan 12, 2022
@joestringer joestringer deleted the pr/jibi/fix-v1.10-16656-backport branch January 12, 2022 19:36
@jibi jibi mentioned this pull request Jan 14, 2022
Closed
2 tasks
@joestringer joestringer mentioned this pull request Jan 18, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pchaigno pchaigno pchaigno approved these changes

@aanm aanm aanm approved these changes

Assignees
No one assigned
Labels
kind/backports This PR provides functionality previously merged into master. kind/regression This functionality worked fine before, but was broken in a newer release of Cilium. ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jibi @pchaigno @joestringer @aanm