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

[Enhancement] Improved Podman compatibility #868

Merged
merged 6 commits into from
Dec 4, 2021

Conversation

serverwentdown
Copy link
Contributor

@serverwentdown serverwentdown commented Nov 20, 2021

What

  1. Removes unnecessary call to ContainerExecStart
    • The call seems unnecessary because a container should either be started or attached, according to Docker CLI source code.
    • Podman flags start and then attach as an invalid state transition and fails.
  2. Adds a --default-network option to registry create for Podman users & special cases

Why

  • Some users might like to use k3d with Podman (See [FEATURE] Podman support #84)
  • This PR provides some fixes and workarounds for compatibility with Podman

Implications

None

Docs

I'm working on docs, but they can only be merged when containers/podman#12328 is released.

@all-contributors please add @serverwentdown for code

This might be useful when a user wants to use a different network to run
the registry, especially for Podman users.

This patch avoids adding such a flag to clusterCreate to avoid polluting
the arguments for that command. A better long-term solution would be to
create a new network for the registry.
@iwilltry42 iwilltry42 self-assigned this Dec 3, 2021
@iwilltry42 iwilltry42 added the enhancement New feature or request label Dec 3, 2021
@iwilltry42 iwilltry42 added this to the v5.3.0 milestone Dec 3, 2021
cmd/registry/registryCreate.go Outdated Show resolved Hide resolved
cmd/registry/registryCreate.go Outdated Show resolved Hide resolved
cmd/registry/registryCreate.go Outdated Show resolved Hide resolved
pkg/client/cluster.go Outdated Show resolved Hide resolved
pkg/client/registry.go Outdated Show resolved Hide resolved
pkg/client/registry.go Outdated Show resolved Hide resolved
pkg/types/defaults.go Outdated Show resolved Hide resolved
pkg/types/registry.go Outdated Show resolved Hide resolved
- network is not ambiguous for registry, so it can just be network instead of DefaultNetwork
- DefaultNetwork as a type however could be ambiguous in that it could be confused for k3d's default network (i.e. `k3d cluster create` without a name would create the "default" `k3d-k3s-default` network)
@iwilltry42 iwilltry42 changed the title feat: Weak Podman compatibility [Enhancement] Improved Podman compatibility Dec 4, 2021
@iwilltry42 iwilltry42 merged commit 858c314 into k3d-io:main Dec 4, 2021
@iwilltry42
Copy link
Member

Oh, I never actually wrote a comment 😁
Hi @serverwentdown , thanks a lot for this PR! 🙏
I hope we can make it to 100% Podman support soon.

@iwilltry42
Copy link
Member

iwilltry42 commented Dec 4, 2021

I just gave it a try:

  • PopOS 21.10 (beta)
  • Podman 3.4.2 installed via Kubic Repo
  • Podman API Service started via podman system service --time=0 unix:///var/run/docker.sock (I chowned it for simplicity)
  • Used k3d v5.2.0 to create a cluster, which works up to the point of parsing the network information from containers as per your original comment

This is great! 😃

@iwilltry42
Copy link
Member

@all-contributors please add @serverwentdown for code

@allcontributors
Copy link
Contributor

@iwilltry42

I've put up a pull request to add @serverwentdown! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants