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

Unable to create pod with specific CNI network #4718

Open
danopia opened this issue Dec 16, 2019 · 15 comments
Open

Unable to create pod with specific CNI network #4718

danopia opened this issue Dec 16, 2019 · 15 comments

Comments

@danopia
Copy link

@danopia danopia commented Dec 16, 2019

/kind feature

Description

Hello, I have created multiple different CNI conflists. I don't seem able to specify which conflist a pod's infra container will use.

~> ls -1 /etc/cni/net.d
87-podman-bridge.conflist
89-demo-bridge.conflist

In this case conflist 87 is the default podman network, and 89 is a custom demo network (10.89.0.0/24) which I intend to use for certain pods. I am able to use --net=demo when creating containers but not when creating pods.

Steps to reproduce the issue:

  1. Create a second CNI with different IP range.

  2. podman create pod - a pod and infra container is created. Note that --net argument is NOT valid here.

  3. Lookup infra container: podman pod inspect -l | grep infraContainerID and check its IP address: podman inspect 97...93 | grep IPAddress

Describe the results you received:
The infra container received an IP address from the first available CNI. In this case, 10.88.0.6 was selected.

Describe the results you expected:
I expected to be able to supply --net=demo at step 2 so that 10.89.0.0/24 would be used for the infra pod.

Additional information you deem important (e.g. issue happens only occasionally):
I attempted to supply --net=demo when adding containers to the pod, figuring maybe the infra container would be moved to that network. However it only resulted in the containers getting their own individual IP addresses from demo. This breaks the shared-localhost concept which is half of why pods are useful to begin with.

Output of podman version:

Version:       1.0.5
Go Version:    go1.11.6
OS/Arch:       linux/amd64

I know this is old!! CentOS Stream 8 doesn't have any way to upgrade. I've checked the latest podman-pod-create.1.md to ensure --network is still not accepted in pod create.

Output of podman info --debug:

debug:
  compiler: gc
  git commit: ""
  go version: go1.11.6
  podman version: 1.0.5
host:
  BuildahVersion: 1.6-dev
  Conmon:
    package: podman-1.0.5-1.gitf604175.module_el8.0.0+194+ac560166.x86_64
    path: /usr/libexec/podman/conmon
    version: 'conmon version 1.14.0-dev, commit: db4132fdf7a7a29546679331f7119a745266f613-dirty'
  Distribution:
    distribution: '"centos"'
    version: "8"
  MemFree: 32315129856
  MemTotal: 33440043008
  OCIRuntime:
    package: runc-1.0.0-55.rc5.dev.git2abd837.module_el8.0.0+58+91b614e7.x86_64
    path: /usr/bin/runc
    version: 'runc version spec: 1.0.0'
  SwapFree: 0
  SwapTotal: 0
  arch: amd64
  cpus: 4
  hostname: ausbox
  kernel: 4.18.0-147.6.el8.x86_64
  os: linux
  rootless: false
  uptime: 1h 4m 25.23s (Approximately 0.04 days)
insecure registries:
  registries: []
registries:
  registries:
  - registry.redhat.io
  - quay.io
  - docker.io
store:
  ConfigFile: /etc/containers/storage.conf
  ContainerStore:
    number: 4
  GraphDriverName: overlay
  GraphOptions: null
  GraphRoot: /var/lib/containers/storage
  GraphStatus:
    Backing Filesystem: xfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  ImageStore:
    number: 2
  RunRoot: /var/run/containers/storage

Package info (e.g. output of rpm -q podman or apt list podman):

podman-1.0.5-1.gitf604175.module_el8.0.0+194+ac560166.x86_64

Additional environment details (AWS, VirtualBox, physical, etc.):
Physical machine, blah blah.

My intention is to create a CNI network inside of a WireGuard VPN space so that individual pods can be issued VPN-routable IP addresses.

@danopia

This comment has been minimized.

Copy link
Author

@danopia danopia commented Dec 16, 2019

Since github's jump-to-definition is working these days I poked around a bit, looks like the desync is that creating an infra container always uses a zero-length network list:

// Since user namespace sharing is not implemented, we only need to check if it's rootless
networks := make([]string, 0)
netmode := "bridge"
if isRootless {
netmode = "slirp4netns"
}
// PostConfigureNetNS should not be set since user namespace sharing is not implemented
// and rootless networking no longer supports post configuration setup
options = append(options, WithNetNS(p.config.InfraContainer.PortBindings, false, netmode, networks))

While creating a normal container supports adding one or more userNetworks to the list:

userNetworks := c.NetMode.UserDefined()
networks := make([]string, 0)
if IsPod(userNetworks) {
userNetworks = ""
}
if userNetworks != "" {
for _, netName := range strings.Split(userNetworks, ",") {
if netName == "" {
return nil, errors.Errorf("container networks %q invalid", userNetworks)
}
networks = append(networks, netName)
}
}
if c.NetMode.IsNS() {
ns := c.NetMode.NS()
if ns == "" {
return nil, errors.Errorf("invalid empty user-defined network namespace")
}
_, err := os.Stat(ns)
if err != nil {
return nil, err
}
} else if c.NetMode.IsContainer() {
connectedCtr, err := runtime.LookupContainer(c.NetMode.Container())
if err != nil {
return nil, errors.Wrapf(err, "container %q not found", c.NetMode.Container())
}
options = append(options, libpod.WithNetNSFrom(connectedCtr))
} else if !c.NetMode.IsHost() && !c.NetMode.IsNone() {
postConfigureNetNS := userns.getPostConfigureNetNS()
options = append(options, libpod.WithNetNS(portBindings, postConfigureNetNS, string(c.NetMode), networks))
}

@mheon

This comment has been minimized.

Copy link
Collaborator

@mheon mheon commented Dec 16, 2019

Yeah, this just needs to be wired up in the command line, though @haircommander and I were discussing moving infra container creation logic such that we wouldn't also need to pass all infra config through the pod configuration, which would make it easier to allow more container configuration options for pods.

@danopia

This comment has been minimized.

Copy link
Author

@danopia danopia commented Dec 16, 2019

That would be a good refactor, I don't really want to submit a PR that just copy/pastes the split code..

Would it be more practical, short term at least, to defer the infra container creation, and allow following up with some sort of container create --pod=... --as-infra ... command so the full option set is available?

@mheon

This comment has been minimized.

Copy link
Collaborator

@mheon mheon commented Dec 16, 2019

I think that would be an end-goal of our refactor - allowing podman pod create without an infra container and then podman create --as-infra on that pod.

@danopia

This comment has been minimized.

Copy link
Author

@danopia danopia commented Dec 16, 2019

Ah, I initially read it as somehow inheriting all the create flags in pod create and internally handing off the infra handling.

In the case of allowing a followup command, I'm not sure what actual refactoring is needed. Given that the infra pod is already opt-out-able at least. Sounds like creating an --as-infra flag on create would do it, as long as it invokes a copy of this code:

pod.state.InfraContainerID = ctr.ID()
if err := pod.save(); err != nil {
return nil, err
}

With no further patches, it would definitely be up the caller to properly configure that pause container (image, name, readonly fs, etc) via the existing create flags. Is there some other aspect I'm not seeing? I'm new to podman, but not golang :p

@mheon

This comment has been minimized.

Copy link
Collaborator

@mheon mheon commented Dec 16, 2019

We'd need the ability to set pod.state.InfraContainerID (presently it's not really accessible), but only once - I don't expect that we'll allow infra containers to be overwritten once set, at least at first. withInfraContainer() will need to be made public, and wired into the command line for container creation. It might also be necessary to make some changes to pod creation to allow the pod namespace bools (UsePodPID, UsePodIPC et al) to be set if an infra container was not created with the pod (so we still try and share namespaces when the infra container is eventually created).

@danopia

This comment has been minimized.

Copy link
Author

