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

cmd/cleanup: add socketlb program cleanup #25136

Merged
merged 1 commit into from May 10, 2023
Merged

cmd/cleanup: add socketlb program cleanup #25136

merged 1 commit into from May 10, 2023

Conversation

rgo3
Copy link
Contributor

@rgo3 rgo3 commented Apr 26, 2023

This leverages this newly added socketlb package API [1] to remove any leftover socketlb bpf programs.

Fixes: #10067

@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 26, 2023
@rgo3 rgo3 added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Apr 26, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Apr 26, 2023
@rgo3 rgo3 marked this pull request as ready for review May 3, 2023 09:36
@rgo3 rgo3 requested a review from a team as a code owner May 3, 2023 09:36
@rgo3 rgo3 requested a review from joamaki May 3, 2023 09:36
@rgo3
Copy link
Contributor Author

rgo3 commented May 3, 2023

Marking as ready for review now that I have manually tested this with a minikube setup and successfully removed socketlb progs from a node by running cilium cleanup.

@rgo3
Copy link
Contributor Author

rgo3 commented May 3, 2023 •
edited by maintainer-s-little-helper bot

/test

Job 'Cilium-PR-K8s-1.16-kernel-4.19' failed:

Click to show.

Test Name

K8sAgentPolicyTest Multi-node policy test with L7 policy using connectivity-check to check datapath

Failure Output

FAIL: cannot install connectivity-check

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.16-kernel-4.19/1251/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.16-kernel-4.19 so I can create one.

Then please upload the Jenkins artifacts to that issue.

Job 'Cilium-PR-K8s-1.26-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathServicesTest Checks N/S loadbalancing With host policy Tests NodePort

Failure Output

FAIL: Can not connect to service "tftp://192.168.56.11:30096/hello" from outside cluster (1/10)

Jenkins URL: https://jenkins.cilium.io/job/Cilium-PR-K8s-1.26-kernel-net-next/2049/

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.26-kernel-net-next so I can create one.

Then please upload the Jenkins artifacts to that issue.

asauber
asauber approved these changes May 4, 2023
View reviewed changes
Copy link
Member

@asauber asauber left a comment

Choose a reason for hiding this comment

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

LGTM

lmb
lmb reviewed May 4, 2023
View reviewed changes
cilium/cmd/cleanup.go Outdated
@@ -249,10 +251,14 @@ func (c ciliumCleanup) cleanupFuncs() []cleanupFunc {
cleanupTCFilters := func() error {
return removeTCFilters(c.tcFilters)
}
cleanupSocketLBPrograms := func() error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: removeSocketLBPrograms is already func() error so the closure isn't really necessary?

@rgo3
cmd/cleanup: add socketlb program cleanup
4893f45 
Fixes: cilium#10067

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
@rgo3
Copy link
Contributor Author

rgo3 commented May 8, 2023

/test

@rgo3
Copy link
Contributor Author

rgo3 commented May 8, 2023

ci-mulitcluster failed with #25064, rerunning.

@ti-mo ti-mo merged commit 6f6296f into cilium:main May 10, 2023
57 checks passed
@ti-mo ti-mo deleted the socketlb-cleanup branch May 10, 2023 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmb lmb lmb approved these changes

@asauber asauber asauber approved these changes

@joamaki joamaki Awaiting requested review from joamaki joamaki was automatically assigned from cilium/cli

Assignees
No one assigned
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove attached bpf_sock.o when doing cilium cleanup
4 participants
@rgo3 @lmb @asauber @ti-mo