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

Podman support beta #674

Merged
merged 33 commits into from
Jan 24, 2022
Merged

Podman support beta #674

merged 33 commits into from
Jan 24, 2022

Conversation

LimeHat
Copy link
Member

@LimeHat LimeHat commented Oct 29, 2021

As discussed. Added build tags to avoid breaking things

@hellt
Copy link
Member

hellt commented Nov 17, 2021

in #679 we have added support for CPU quota and mem limits

@LimeHat
Copy link
Member Author

LimeHat commented Jan 5, 2022

All right, finally managed to get back to this.

For build process to work without cgo and extra dependencies need to add a few build tags

podman exclude_graphdriver_btrfs btrfs_noversion exclude_graphdriver_devicemapper exclude_graphdriver_overlay containers_image_openpgp
E.g.:
sergey@clab-1:~/clab-podman/containerlab$ CGO_ENABLED=0 go build -trimpath -tags "podman exclude_graphdriver_btrfs btrfs_noversion exclude_graphdriver_devicemapper exclude_graphdriver_overlay containers_image_openpgp"

Will add #679 soon and do more checks to see how it behaves with the latest main.

@LimeHat LimeHat marked this pull request as ready for review January 6, 2022 17:27
@LimeHat
Copy link
Member Author

LimeHat commented Jan 6, 2022

Looks decent for a beta. Still need to add ipv6/dual-stack for mgmt addressing (prob. will do this separately); update docs & the build pipeline.
@hellt take a look when you're back. thx.

@LimeHat LimeHat changed the title Podman support alpha Podman support beta Jan 6, 2022
@LimeHat LimeHat linked an issue Jan 6, 2022 that may be closed by this pull request
Comment on lines +1 to +11
//go:build linux && podman
// +build linux,podman

package all

import (
_ "github.com/srl-labs/containerlab/runtime/containerd"
_ "github.com/srl-labs/containerlab/runtime/docker"
_ "github.com/srl-labs/containerlab/runtime/ignite"
_ "github.com/srl-labs/containerlab/runtime/podman"
)
Copy link
Member

Choose a reason for hiding this comment

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

do we still need build tags to have a build with podman, considering that you managed to exclude cgo dependency?

Copy link
Member Author

@LimeHat LimeHat Jan 13, 2022

Choose a reason for hiding this comment

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

In principle - we don't; but the build process would require the presence of the other tags i mentioned earlier:
exclude_graphdriver_btrfs btrfs_noversion exclude_graphdriver_devicemapper exclude_graphdriver_overlay containers_image_openpgp

In the current form this is a safety first approach: in case of no tags go build will succeed without podman (based on runtime/all/all.go); otherwise we expect to see all the tags (I even contemplated to include them all).

It's a little bit ugly either way. 😞
I suspect this approach might be more straightforward for external contributors..

Copy link
Member

@hellt hellt Jan 14, 2022

Choose a reason for hiding this comment

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

yes, that is what I thought, but once this PR is done, I think it would be best to include podman support to the GA build of containerlab (ofc we will include those tags to a regular build pipeline delivered by goreleaser)

That way it would be possible to let users with podman to play with it, without fetching separate binary. At this point, the binary increase that podman will incur won't matter much I guess.

we can keep the all and all_w_podman files for the time being

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be best to include podman support to the GA build of containerlab

I mean, that is the plan;
the question is whether we want to keep an option to build clab without podman support for other contributors?

@hellt
Copy link
Member

hellt commented Jan 14, 2022

Do you know which build tag is missing in my trial to build the tip of that branch with the following cmd on ubuntu 20.04

CGO_ENABLED=0 go build -o $(pwd)/bin/containerlab -ldflags="-s -w -X 'github.com/srl-labs/containerlab/cmd.version=0.0.0' -X 'github.com/srl-labs/containerlab/cmd.commit=$(git rev-parse --short HEAD)' -X 'github.com/srl-labs/containerlab/cmd.date=$(date)'" -trimpath -tags "podman exclude_graphdriver_btrfs btrfs_noversion exclude_graphdriver_devicemapper exclude_graphdriver_overlay containers_image_openpgp" main.go
# github.com/containers/image/v5/manifest
../go/pkg/mod/github.com/containers/image/[email protected]/manifest/oci.go:44:234: undefined: v1.MediaTypeImageLayerNonDistributableZstd
../go/pkg/mod/github.com/containers/image/[email protected]/manifest/oci.go:44: undefined: v1.MediaTypeImageLayerZstd
../go/pkg/mod/github.com/containers/image/[email protected]/manifest/oci.go:105:39: undefined: v1.MediaTypeImageLayerNonDistributableZstd
../go/pkg/mod/github.com/containers/image/[email protected]/manifest/oci.go:110:39: undefined: v1.MediaTypeImageLayerZstd
make: *** [Makefile:13: build-with-podman] Error 2