@danopia danopia commented Dec 17, 2019

I went ahead with patching a --network flag into pod create in #4719 to fulfill the original request. It works for my needs locally and maybe makes sense to upstream too.

Given the work described to properly allow full flexibility with infra containers, and the fact that I only have pretty crappy podman environments available I don't think I'd manage a proper attempt at that larger refactor before EOY, if I'm even the right person to attempt it.

danopia added a commit to danopia/libpod that referenced this issue Dec 17, 2019
danopia added a commit to danopia/libpod that referenced this issue Dec 17, 2019
Closes containers#4718

Signed-off-by: Daniel Lamando <dan@danopia.net>
danopia added a commit to danopia/libpod that referenced this issue Dec 17, 2019
Closes containers#4718

Signed-off-by: Daniel Lamando <dan@danopia.net>
@github-actions

This comment has been minimized.

Copy link

@github-actions github-actions bot commented Jan 17, 2020

A friendly reminder that this issue had no activity for 30 days.

@gima

This comment has been minimized.

Copy link

@gima gima commented Jan 22, 2020

Why does the infra container need a network connectivity at all?
Given that anything can be in the infra container image, I'd say it's a security hole waiting to happen.

Currently the infra container's networking is hard-coded to "bridge": runtime_pod_infra_linux.go#L98.

Couldn't it simply be "none"?

Hmm. Maybe this comment should've been over there: #4908 Infra containers receive IP addresses implicitly from unused network

@danopia

This comment has been minimized.

Copy link
Author

@danopia danopia commented Jan 22, 2020

@gima Actually, the purpose of the infra pod is to hold one consistent namespaces including networking, so that the pod's containers can depend on the network etc. namespace existing at all times and automatically reuse them.

From #4908

In case the infra images do need an IP then why don't they got it from the same network a container in that pod was attached to?

That usecase sounds backwards to how a pod is supposed to work - you'd want to set up the pod's intended network profile on the pod itself (aka the infra container), and then whatever containers you add to the pod will automatically inherit that networking, IP address, etc. If you specify networking when adding a container to a pod, you are opting-out of some of the intended functionality of pods - in particular, containers in a pod are intended to have a common 'localhost' by default.

This ticket #4718 is to track the lack of pod networking options. If there were more options, like 'none' or 'this other specific network' then #4908 might not be a problem to begin with - you'd set the network when creating the pod, and stop specifying network options for containers within the pod.

@gima

This comment has been minimized.

Copy link

@gima gima commented Jan 22, 2020

Ahhh, I missed a thing.
Apparently it's not possible to specify which network(CNI netconf file) to use for the containers in the pod when using podman play kube command. It's just hard-coded to "bridge" in libpod/runtime_pod_infra_linux.go#L98.

@rhatdan

This comment has been minimized.

Copy link
Member

@rhatdan rhatdan commented Jan 24, 2020

If someone were to open a PR on this, I am sure we would consider it.

@mheon

This comment has been minimized.

Copy link
Collaborator

@mheon mheon commented Jan 24, 2020

@rhatdan

This comment has been minimized.

Copy link
Member

@rhatdan rhatdan commented Feb 17, 2020

@mheon Any progress?

mheon added a commit to mheon/libpod that referenced this issue Feb 17, 2020
Enables most of the network-related functionality from
`podman run` in `podman pod create`. Custom CNI networks can be
specified, host networking is supported, DNS options can be
configured.

Fixes containers#4718
Fixes containers#4770

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
@mheon

This comment has been minimized.

Copy link
Collaborator

@mheon mheon commented Feb 17, 2020

#5241 to fix - a lot of other options got mixed in to the PR.

mheon added a commit to mheon/libpod that referenced this issue Feb 17, 2020
Enables most of the network-related functionality from
`podman run` in `podman pod create`. Custom CNI networks can be
specified, host networking is supported, DNS options can be
configured.

Fixes containers#3837
Fixes containers#4718
Fixes containers#4770

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.