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

Fixes the iptables option for older k8s #4774

Merged
merged 2 commits into from Jun 2, 2023
Merged

Fixes the iptables option for older k8s #4774

merged 2 commits into from Jun 2, 2023

Conversation

Nino-K
Copy link
Member

@Nino-K Nino-K commented May 26, 2023

  • The previous logic to enable k8s and iptables didn't seem to be correct. The new changes relies on the actual config to determine enabling k8s.

Fixes: #4730
Fixes: #4682

@Nino-K Nino-K requested a review from jandubois May 26, 2023 16:27
@Nino-K Nino-K requested review from mook-as and removed request for jandubois May 31, 2023 17:38
- The previous logic to enable k8s and iptables
didn't seem to be correct. The new changes relies
on the actual config to determine enabling k8s.

Signed-off-by: Nino Kodabande <nkodabande@suse.com>
pkg/rancher-desktop/backend/wsl.ts Outdated Show resolved Hide resolved
pkg/rancher-desktop/backend/wsl.ts Outdated Show resolved Hide resolved
Signed-off-by: Nino Kodabande <nkodabande@suse.com>
@Nino-K Nino-K requested a review from mook-as June 1, 2023 17:07
Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

It looks like we still need -iptables when using nerdctl.

Tested with:

  • container engine: containerd
  • Kubernetes version: v1.26.5
  • Old networking

Created a Kubernetes container:

  • kubectl run nginx --image nginx:stable --port 80 --expose
  • kubectl edit svc nginx and change it to use NodePort
  • curl, that worked.

Created a containerd container:

  • nerdctl run -d --restart=always --publish 8111:80 nginx:stable
  • curl did not work.

I believe we still need to turn on iptables if we're using the containerd back end. I think I made a table in some random comment…

I'm probably thinking of #2593 (comment) or something like that.

Copy link
Contributor

@mook-as mook-as left a comment

Choose a reason for hiding this comment

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

Looking at it more, I believe the fact that containerd is busted is a separate issue, and this PR is fine as it is.

However, we may need to fix up the table at https://github.com/rancher-sandbox/rancher-desktop-agent/blob/4616f600c9a58785ef52933b5e975086ccb258a8/cmd/rancher-desktop-guestagent/main.go#L60-L74 once we figure everything out to better describe the correct flags.

@Nino-K Nino-K merged commit 8bd0481 into main Jun 2, 2023
9 checks passed
@Nino-K Nino-K deleted the fix-enable-kubernetes branch June 2, 2023 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants