podman checkpoint restore --ignore-static-mac does not change mac address

[jelly]

/kind bug

Description

Podman's checkpoint restore does not honour --ignore-static-mac (ignore-static-ip untested)

Steps to reproduce the issue:

podman run -dit --mac-address 92:d0:c6:0a:29:38 --name fo busybox:latest sh

podman container checkpoint fo

podman container restore --ignore-static-mac fo

podman inspect --format '{{.NetworkSettings.MacAddress}}' fo

[root@fedora-37-127-0-0-2-2201 ~]# podman inspect --format '{{.NetworkSettings.MacAddress}}' fo
92:d0:c6:0a:29:38

Describe the results you received:

The mac address stays the same, while reading the man page it gives me the impression that the set mac address should change.

Using --ignore-static-mac tells Podman to ignore the MAC address if it was configured with --mac-address during container creation.

Describe the results you expected:

A changed MAC address

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

Client:       Podman Engine
Version:      4.3.1
API Version:  4.3.1
Go Version:   go1.19.2
Built:        Fri Nov 11 15:01:27 2022
OS/Arch:      linux/amd64

Output of podman info:

host:
  arch: amd64
  buildahVersion: 1.28.0
  cgroupControllers:
  - cpuset
  - cpu
  - io
  - memory
  - hugetlb
  - pids
  - misc
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:                                                                                                                                                           [81/137]
    package: conmon-2.1.4-3.fc37.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.4, commit: '
  cpuUtilization:
    idlePercent: 98.07
    systemPercent: 1.13
    userPercent: 0.8
  cpus: 1
  distribution:
    distribution: fedora
    variant: cloud
    version: "37"
  eventLogger: journald
  hostname: fedora-37-127-0-0-2-2201
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 6.0.9-300.fc37.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 498061312
  memTotal: 1140609024
  networkBackend: netavark
  ociRuntime:
    name: crun
    package: crun-1.7-1.fc37.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.7
      commit: 40d996ea8a827981895ce22886a9bac367f87264
      rundir: /run/user/0/crun
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +LIBKRUN +WASM:wasmedge +YAJL
  os: linux
  remoteSocket:
    exists: true
    path: /run/podman/podman.sock
  security:
    apparmorEnabled: false
    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: true
  serviceIsRemote: false
  slirp4netns:
    executable: /usr/bin/slirp4netns
    package: slirp4netns-1.2.0-8.fc37.x86_64
    version: |-
      slirp4netns version 1.2.0
      commit: 656041d45cfca7a4176f6b7eed9e4fe6c11e8383
      libslirp: 4.7.0
      SLIRP_CONFIG_VERSION_MAX: 4
      libseccomp: 2.5.3
  swapFree: 1139798016
  swapTotal: 1139798016
  uptime: 0h 43m 50.00s
plugins:
  authorization: null
  log:
  - k8s-file
  - none
  - passthrough
  - journald
  network:
  - bridge
  - macvlan
  volume:
  - local
registries:
  search:
  - registry.fedoraproject.org
  - registry.access.redhat.com
  - docker.io
  - quay.io
store:                                                                                                                                                               [0/132]
  configFile: /usr/share/containers/storage.conf
  containerStore:
    number: 3
    paused: 0
    running: 3
    stopped: 0
  graphDriverName: overlay
  graphOptions:
    overlay.mountopt: nodev,metacopy=on
  graphRoot: /var/lib/containers/storage
  graphRootAllocated: 12798898176
  graphRootUsed: 1680064512
  graphStatus:
    Backing Filesystem: btrfs
    Native Overlay Diff: "false"
    Supports d_type: "true"
    Using metacopy: "true"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 3
  runRoot: /run/containers/storage
  volumePath: /var/lib/containers/storage/volumes
version:
  APIVersion: 4.3.1
  Built: 1668178887
  BuiltTime: Fri Nov 11 15:01:27 2022
  GitCommit: ""
  GoVersion: go1.19.2
  Os: linux
  OsArch: linux/amd64
  Version: 4.3.1

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

podman-4.3.1-1.fc37.x86_64

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide?

No, not with the latest Git master

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

Virtual machine/hardware (also reproducible on Arch Linux)

[mheon]

@adrianreber PTAL

[Luap99]

OK, I am pretty sure I broke this with the 4.0 rewrite due to not correctly understanding what the restore behaviour is/was.
This is not surprising given that there is no test for --ignore-static-mac and only a single test for --ignore-static-ip but this test doesn't even check for the ip so it can be considered pointless.

So looking back at the v3 version I think the correct behaviour should be:

1. if --mac-address was set during container creation and --ignore-static-mac is set we have to remove the mac address from the container config
2. same as 1. for --ip/ip6 and --ignore-static-ip
3. when no static ip/mac address was set we should still restore with the ip/mac that was in use during the checkpoint but the mac/ip should not be part of the container config (so a stop/start will use a different ip/mac)
4. if the --name option is used for restore we should not restore with the same ip/mac like 3., only when they were set statically on container creation we use the static ones in which case the user needs to disable it explicitly with --ignore-static-ip/mac

@adrianreber @rst0git Does this sound right?

[adrianreber]

My thoughts are the same. This probably broke with the new network backend and because there are no tests.

I was not involved in that part of the code so that I am not sure it is very important, but maybe someone depends on it. So maybe it should be fixed if possible. At this point I actually do not remember why the switches exist for a IPs and or MAC.

@Luap99 Your list sounds correct.

[Luap99]

I will fix it, I just needed to know if my thoughts are correct.

[Luap99]

Fix in #16800