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

Add podman as an option when using kind #415

Closed
wants to merge 17 commits into from

Conversation

giulianisanches
Copy link

Changes

  • 🎁 Instead of using docker hardcoded for some operations, I have added a function that search from docker or podman.

/kind enhancement

Fixes #408

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented May 2, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@knative-prow
Copy link

knative-prow bot commented May 2, 2023

Welcome @giulianisanches! It looks like this is your first PR to knative-sandbox/kn-plugin-quickstart 🎉

@knative-prow knative-prow bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label May 2, 2023
@knative-prow
Copy link

knative-prow bot commented May 2, 2023

Hi @giulianisanches. Thanks for your PR.

I'm waiting for a knative-sandbox member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow knative-prow bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label May 2, 2023
@psschwei
Copy link
Contributor

psschwei commented May 4, 2023

/ok-to-test

@knative-prow knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 4, 2023
@giulianisanches
Copy link
Author

I have opened this PR to get some feedback as I'm not a Go developer :). Feel free to ping me and ask for improvements and fixes :)

Thank you very much.

Copy link
Contributor

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Left a few comments.

You'll also need to sign the CLA before this can be merged.

pkg/kind/kind.go Outdated Show resolved Hide resolved
pkg/kind/kind.go Outdated Show resolved Hide resolved
pkg/kind/kind.go Outdated Show resolved Hide resolved
pkg/kind/kind.go Outdated Show resolved Hide resolved
Copy link
Contributor

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

Looks like podman is running into issues trying to expose port 80...

$ ./kn-quickstart kind
<snip>
ERROR: failed to create cluster: command "podman run --name knative-control-plane --hostname knative-control-plane --label io.x-k8s.kind.role=control-plane --privileged --tmpfs /tmp --tmpfs /run --volume 42da644ee9028ed18a6873b4dce9b6d690ef8284853806bce51b0e48c8533c9f:/var:suid,exec,dev --volume /lib/modules:/lib/modules:ro -e KIND_EXPERIMENTAL_CONTAINERD_SNAPSHOTTER --detach --tty --net kind --label io.x-k8s.kind.cluster=knative -e container=podman --device /dev/fuse --publish=127.0.0.1:80:31080/tcp --publish=127.0.0.1:43605:6443/tcp -e KUBECONFIG=/etc/kubernetes/admin.conf docker.io/kindest/node:v1.25.3" failed with error: exit status 126
Command Output: Error: rootlessport cannot expose privileged port 80, you can add 'net.ipv4.ip_unprivileged_port_start=80' to /etc/sysctl.conf (currently 1024), or choose a larger port number (>= 1024): listen tcp 127.0.0.1:80: bind: permission denied
Error: creating cluster: existing cluster: new cluster: kind create: piping output: exit status 1
<snip>

Changing the port (both here and in the kourier install) leads to another error, when trying to connect to the registry:

$ podman network connect kind kind-registry
Error: "slirp4netns" is not supported: invalid network mode

pkg/kind/kind.go Outdated Show resolved Hide resolved
pkg/kind/kind.go Outdated Show resolved Hide resolved
@giulianisanches
Copy link
Author

Hi, sorry for the lack of activity.

I will get back on this next sunday afternoon.

@knative-prow-robot knative-prow-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 27, 2023
@knative-prow
Copy link

knative-prow bot commented Jun 30, 2023

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: giulianisanches
Once this PR has been reviewed and has the lgtm label, please ask for approval from psschwei. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 30, 2023
This error message is related to when we fail
to connect to local registry
It was not working because it try to combine both `&&` and `||`.

The result command is

`podman rm -f <...> && || true`

When it should be podman

`rm -f <...> && true`
or
`podman rm -f <...> || true`.

Removing this combination allow the error
to be handled when the deleteContainerRegistry
is called.
This is fixed in the upstream, but my rebase didn't got this change.
This error message is related to when we fail
to connect to local registry
It was not working because it try to combine both `&&` and `||`.

The result command is

`podman rm -f <...> && || true`

When it should be podman

`rm -f <...> && true`
or
`podman rm -f <...> || true`.

Removing this combination allow the error
to be handled when the deleteContainerRegistry
is called.
@giulianisanches
Copy link
Author

Again, sorry for the lack of activity.

I have reviewed all the comments and also tried to rebase my code with yours and I hope to have made everything right.

@giulianisanches giulianisanches requested a review from psschwei June 30, 2023 19:12
@psschwei
Copy link
Contributor

psschwei commented Jul 6, 2023

Some of the diffs on your PR seem a bit out of whack... I don't think you should have any changes other than in pkg/kind/kind.go (the rest should already bee in main). I'm going to try closing / reopening to see if that cleans things up...

@psschwei psschwei closed this Jul 6, 2023
@psschwei psschwei reopened this Jul 6, 2023
@psschwei
Copy link
Contributor

