podman-2.2.0_rc2 no longer allows later parameters to override those prior

[srcshelton]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

In all podman releases up until 2.1.1 (possibly 2.2.0_rc1) It was possible to provide the same parameter multiple times, and the final instance of the parameter would that which was acted upon.

It was therefore possible to define a podman invocation with a whole series of parameters into a string or array, and then run the container in a different way (perhaps with an override to perform a configuration check) simply by appending further values onto the end of the original variable, e.g.

dockeropts=(
        #--log-level debug
        --cap-drop ALL
        --cap-add SETGID
        --cap-add SETUID
        --interactive
        --mount type=bind,source=/var/run/syslog-ng/log,destination=/dev/log
        --mount type=bind,source=/etc/${PN}/,destination=/etc/${PN}/,ro=true
        --mount type=bind,source=/var/lib/${PN}/,destination=/var/lib/${PN}
        --mount type=bind,source=/var/log/${PN}/,destination=/var/log/${PN}
        --mount type=bind,source=/var/run/${PN}/,destination=/var/run/${PN}
        --name "openrc-${PN}-${PV}"

        --network host

        --restart on-failure
        #--rm
        --tty
)
dockerimage="service.${CATEGORY}.${PN}:${PV}"

name="containerised ${PN}"
command='podman'
command_args="--log-level=info run ${dockeropts[@]} ${dockerimage} -c ${CONFFILE}"

...

        ebegin "Checking configuration in '${CONFFILE}'"
        podman run "${dockeropts[@]}" \
                        --cap-drop ALL \
                        --network none \
                        --name "openrc-${PN}-${PV}-checkconfig" \
                        --replace \
                        --restart no \
                        --rm \
                "${dockerimage}" -c "${CONFFILE}" -n || rc=${?}

... but whereas this worked in all prior releases, in 2.2.0_rc2 podman instead outputs:

Error: network conflict between type host and none

On a personal note, this change breaks a whole bunch of my scripts - but in general, it's a significant change in behaviour from previous releases. Whilst it's not impossible to filter strings or arrays to remove duplicates, it's more processing which was not previously required.

If this change is intentional, then could it please at least be relegated to a warning (with the previous behaviour maintained of acting on the last parameter specified) rather than being treated as an error?

Output of podman version:

Version:      2.2.0-rc2
API Version:  2.1.0
Go Version:   go1.15.5
Git Commit:   cbdb4d54bd3dddb8b4452adbfc29ca7702b8e387
Built:        Thu Nov 26 17:49:10 2020
OS/Arch:      linux/amd64

Output of podman info --debug:

host:
  arch: amd64
  buildahVersion: 1.18.0
  cgroupManager: cgroupfs
  cgroupVersion: v2
  conmon:
    package: Unknown
    path: /usr/bin/conmon
    version: 'conmon version 2.0.21, commit: 35a2fa83022e56e18af7e6a865ba5d7165fa2a4a'
  cpus: 8
  distribution:
    distribution: gentoo
    version: unknown
  eventLogger: file
  hostname: dellr330
  idMappings:
    gidmap: null
    uidmap: null
  kernel: 5.9.6-gentoo
  linkmode: dynamic
  memFree: 1143275520
  memTotal: 33392836608
  ociRuntime:
    name: crun
    package: Unknown
    path: /usr/bin/crun
    version: |-
      crun version 0.16
      commit: eb0145e5ad4d8207e84a327248af76663d4e50dd
      spec: 1.0.0
      +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +YAJL
  os: linux
  remoteSocket:
    path: /run/podman/podman.sock
  rootless: false
  slirp4netns:
    executable: ""
    package: ""
    version: ""
  swapFree: 23645081600
  swapTotal: 25769787392
  uptime: 118h 6m 25.98s (Approximately 4.92 days)
registries:
  search:
  - docker.io
  - docker.pkg.github.com
  - quay.io
store:
  configFile: /etc/containers/storage.conf
  containerStore:
    number: 23
    paused: 0
    running: 21
    stopped: 2
  graphDriverName: overlay
  graphOptions:
    overlay.ignore_chown_errors: "false"
  graphRoot: /space/podman/storage
  graphStatus:
    Backing Filesystem: extfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageStore:
    number: 431
  runRoot: /space/podman/run
  volumePath: /space/podman/volumes
version:
  APIVersion: 2.1.0
  Built: 1606412950
  BuiltTime: Thu Nov 26 17:49:10 2020
  GitCommit: cbdb4d54bd3dddb8b4452adbfc29ca7702b8e387
  GoVersion: go1.15.5
  OsArch: linux/amd64
  Version: 2.2.0-rc2

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

Yes

[srcshelton]

Looks as if the breaking change is f441190

[Luap99]

The reason for this change was to provide a better ux when specifying multiple cni networks.

The reason your overwriting works is because that's how the pflag library handles this. I personally don't like this behaviour. It's easy to overwrite the previous value by accident and having no clear indication why it doesn't work as they would like.

I didn't expect somebody to use this intentionally. I don't have strong feelings about keeping f441190 in 2.2 but I would love to have this in 3.0.
The maintainers should decide if they want to keep this change for 2.2, 3.0 or not at all?
cc @baude @mheon @rhatdan

[srcshelton]

As a counter-argument, I'd suggest that the pattern of acting on the most recently specified option is a long-established UNIX tradition, and this is a breaking change, as highlighted above.

As I said, I'm certainly not against parameter re-definition being made a warning, but please don't make it an error (in any release!) as I suspect that the blast-radius of such a change would be wider than you imagine...

[srcshelton]

(Is there anything else which could be done to both improve the UX around using multiple CNI networks without breaking existing CLI behaviour?)

[vrothberg]

It's reverted now with #8514. Thanks for reaching out, @srcshelton !