I've asked the same q here - containers/podman#6866 (comment)

@LimeHat
Copy link
Member Author

LimeHat commented Jan 14, 2022

Do you know which build tag is missing in my trial to build the tip of that branch with the following cmd on ubuntu 20.04

Nothing to do with the tags; just a regular dependency management pain :)
I did this commit for a reason, but the last merge undid it

@hellt
Copy link
Member

hellt commented Jan 14, 2022

ouch, I wonder how did you find it out? My google foo led to me to similar reports without a solution

@LimeHat
Copy link
Member Author

LimeHat commented Jan 14, 2022

Do you know which build tag is missing in my trial to build the tip of that branch with the following cmd on ubuntu 20.04

Nothing to do with the tags; just a regular dependency management pain :) I did this commit for a reason, but the last merge undid it

Looks like the new containerd version introduced dependency on the regular v1.0.2 of image-spec; so poor go mod cannot resolve this anymore. Duh.

@LimeHat
Copy link
Member Author

LimeHat commented Jan 14, 2022

@hellt can you try this one. bumped to non-released v1.0.3, that should do it.

runtime/podman/portmaps.go Outdated Show resolved Hide resolved
runtime/podman/portmaps.go Outdated Show resolved Hide resolved
runtime/podman/util.go Outdated Show resolved Hide resolved
@hellt
Copy link
Member

hellt commented Jan 17, 2022

I have a weird issue when I try this branch with podman installed on ubuntu 20.04
it looks like the issue with the missing podman.socket file

❯ podman info
host:
  arch: amd64
  buildahVersion: 1.23.1
  cgroupControllers:
  - cpuset
  - cpu
  - cpuacct
  - blkio
  - memory
  - devices
  - freezer
  - net_cls
  - perf_event
  - net_prio
  - hugetlb
  - pids
  - rdma
  cgroupManager: systemd
  cgroupVersion: v1
  conmon:
    package: 'conmon: /usr/libexec/podman/conmon'
    path: /usr/libexec/podman/conmon
    version: 'conmon version 2.0.30, commit: '
  cpus: 8
  distribution:
    codename: focal
    distribution: ubuntu
    version: "20.04"
  eventLogger: journald
  hostname: devbox-u20
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.4.60-uksm
  linkmode: dynamic
  logDriver: journald
  memFree: 13552488448
  memTotal: 25221267456
  ociRuntime:
    name: crun
    package: 'crun: /usr/bin/crun'
    path: /usr/bin/crun
    version: |-
      crun version UNKNOWN
      commit: ea1fe3938eefa14eb707f1d22adff4db670645d6
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +YAJL
  os: linux
  remoteSocket:
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: true
    capabilities: CAP_CHOWN,CAP_DAC_OVERRIDE,CAP_FOWNER,CAP_FSETID,CAP_KILL,CAP_NET_BIND_SERVICE,CAP_SETFCAP,CAP_SETGID,CAP_SETPCAP,CAP_SETUID,CAP_SYS_CHROOT
    rootless: false
    seccompEnabled: true
    seccompProfilePath: /usr/share/containers/seccomp.json
    selinuxEnabled: false
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: 'slirp4netns: /usr/bin/slirp4netns'
    version: |-
      slirp4netns version 1.1.8
      commit: unknown
      libslirp: 4.3.1-git
      SLIRP_CONFIG_VERSION_MAX: 3
      libseccomp: 2.4.3
  swapFree: 0
  swapTotal: 0
  uptime: 641h 0m 46.64s (Approximately 26.71 days)
plugins:
  log:
  - k8s-file
  - none
  - journald
  network:
  - bridge
  - macvlan
  volume:
  - local
