Auto-deploy Manifests Should Periodically Re-evaluate

[ekristen]

Environmental Info:
K3s Version:

k3s version v1.21.3+k3s1 (1d1f220f)
go version go1.16.6

Node(s) CPU architecture, OS, and Version:

Linux ip-10-130-0-25 5.8.0-1041-aws #43~20.04.1-Ubuntu SMP Thu Jul 15 11:07:29 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Cluster Configuration:

1 server

Describe the bug:

This could easily be a bug or a feature request. Technically the behavior 15 months ago was preferred from my perspective, but due to issues with HA and upgrades was altered.

As it stands today, once the server does the initial auto-deploy, it will only pick up changes IF the modtime on the file has changed.

Previously it behaved more like gitops model where the manifests were treated as a source of truth, whatever was in manifests directory was always applied against the cluster, so if you made changes directly to the cluster that contradicted the manifest files, they'd be overwritten on the next evaluation loop.

In my opinion, the previous way was better.

From a HelmChart use case, when it deploys a service along with a deployment and said service is deleted, the service will never come back.

The behavior of the auto-deploy is a bit inconsistent too, the first run of the deploy controller will always evaluate all manifests but subsequent runs will not unless the modtime has changed. This means that after k3s is up if someone goes and modifies the traefic or metrics-server deployment directly on the cluster it'll be 100% ok, but as soon as the deploy controller restarts for any reason, all those changes will be overwritten, which could be 1 hour from now or 30 days from now.

I suggest that the manifests directory should always be applied therefore being a reliable source of truth.

I currently have a work around for this. I place an entry in /etc/cron.d with a script that literally runs touch against every file in the /var/lib/rancher/k3s/server/manifests directory every minute. This forces k3s to re-evaluate on it's 15 second interval and will see the modtime changed and then apply the changes against the cluster.

Steps To Reproduce:

• Install K3s
• Place manifest file that creates a deployment
• Delete the deployment

Expected behavior:

I would expect the auto-deploy to re-evaluate and have the deployment return eventually.

Actual behavior:

The deployment never returns unless the modtime of the file is modified.

[brandond]

This behavior hasn't been changed - the manifests have only ever been applied under two conditions:

• All manifests are applied at server node startup
• While k3s is running, manifests are re-applied whenever their mtime and content changes.

[ekristen]

@brandond thanks for your response, ultimately I think this should be treated more as a feature request than a bug.

It would be nice to get this feature more on parity with flux and the gitops model of being a source of truth, if set, it should always ensure anything defined is in the state expected vs run once and forget unless modified. It also then creates a wonky behavior of on a restart it gets re-applied.

However I would have to disagree with your assertion that it hasn't changed.

Your first assertion is correct, it will always apply at start. As for your second assertion, it'll re-apply anytime mtime is modified (regardless of content changes) unless compareChecksums is enabled and the checksum matched. You can test this by simply running touch manifest.yaml file.

However prior to 15 months ago, they were always applied unless compareChecksum was enabled and the checksum matched.

By default I never see checksum set on the Addon.Spec so I think the compareChecksums is likely not a factor.

[brandond]

Ah I see, the checksum validation behavior was broken just over a month ago, in a7d1159. I'm not sure what might have changed 15 months ago - that would have probably around the 1.17 time frame? Can you clarify which release you're coming from that behaved as you're describing?

[ekristen]

I don't have a version, I've been using it a long time, just something I noticed and then I dug into the code and backtracked the changes to see the commits that changed the behavior,

15 months ago the mtime check was added.

[ekristen]

I don't have a version, I've been using it a long time, just something I noticed and then I dug into the code and backtracked the changes to see the commits that changed the behavior,

15 months ago the mtime check was added.

[rancher-max]

Validated on master branch commit 699ea165231cf6d665425c50ff5c75e1831f4afb

• Deploying custom manifests that are located in /var/lib/rancher/k3s/server/manifests continues to work on startup. I did this by adding the following file there as busyb.yaml:

apiVersion: v1
kind: Service
metadata:
  name: busyb
spec:
  selector:
    app: busy
  clusterIP: None
  ports:
  - name: foo # Actually, no port is needed.
    port: 1234
    targetPort: 1234
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: busydep
spec:
  replicas: 3
  selector:
    matchLabels:
      app: busy
  template:
    metadata:
      labels:
        app: busy
    spec:
      containers:
        - name: busybox
          image: busybox:1.28
          command:
            - sleep
            - "3600"
          securityContext:
            runAsUser: 1000
            runAsNonRoot: true

• I can then delete the busyb deployment in the cluster
  • Then, if I just run touch /var/lib/rancher/k3s/server/manifests/busyb.yaml it does NOT redeploy since the checksum did not change
  • If I run touch /var/lib/rancher/k3s/server/manifests/coredns.yaml it does also not try to redeploy that
  • If I now change contents in /var/lib/rancher/k3s/server/manifests/busyb.yaml, for example change the sleep 3600 to sleep 1800, then it successfully redeploys
• Confirmed that checksums match on both this custom manifest and default ones like coredns and traefik with commands such as the following:

$ kubectl get addon coredns -n kube-system -o yaml
apiVersion: k3s.cattle.io/v1
kind: Addon
metadata:
  creationTimestamp: "2021-09-14T18:37:35Z"
  generation: 2
  name: coredns
  namespace: kube-system
  resourceVersion: "308"
  uid: 3ac713db-721f-4583-8457-4f58b34948a9
spec:
  checksum: 6f91460074c0a726b39dc38be9e7ae89fbab1a2a81fb3e999f548420c54eafe1
  source: /var/lib/rancher/k3s/server/manifests/coredns.yaml
status: {}

$ sudo sha256sum /var/lib/rancher/k3s/server/manifests/coredns.yaml
6f91460074c0a726b39dc38be9e7ae89fbab1a2a81fb3e999f548420c54eafe1  /var/lib/rancher/k3s/server/manifests/coredns.yaml

[ekristen]

Wait just so I understand, the manifests only get applied if the file contents has changed?

The applying if the modtime has changed has been removed?

[brandond]

that's correct, that was the expected behavior that was regressed as discussed in #3711 (comment). Manifests are only intended to apply on startup or when changed.

[ekristen]

Well shoot. Removing the modtime now takes my entire workaround away. Perhaps I didn't communicate effectively.

The problem that the original issue was calling out is that the manifests weren't being re-evaluated on a periodic basis, where early versions I'm fairly confident they were, but no hard proof.

The problem was that if I added the following service, then deleted it, unless the contents changed or the modtime changed it would never get re-evaluated against the cluster, which I felt was the real issue.

Now with this change, unless the contents have changed it'll never get re-applied which is unfortunately because it was the source of truth initially, but now even though the service doesn't need to change won't get applied until it changes.

apiVersion: v1
kind: Service
metadata:
  name: busyb
spec:
  selector:
    app: busy
  clusterIP: None
  ports:
  - name: foo # Actually, no port is needed.
    port: 1234
    targetPort: 1234

My point/suggestion was that all manifests should be re-evaluated against the cluster periodically and treat them as a source of truth like Flux for example does.

With this fix now, I have no way to ensuring that the manifests in the directly remain applied against the cluster at all times.

Having used the workaround now for several months I think the modtime regression should stay in place.

Ideally I think there should be three triggers for re-apply/re-evaluating against the cluster

1. modtime changes
2. checksum changes
3. period of time passes (ie 2min/5min, possibly configurable)