play kube: cannot restart pod with named volumes

[mhjacks]

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

/kind bug

Description

I am deploying the following YAML file via play kube:

---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: megamek-latest-savegames
  namespace: megamek-server-latest
spec:
  accessModes:
    - ReadWriteOnce
  storageClassName: local-path
  resources:
    requests:
      storage: 2Gi
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: megamek-latest-logs
  namespace: megamek-server-latest
spec:
  accessModes:
    - ReadWriteOnce
  storageClassName: local-path
  resources:
    requests:
      storage: 2Gi
---
apiVersion: v1
kind: PersistentVolumeClaim
metadata:
  name: megamek-latest-mmconf
  namespace: megamek-server-latest
spec:
  accessModes:
    - ReadWriteOnce
  storageClassName: local-path
  resources:
    requests:
      storage: 2Gi
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: megamek-server-latest
  namespace: megamek-server-latest
  labels:
    app: megamek-server-latest
spec:
  replicas: 1
  selector:
    matchLabels:
      app: megamek-server-latest
  template:
    metadata:
      labels:
        app: megamek-server-latest
    spec:
      volumes:
        - name: megamek-latest-savegames
          persistentVolumeClaim:
            claimName: megamek-latest-savegames
        - name: megamek-latest-mmconf
          persistentVolumeClaim:
            claimName: megamek-latest-mmconf
        - name: megamek-latest-logs
          persistentVolumeClaim:
            claimName: megamek-latest-logs
      containers:
        - name: megamek-server-latest
          image: quay.io/martjack/megamek-server:0.49.10
          ports:
            - containerPort: 2346
              name: megamek
          volumeMounts:
            - mountPath: "/home/jboss/megamek/savegames"
              name: megamek-latest-savegames
            - mountPath: "/home/jboss/megamek/logs"
              name: megamek-latest-logs
            - mountPath: "/home/jboss/megamek/mmconf"
              name: megamek-latest-mmconf

Steps to reproduce the issue:

1. podman play kube deploy-megamek.yaml

2. podman play kube deploy-megamek.yaml --down

3. podman play kube deploy-megamek.yaml

Describe the results you received:

In step 3, an error like the following occurs:

podman kube play deploy-megamek.yaml 
Error: volume with name megamek-latest-mmconf already exists: volume already exists

Describe the results you expected:

I expected the pod to re-start. It seems important that, for example, when re-starting a node running services via the systemd integration, that pods with volumes should be able to shutdown and restart gracefully

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

Output of podman version:

Client:       Podman Engine
Version:      4.3.0
API Version:  4.3.0
Go Version:   go1.19.2
Built:        Fri Oct 21 03:09:51 2022
OS/Arch:      linux/amd64

Output of podman info:

host:
  arch: amd64
  buildahVersion: 1.28.0
  cgroupControllers:
  - memory
  - pids
  cgroupManager: systemd
  cgroupVersion: v2
  conmon:
    package: conmon-2.1.4-3.fc37.x86_64
    path: /usr/bin/conmon
    version: 'conmon version 2.1.4, commit: '
  cpuUtilization:
    idlePercent: 99.15
    systemPercent: 0.33
    userPercent: 0.52
  cpus: 2
  distribution:
    distribution: fedora
    version: "37"
  eventLogger: journald
  hostname: srv-f-podman.imladris.lan
  idMappings:
    gidmap:
    - container_id: 0
      host_id: 344800018
      size: 1
    - container_id: 1
      host_id: 2147549184
      size: 65536
    uidmap:
    - container_id: 0
      host_id: 344800018
      size: 1
    - container_id: 1
      host_id: 2147549184
      size: 65536
  kernel: 6.0.5-300.fc37.x86_64
  linkmode: dynamic
  logDriver: journald
  memFree: 15353282560
  memTotal: 16772775936
  networkBackend: netavark
  ociRuntime:
    name: crun
    package: crun-1.6-2.fc37.x86_64
    path: /usr/bin/crun
    version: |-
      crun version 1.6
      commit: 18cf2efbb8feb2b2f20e316520e0fd0b6c41ef4d
      spec: 1.0.0
      +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +CRIU +LIBKRUN +YAJL
  os: linux
  remoteSocket:
    path: /run/user/344800018/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: true
    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: 8589930496
  swapTotal: 8589930496
  uptime: 2h 25m 41.00s (Approximately 0.08 days)
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:
  configFile: /home/podman/.config/containers/storage.conf
  containerStore:
    number: 0
    paused: 0
    running: 0
    stopped: 0
  graphDriverName: overlay
  graphOptions: {}
  graphRoot: /home/podman/.local/share/containers/storage
  graphRootAllocated: 41872785408
  graphRootUsed: 2622685184
  graphStatus:
    Backing Filesystem: btrfs
    Native Overlay Diff: "true"
    Supports d_type: "true"
    Using metacopy: "false"
  imageCopyTmpDir: /var/tmp
  imageStore:
    number: 2
  runRoot: /run/user/344800018/containers
  volumePath: /home/podman/.local/share/containers/storage/volumes