registries:
  search:
  - docker.io
  - quay.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: overlay
  graphOptions:
    overlay.mountopt: nodev,metacopy=on
  graphRoot: /var/lib/containers/storage
  graphStatus:
    Backing Filesystem: extfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "true"
  imageStore:
    number: 0
  runRoot: /run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 3.4.2
  Built: 0
  BuiltTime: Thu Jan  1 01:00:00 1970
  GitCommit: ""
  GoVersion: go1.16.6
  OsArch: linux/amd64
  Version: 3.4.2

podman cli works:

❯ podman ps -a
CONTAINER ID  IMAGE       COMMAND     CREATED     STATUS      PORTS       NAMES

which makes me think that it talks to a socket?

But the clab fails:

❯ dclab dep -t srl.clab.yml --runtime podman
INFO[0000] Containerlab v0.0.0 started                  
INFO[0000] Parsing & checking topology file: srl.clab.yml 
Error: could not list containers: unable to connect to Podman socket: Get "http://d/v3.4.4/libpod/_ping": dial unix /run//podman/podman.sock: connect: no such file or directory

and there is no socket file by that path

❯ ls -la /run/podman/podman.socket
ls: cannot access '/run/podman/podman.socket': No such file or directory

@LimeHat
Copy link
Member Author

LimeHat commented Jan 17, 2022

Right;
podman API depends on the systemd service so you need to enable or start it first (sudo systemctl start podman). Sorry I haven't documented this yet.

@hellt
Copy link
Member

hellt commented Jan 17, 2022

Thanks @LimeHat, that is a solid chunk of work!

I have tuned the test pipeline a bit to accommodate for podman smoke tests

Currently I excluded the following tests, please check if my understanding is correct as to why these tests can not be currently executed with the podman runtime

1 Matching IPv6 address in clab ins -a output

This test is excluded as podman network is created with ipv4 address only.
Is IPv6 just not implemented in this PR or is there any blocker that prevents creating a backing linux bridge with ipv6 address?

2 Verify static ipv6 mgmt assignment

This test is skipped for the reason (1)

3 Host entries verification

Skipped for IPv6, since no IPv6 entries are added because of (1)

4 verification that management network can use an existing bridge

In this test the failure seems to be related to the fact that with podman runtime we can't use the custom name for the podman bridge?
Here is the error that I got for this test case

Verify management network is using user-specified bridge              | FAIL |
'278: veth46c36a53@if3: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue master cni-podman1 state UP mode DEFAULT group default ' does not contain 'master 01-03-mgmt state UP'

the expectation of this test case is that the management veth pairs will have one end in the user defined bridge (named 01-03-mgmt in that test case) to ensure that the existing bridge is used to connect management interfaces.

5 custom bridge name test suite

This suite is skipped, as it tests the same as (4) it seems :D

6 check if --keep-mgmt-name works

this suite is skipped because destroy -t ${topo} --keep-mgmt-net have no effect, and the reason is that setting bridge name for podman to a certain name is not respected by the runtime.

@LimeHat
Copy link
Member Author

LimeHat commented Jan 18, 2022

Hi Roman, thanks for the thorough review & test efforts! 👍

So for 1-3), AFAIR, I encountered a limitation of the current API design where we cannot supply 2 distinct subnets to the network create call. So v6 itself it not an issue (I could've implemented it as either v4 or v6), but dual-stack is challenging. There are potential ways to workaround this (e.g. switch to a lower-level API or just implement custom cni cfg file logic); I just haven't spend enough time on this and planned to implement this later.
Now, there's a podman 4.0 release on the horizon (rc1 is out), which introduces some compatibility-breaking API changes (😩), but I believe they reworked this part so a slice of subnets will be accepted. I need to look at it in more detail, but that's another option.

For 4-6 - right.. I see I left a TODO item in the code on this 😄 let me check if i can accommodate custom bridge naming without too much hassle, I think I may be able to.
Since podman uses CNI-based networking suite, there's a bit of a distinction between a "network name" and an actual network interface naming in the underlying OS (and in the case of bridges, they don't inherit the former by default).
As a matter of fact, p.6 was kind of implemented (but based on the network name instead of bridge name).

Anyway, all the good points, and thanks for jogging my memory on the bridge naming thing. I'll look into it and get back to you soon.

@hellt
Copy link
Member

hellt commented Jan 24, 2022

Thanks @LimeHat
I am merging this, the future improvements will follow in subsequent PRs to keep them short and sweet

@hellt hellt merged commit db5847d into srl-labs:main Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add podman support
2 participants