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

Add Bidirectional mount in Cilium DaemonSet #16656

Merged
merged 1 commit into from Aug 7, 2021
Merged

Add Bidirectional mount in Cilium DaemonSet #16656

merged 1 commit into from Aug 7, 2021

Conversation

aanm
Copy link
Member

@aanm aanm commented Jun 25, 2021

install/kubernetes: use bidirectional mounts to mount bpf fs

Bidirectional mounts are available in Kubernetes since 1.4 [1].
This allows Cilium container to mount the bpf fs automatically
and propagate the mount into the host.

This will improve Cilium's UX as it will remove the requirement of
mounting the BPF fs in the host.

[1] https://kubernetes.io/docs/concepts/storage/volumes/#mount-propagation

Auto-mount bpf file-system from within Cilium DaemonSet and remove the requirement of having it mounted in the host.

@aanm aanm added sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. needs-backport/1.10 labels Jun 25, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.2 Jun 25, 2021
@aanm
Copy link
Member Author

aanm commented Jun 25, 2021

test-me-please

@aanm aanm marked this pull request as ready for review June 25, 2021 21:21
@aanm aanm requested a review from a team June 25, 2021 21:21
@aanm aanm requested review from a team as code owners June 25, 2021 21:21
@aanm aanm requested review from a team, jrfastab, borkmann and qmonnet June 25, 2021 21:21
@aanm aanm requested a review from errordeveloper June 25, 2021 21:21
@aanm
Copy link
Member Author

aanm commented Jun 25, 2021

test-me-please

@aanm
Copy link
Member Author

aanm commented Jun 26, 2021 •
edited

CI was green before pushing the changes proposed by Aditi. No need to re-run the CI since we don't have cri-o coverage:

Click to show.

image

@aanm aanm requested a review from aditighag June 26, 2021 01:26
@aditighag
Copy link
Member

aditighag commented Jun 28, 2021 •
edited

CI was green before pushing the changes proposed by Aditi.

@aanm I'm not sure why you re-requested review without addressing the feedback.

Edit : Looks like there was some confusion -- my review comment applies to all the places that the PR has made the switch from /sys/fs/bpf to /sys/fs.

aditighag
aditighag requested changes Jun 28, 2021
View reviewed changes
Copy link
Member

@aditighag aditighag left a comment

Choose a reason for hiding this comment

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

Feedback still hasn't been addressed.

@aanm aanm marked this pull request as draft June 29, 2021 04:12
@aanm
Copy link
Member Author

aanm commented Jul 28, 2021

test-me-please

@aanm
Copy link
Member Author

aanm commented Aug 2, 2021 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-Runtime-4.9

👍 created #17034

@aanm
Copy link
Member Author

aanm commented Aug 3, 2021 •
edited by maintainer-s-little-helper bot

test-me-please

Job 'Cilium-PR-Runtime-4.9' hit: #17034 (94.13% similarity)

@maintainer-s-little-helper maintainer-s-little-helper bot mentioned this pull request Aug 3, 2021
Closed
@aanm
Copy link
Member Author

aanm commented Aug 3, 2021

Runtime hit #17038

@aanm
Copy link
Member Author

aanm commented Aug 5, 2021

Fixed all flakes, rebased with master and re-triggering the tests

@aanm
Copy link
Member Author

aanm commented Aug 5, 2021 •
edited by maintainer-s-little-helper bot

test-me-please

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

Click to show.

Test Name

K8sDatapathConfig Encapsulation Check connectivity with sockops and VXLAN encapsulation

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.19-kernel-5.4/src/github.com/cilium/cilium/test/k8sT/manifests/l3-policy-demo.yaml: Cannot retrieve cilium pod cilium-zdm75 policy revision: cannot get revision from json: could not parse JSON from command "kubectl exec -n kube-system cilium-zdm75 -- cilium policy get -o json"

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

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

Click to show.

Test Name

K8sFQDNTest Validate that multiple specs are working correctly

Failure Output

FAIL: Cannot install fqdn proxy policy

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

@aanm
Copy link
Member Author

aanm commented Aug 6, 2021 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-K8s-1.19-kernel-5.4

👍 created #17069

@aanm
Copy link
Member Author

aanm commented Aug 6, 2021 •
edited by maintainer-s-little-helper bot

/mlh new-flake Cilium-PR-K8s-1.21-kernel-4.9

👍 created #17071

@aanm
install/kubernetes: use bidirectional mounts to mount bpf fs
b062ca8 
Bidirectional mounts are available in Kubernetes since 1.4 [1].
This allows Cilium container to mount the bpf fs automatically
and propagate the mount into the host.

This will improve Cilium's UX as it will remove the requirement of
mounting the BPF fs in the host.

[1] https://kubernetes.io/docs/concepts/storage/volumes/#mount-propagation

Signed-off-by: André Martins <andre@cilium.io>
@aanm
Copy link
Member Author

aanm commented Aug 6, 2021

test-me-please

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 6, 2021
@aanm aanm merged commit f7a3f59 into cilium:master Aug 7, 2021
@gandro gandro mentioned this pull request Aug 10, 2021
Merged
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.4 Aug 10, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.4 Aug 13, 2021
@joestringer joestringer mentioned this pull request Sep 1, 2021
Merged
@dghubble dghubble mentioned this pull request Sep 21, 2021
Merged
@ReillyBrogan ReillyBrogan mentioned this pull request Sep 23, 2021
Merged
@aditighag aditighag mentioned this pull request Nov 29, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joestringer joestringer joestringer approved these changes

@oblazek oblazek oblazek left review comments

@errordeveloper errordeveloper errordeveloper approved these changes

@borkmann borkmann borkmann approved these changes

@jrfastab jrfastab jrfastab approved these changes

@qmonnet qmonnet qmonnet approved these changes

@aditighag aditighag aditighag approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/minor This PR changes functionality that users may find relevant to operating Cilium. sig/k8s Impacts the kubernetes API, or kubernetes -> cilium internals translation layers.
Projects
No open projects
1.10.4
Backport done to v1.10
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@aanm @aditighag @errordeveloper @borkmann @joestringer @jrfastab @oblazek @qmonnet @gandro @ti-mo