version:
  APIVersion: 4.3.0
  Built: 1666339791
  BuiltTime: Fri Oct 21 03:09:51 2022
  GitCommit: ""
  GoVersion: go1.19.2
  Os: linux
  OsArch: linux/amd64
  Version: 4.3.0

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

podman-4.3.0-2.fc37.x86_64

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/main/troubleshooting.md)

Yes

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

This is running in a KVM VM on a Fedora 37 hypervisor

[ygalblum]

I am also looking at a similar issue. The root cause of this failure is the fact that podman play kube --down does not delete the volumes. As a result, when running podman play kube again, the volume is still there, but podman tried to create it (since it does not know that it is the same one).

I implemented the code to remove the volume as part of calling --down (https://github.com/ygalblum/podman/tree/kube_volume_down). But, when I started writing the test, I found that there is a dedicated test checking that the volume is not removed (https://github.com/containers/podman/blob/main/test/e2e/play_kube_test.go#L3423). So, I'm not sure if this is not "by design".

Having said that, I'm not sure which behavior you are looking for - deleting the volumes or reusing them.

[vrothberg]

The logic was added with #11298 but I am unable to extract the reasoning for excluding volumes from teardown. @baude can you elaborate on that?

[baude]

the idea was that podman should not delete important information by default ... in this use case, it does make sense to nuke the volume I guess. But in the case where the user is expecting the volume data to be preserved, it does not ... should we just not error if the volume already exists by name? that would leave the responsibility of what to do with the stuff on the volume to the user. we could also add a flag for tearing down volumes optionally as well?

[vrothberg]

the idea was that podman should not delete important information by default ... in this use case, it does make sense to nuke the volume I guess. But in the case where the user is expecting the volume data to be preserved, it does not ... should we just not error if the volume already exists by name?

That sounds good. @alexlarsson added the --ignore flag, so the plumbing should be easy.

BUT: How does Kubernetes behave?

that would leave the responsibility of what to do with the stuff on the volume to the user. we could also add a flag for tearing down volumes optionally as well?

Note that there is podman kube down now where we can easily add new flags. There is also podman kube play --down which isn't ideal for extending the behavior.

[mheon]

Tackling this from a different direction: why does play kube refuse to start if a volume with the given name already exists? I would think it would just use the existing volume in that case.

[mhjacks]

We've just been talking through that on gchat. Current proposal (please correct me if this is inaccurate @baude) is to create volumes if they don't exist, use volumes if they do exist at start time, and add a --force flag to the "down" command to delete volumes if the user wants to remove them.

[mhjacks]

Presumably the systemd integration will not add the "--force" flag by default? This means the default case for systemd (systemctl stop/restart) service will work, as will a reboot; an edge case would be if the node loses power; but in such a situation I think it's reasonable to at least try to start the service with existing volumes; if that fails that can be detected, and the volumes can be cleared with --force or another out-of-band teardown mechanism. "kube play" would then be able to proceed in the normal "no existing volumes" case by creating them

[vrothberg]

Presumably the systemd integration will not add the "--force" flag by default?

ACK. Unless we find a compelling reason, it seems like a sane default to not remove volumes.

We plan on adding Kube support to Quadlet, where users should be able to specify whether they want volumes to be removed.