devspace-sh / devspace

DevSpace - The Fastest Developer Tool for Kubernetes ⚡ Automate your deployment workflow with DevSpace and develop software directly inside Kubernetes.
https://devspace.sh
Apache License 2.0
4.28k stars 359 forks source link

Enabling `persistPaths` in devPod spec always replaces devPod even when there has been no config/podSpec changes #2870

Open marcm-ml opened 2 months ago

marcm-ml commented 2 months ago

What happened?

My devPod is always replaced by a new "pod" which is identical to the already running pod. This cause some delay between consecutive devspace dev command which is unecessary as it is the same podSpec. When using --debug I can see a log statement:

Update replaced deployment with patch:
<json patches>

which comes from: https://github.com/devspace-sh/devspace/blob/3212b31cc90cc324f0f7ddfbec42a79ea4ff7cb3/pkg/devspace/services/podreplace/replace.go#L175

There I could identify the root cause which boils down to using persistPaths which adds initContainer to the pods spec. Apparently, these always cause a replacement which I find odd since they did not change between consecutive sessions. This is also true for the config-hash annotations of the deployment resource which stays the same between sessions. The podSpec is also identical but the pod-template-hash annotations changes which causes the replacement in the first place, I would assume.

What did you expect to happen instead?

I expect no replacement to happen if the devspace.yaml config stays the same.

How can we reproduce the bug? (as minimally and precisely as possible)

Any devspace.yaml wich configures as persistPaths option:

dev:
  dev:
    persistPaths:
      - path: /app
        volumePath: app

Removing the persistPaths fixes the issue and no replacement happens between sessions. Curiously, for my config it still prints the same log messages and shows me some patches (without initContainers) that are applied but these do not cause the deployment to change and thus do not trigger a replacement.

Local Environment:

Anything else we need to know?

marcm-ml commented 2 months ago

taken from the logs and prettified:

Json patches causing replacement:

{
 "spec": {
  "template": {
   "spec": {
    "initContainers": [
     {
      "args": [
       "-c",
       "if [ ! -d \"/devspace-persistence/.devspace/\" ] && [ -d \"/app\" ]; then\n echo 'DevSpace is initializing the sync volume...'\n cp -a /app/. /devspace-persistence/ && mkdir /devspace-persistence/.devspace\nfi"
      ],
      "command": [
       "sh"
      ],
      "image": "python:latest",
      "name": "path-0-init",
      "resources": {},
      "volumeMounts": [
       {
        "mountPath": "/devspace-persistence",
        "name": "devspace-persistence",
        "subPath": "app"
       }
      ]
     },
     {
      "args": [
       "-c",
       "if [ ! -d \"/devspace-persistence/.devspace/\" ] && [ -d \"/root\" ]; then\n echo 'DevSpace is initializing the sync volume...'\n cp -a /root/. /devspace-persistence/ && mkdir /devspace-persistence/.devspace\nfi"
      ],
      "command": [
       "sh"
      ],
      "image": "python:latest",
      "name": "path-1-init",
      "resources": {},
      "volumeMounts": [
       {
        "mountPath": "/devspace-persistence",
        "name": "devspace-persistence",
        "subPath": "user_home"
       }
      ]
     }
    ],
    "securityContext": null
   }
  }
 }
}

Json patches causing NO replacement with disabled persistPaths option:

{
    "metadata": {
        "annotations": {
            "deployment.kubernetes.io/revision": "4"
        }
    },
    "spec": {
        "template": {
            "spec": {
                "securityContext": null
            }
        }
    }
}
marcm-ml commented 1 month ago

@FabianKramm sorry for the ping but when researching this issue and ways around it, I have come across an old commit from yours: https://github.com/devspace-sh/devspace/commit/fd99ddf81651b8150102d5a64b375a37a59f1fe3#diff-104ea0e9434e5a0a563d2de4dfc8d255b19fbef07cb2b1d4e22be9b68d1d293eL157-L164

do you remember what the issue was with the previous approach (L157-L164) and why you removed it? Also will anyone work on the issue?