psschwei commented Jul 6, 2023

ok, that's better 😄

Copy link
Contributor

@psschwei psschwei left a comment

Choose a reason for hiding this comment

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

I'm still running into the issues from #415 (review)

We need to address the port issue (probably by not using port 80) and that podman network connect is unimplemented for slirp4netns mode as per kubernetes-sigs/kind#2694 (comment)

@giulianisanches
Copy link
Author

Some of the diffs on your PR seem a bit out of whack... I don't think you should have any changes other than in pkg/kind/kind.go (the rest should already bee in main). I'm going to try closing / reopening to see if that cleans things up...

I have merged the changes from upstream:main into my fork. Maybe these are the changes that you're menioning.

@giulianisanches
Copy link
Author

Changing the port to 8080 and running the command './kn-quickstart kind' i get the following output:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ ./kn-quickstart kind
Running Knative Quickstart using Kind
✅ Checking dependencies...
    Kind version is: 0.20.0

☸ Creating Kind cluster...
enabling experimental podman provider
Creating cluster "knative" ...
 ✓ Ensuring node image (kindest/node:v1.25.3) 🖼
 ✓ Preparing nodes 📦  
 ✓ Writing configuration 📜 
 ✓ Starting control-plane 🕹️ 
 ✓ Installing CNI 🔌 
 ✓ Installing StorageClass 💾 
 ✓ Waiting ≤ 2m0s for control-plane = Ready ⏳ 
 • Ready after 19s 💚
Set kubectl context to "kind-knative"
You can now use your cluster with:

kubectl cluster-info --context kind-knative

Thanks for using kind! 😊

Error: creating cluster: existing cluster: local-registry: failed to connect local registry to kind cluster: exit status 125
Usage:
  kn-quickstart kind [flags]

Flags:
  -h, --help                        help for kind
      --install-eventing            install Eventing on quickstart cluster
      --install-serving             install Serving on quickstart cluster
  -k, --kubernetes-version string   kubernetes version to use (1.x.y) or (kindest/node:v1.x.y)
  -n, --name string                 kind cluster name to be used by kn-quickstart (default "knative")

creating cluster: existing cluster: local-registry: failed to connect local registry to kind cluster: exit status 125

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ 

The command podman network connect kind kind-registry also results in:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ podman network connect kind kind-registry
Error: "slirp4netns" is not supported: invalid network mode

Operating system:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ uname -a
Linux fedora 6.3.11-200.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Sun Jul  2 13:17:31 UTC 2023 x86_64 GNU/Linux

@giulianisanches
Copy link
Author

Additional references:

containers/podman#13109
containers/podman#8193

@giulianisanches
Copy link
Author

giulianisanches commented Jul 11, 2023

I have the following containers running on my machine:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ podman ps                                                         
CONTAINER ID  IMAGE                           COMMAND               CREATED         STATUS         PORTS                                               NAMES
112c0abd83f8  docker.io/library/registry:2    /etc/docker/regis...  31 minutes ago  Up 31 minutes  0.0.0.0:5001->5000/tcp                              kind-registry
767ebe3c7106  docker.io/kindest/node:v1.25.3                        31 minutes ago  Up 31 minutes  127.0.0.1:35183->6443/tcp, 0.0.0.0:8080->31080/tcp  knative-control-plane

For the kind-registry container the NetworkMode is:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ podman inspect 112c0abd83f8 --format '{{.HostConfig.NetworkMode}}'
slirp4netns

For the knative-control-plane the NetworkMode is:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ podman inspect 767ebe3c7106 --format '{{.HostConfig.NetworkMode}}'
bridge

Trying to connect to the knative-control-plane shows the following message:

~/dev/src/github.com/giulianisanches/kn-plugin-quickstart main*
❯ podman network connect kind knative-control-plane                 
Error: container 767ebe3c710635e3530737f11aa79e371f81afbfd26f0ffcad58b84628438c84 is already connected to network "kind": network is already connected

I will try to create the kind-registry container with the same NetworkMode (I don't know if the slirp4netns for the kind-registry is intentional).

EDITED: Turns out that when you run podman without root you can only use slirp4netns. For some reason that I still need to investigate, the knative-control-plane is able to start with the bridge NetworkMode

Time to get some sleep :)

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 9, 2023
@giulianisanches
Copy link
Author

sorry about the lack of update, but I will need to put this on a hold for now... a lot of thing going on on my personal life... will revisit this at some point in the future.

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 11, 2023
Copy link

github-actions bot commented Jan 9, 2024

This Pull Request is stale because it has been open for 90 days with
no activity. It will automatically close after 30 more days of
inactivity. Reopen with /reopen. Mark as fresh by adding the
comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 9, 2024
@github-actions github-actions bot closed this Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kn quickstart kind - Error: creating cluster: docker not running
3 participants