Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Garbage collection conflicts with Stash controller #315

Closed
Legion2 opened this issue Apr 7, 2021 · 18 comments · Fixed by #362
Closed

Garbage collection conflicts with Stash controller #315

Legion2 opened this issue Apr 7, 2021 · 18 comments · Fixed by #362

Comments

@Legion2
Copy link

Legion2 commented Apr 7, 2021

I'm using Stash to backup my kubernetes applications. Stash is a k8s operator. I manage the configuration of stash with a kustomization, which applies BackupConfigurations to the cluster. Stash will create a service account for those BackupConfigurations, therefore it copies the labels of the BackupConfiguration, which includes the kustomize.toolkit.fluxcd.io/checksum label.

Because the service account is not managed by flux and has the kustomize.toolkit.fluxcd.io/checksum label and pruning is activated, flux garbage collects the service account. As a result, the backups with stash do not work. This is probably a problem which must be fixed in stash, so I created an issue there stashed/stash#1334.

However, the garbage collection behavior of flux is inconsistent. I have four BackupConfigurations each with its own service account, but flux only deletes two of them. The two remaining service accounts have the kustomize.toolkit.fluxcd.io/checksum label, but are not garbage collected by flux.

@kingdonb
Copy link
Member

kingdonb commented Apr 7, 2021

Maybe they are not in a namespace that was targeted by Kustomization with prune enabled?

It is hard to say what might be inconsistent about Flux's behavior, or if it is more likely a configuration surprise that we will understand only by reviewing details of your implementation...

https://toolkit.fluxcd.io/components/kustomize/kustomization/#garbage-collection

There's a workaround for you, in case you can't get Stash to stop doing that and you are still struggling with this until a fix is available, just disable garbage collection of your BackupConfigurations with an annotation or label. Let Stash go on copying the labels, it will copy this one to the service accounts too. (I guess also be sure to re-enable pruning first in the event you ever need to disable and delete an old BackupConfiguration.)

You can disable pruning for certain resources by either labeling or annotating them with:

  kustomize.toolkit.fluxcd.io/prune: disabled

@stefanprodan
Copy link
Member

I have four BackupConfigurations each with its own service account, but flux only deletes two of them. The two remaining service accounts have the kustomize.toolkit.fluxcd.io/checksum label, but are not garbage collected by flux.

It’s a race condition, by the time Flux gets to those SAs, Stash has copied the label value with the new checksum. I don’t see how we can fix any of this in kustomize-controller.

@Legion2
Copy link
Author

Legion2 commented Apr 7, 2021

Maybe they are not in a namespace that was targeted by Kustomization with prune enabled?

Everything in the cluster is managed by flux, I used the flux bootstrap command to create a single Kustomization which reconciles all namespaces. For example, in the monitoring namespace there are two BackupConfigurations, one service account is deleted the other is not deleted:

Name:                stash-backup-grafana-backup
Namespace:           monitoring
Labels:              app.kubernetes.io/component=stash-backup
                     app.kubernetes.io/managed-by=stash.appscode.com
                     kustomize.toolkit.fluxcd.io/checksum=9e737fff1170a97ad93c22161d497eb63bba0211
                     kustomize.toolkit.fluxcd.io/name=flux-system
                     kustomize.toolkit.fluxcd.io/namespace=flux-system
Annotations:         <none>
Image pull secrets:  <none>
Mountable secrets:   stash-backup-grafana-backup-token-vkcvd
Tokens:              stash-backup-grafana-backup-token-vkcvd
Events:              <none>

@Legion2
Copy link
Author

Legion2 commented Apr 7, 2021

It’s a race condition, by the time Flux gets to those SAs, Stash has copied the label value with the new checksum. I don’t see how we can fix any of this in kustomize-controller.

@stefanprodan What exactly is stored in the checksum and how does flux decide if it should delete the resource?

@stefanprodan
Copy link
Member

The value is computed from the source artifact, it changes every time there is a change in the manifests that are included in the kustomize overlay. Do you know if Stash copies annotations too?

@Legion2
Copy link
Author

Legion2 commented Apr 7, 2021

The value is computed from the source artifact, it changes every time there is a change in the manifests that are included in the kustomize overlay.

OK, now I understand the race condition. Flux updates the checksum on the all resources, including the BackupConfiguration, but not the SAs. Then Stash updates the SA labels with the new checksum while flux is garbage collecting all resources with the old checksum. Depending on which controller is faster, the SA is deleted or not.

The only strange thing here is, why does Stash not recreate the SA with the new checksum after they have been deleted.

Is the checksum the same for all resources of the same Kustomization for a specific source artifact revision?

Do you know if Stash copies annotations too?

No it only copies the labels.

@stefanprodan stefanprodan changed the title Inconsistent prune behavior Garbage collection conflicts with Stash controller Apr 15, 2021
@aholbreich
Copy link

I don't understand the problem in detail.
In what situation checksum is changed or will be computed differently?
An is there already an idea of how to fix this?

Experiencing same with Strimzi Kafk Operator

@stefanprodan
Copy link
Member

We're moving the checksum from labels to annotations in #362 but if Strimzi copies the annotations then it's the same issue. Seems that for Stash is will work as it copies only labels from the custom resource to the generated objects.

@aholbreich
Copy link

This is Custome Resource from Strimzi, deployed by Flux

 kubectl get Kafka
#output
NAME           DESIRED KAFKA REPLICAS   DESIRED ZK REPLICAS   READY   WARNINGS
main-cluster   3                        3                     True    
[aholbreich@t490-fedora ~]$ kubectl describe Kafka
Name:         main-cluster
Namespace:    kafka
Labels:       kustomize.toolkit.fluxcd.io/checksum=0fad79a67a6883eba7a461828bb901f3a9ec30b9
              kustomize.toolkit.fluxcd.io/name=flux-system
              kustomize.toolkit.fluxcd.io/namespace=flux-system
Annotations:  <none>
API Version:  kafka.strimzi.io/v1beta2
Kind:         Kafka
...

it doesn't look like it has annotations.

@stefanprodan
Copy link
Member

it doesn't look like it has annotations.

Add an annotation to it and check if the operator copies it on pods and other generated objects, if it doesn't then #362 would fix this.

@aholbreich
Copy link

@stefanprodan i've added annotation and with prune:false on responsible Kustomization it seem not to destroy the Kafka cluster anymore. Using Flux 0.14.2

@stefanprodan
Copy link
Member

@aholbreich you may want to test flux 0.15, it should work fine with pruning enabled due to #362

@aholbreich
Copy link

aholbreich commented Jun 21, 2021

@stefanprodan Unfortunately not.

[kustomization/flux-system.flux-system
KafkaTopic/kafka/test-topic deleted
ConfigMap/kafka/main-cluster-entity-topic-operator-config deleted
ConfigMap/kafka/main-cluster-entity-user-operator-config deleted
Deployment/kafka/main-cluster-entity-operator deleted
Deployment/kafka/main-cluster-kafka-exporter deleted
revision
5c880ae09d90a22e9d313813b2132fb06766450c
17:59 Uhr
kustomization/kafka-ressources.flux-system
ConfigMap/kafka/main-cluster-kafka-config deleted
ConfigMap/kafka/main-cluster-zookeeper-config deleted
Service/kafka/main-cluster-kafka-bootstrap deleted
Service/kafka/main-cluster-kafka-brokers deleted
Service/kafka/main-cluster-zookeeper-client deleted
Service/kafka/main-cluster-zookeeper-nodes deleted
Weniger anzeigen
revision
b85323abaeaac8517bc387551d7d8edef99e8f1a](url)

I'm not sure why it deletes resources that are not directly created by Flux.... like Service/kafka/main-cluster-zookeeper-client deleted

 kubectl describe Kafka
Name:         main-cluster
Namespace:    kafka
Labels:       kustomize.toolkit.fluxcd.io/name=kafka-ressources
              kustomize.toolkit.fluxcd.io/namespace=flux-system
Annotations:  myorg.askme: Alexander
              kustomize.toolkit.fluxcd.io/checksum: b85323abaeaac8517bc387551d7d8edef99e8f1a
API Version:  kafka.strimzi.io/v1beta2
Kind:         Kafka
Metadata:
  Creation Timestamp:  2021-06-21T15:22:50Z
  Generation:          1
  Managed Fields:
    API Version:  kafka.strimzi.io/v1beta2
    Fields Type:  FieldsV1

another two being created by the operator, just for the sake of completeness:

kubectl describe svc main-cluster-kafka-bootstrap
Name:              main-cluster-kafka-bootstrap
Namespace:         kafka
Labels:            app.kubernetes.io/instance=main-cluster
                   app.kubernetes.io/managed-by=strimzi-cluster-operator
                   app.kubernetes.io/name=kafka
                   app.kubernetes.io/part-of=strimzi-main-cluster
                   kustomize.toolkit.fluxcd.io/name=flux-system
                   kustomize.toolkit.fluxcd.io/namespace=flux-system
                   strimzi.io/cluster=main-cluster
                   strimzi.io/discovery=true
                   strimzi.io/kind=Kafka
                   strimzi.io/name=main-cluster-kafka
Annotations:       strimzi.io/discovery:
                     [ {
                       "port" : 9092,
                       "tls" : false,
                       "protocol" : "kafka",
                       "auth" : "none"
                     }, {
                       "port" : 9093,
                       "tls" : true,
                       "protocol" : "kafka",
                       "auth" : "none"
                     } ]
Selector:          strimzi.io/cluster=main-cluster,strimzi.io/kind=Kafka,strimzi.io/name=main-cluster-kafka
Type:              ClusterIP
kubectl describe Deployment main-cluster-entity-operator
Name:               main-cluster-entity-operator
Namespace:          kafka
CreationTimestamp:  Mon, 21 Jun 2021 18:05:56 +0200
Labels:             app.kubernetes.io/instance=main-cluster
                    app.kubernetes.io/managed-by=strimzi-cluster-operator
                    app.kubernetes.io/name=entity-operator
                    app.kubernetes.io/part-of=strimzi-main-cluster
                    kustomize.toolkit.fluxcd.io/name=kafka-ressources
                    kustomize.toolkit.fluxcd.io/namespace=flux-system
                    strimzi.io/cluster=main-cluster
                    strimzi.io/kind=Kafka
                    strimzi.io/name=main-cluster-entity-operator
Annotations:        deployment.kubernetes.io/revision: 1
Selector:           strimzi.io/cluster=main-cluster,strimzi.io/kind=Kafka,strimzi.io/name=main-cluster-entity-operator
Replicas:           1 desired | 1 updated | 1 total | 1 available | 0 unavailable
StrategyType:       Recreate
MinReadySeconds:    0
Pod Template:
  Labels:           app.kubernetes.io/instance=main-cluster
                    app.kubernetes.io/managed-by=strimzi-cluster-operator
                    app.kubernetes.io/name=entity-operator
                    app.kubernetes.io/part-of=strimzi-main-cluster
                    kustomize.toolkit.fluxcd.io/name=kafka-ressources
                    kustomize.toolkit.fluxcd.io/namespace=flux-system
                    strimzi.io/cluster=main-cluster
                    strimzi.io/kind=Kafka
                    strimzi.io/name=main-cluster-entity-operator
  Annotations:      strimzi.io/clients-ca-cert-generation: 0
                    strimzi.io/cluster-ca-cert-generation: 0

@aholbreich
Copy link

By the way... this is strange... but are those resources bounded to KS?
so here the wrong KS "flux-system" took place and delete the resources. (why?)

event when another should care about this place:

metadata:
  name: kafka-ressources
  namespace: flux-system
spec:
  path: ./eks_DevCluster/kafka-ressources/
  interval: 10m
  dependsOn:
    - name: kafka-operator

flux-system 'looks' to everything in path: ./eks_DevCluster/ but should commit ./eks_DevCluster/kafka-ressources/.
Maybe this is not very good setup?
I'm not using any K8s Kutomizations in this setup would it make things controlled?

@aholbreich
Copy link

Retested with new topics. Add remove, modify. Generally, the fix seems to be working! THX!
So the problem I've experienced mus grounding in the issue I've described before... And I'm not sure how to make it robust.
Basically, flux-system has not take over when not intended.

@aholbreich
Copy link

A very positive side-effect of migration to annotations is the smooth migration from non-flux resources to flux managed resources!!
You don't have to delete them, because annotation can be added on existing resources!
Contrary changing labels is not possible without complete deletion/undeplyment of resource, which could be very nasti for productive clusters...

@stefanprodan
Copy link
Member

You're applying the same thing twice and Flux will overwrite/delete all, please see how to organise your repos here: https://fluxcd.io/docs/guides/repository-structure/

PS. Move all things outside the clusters dir and create a Flux Kustomization inside clusters for each kustomize overlay.

@mtneug
Copy link

mtneug commented Jul 8, 2021

Is this really fixed? If I understand the code correctly, a selected resource (i.e. labels kustomize.toolkit.fluxcd.io/name and kustomize.toolkit.fluxcd.io/namespace are set) is still considered stale if there is no kustomize.toolkit.fluxcd.io/checksum annotation. Instead, if this annotation is missing and pruning is active this resource should not be considered stale.

I see the same issue with cert-manager and Certificate resources created from Ingress resources. Labels are copied over as well. The GC will remove some of the Certificate resources, but not all. I don't understand why since now there is no race since the kustomize.toolkit.fluxcd.io/checksum annotation is not copied/updated by cert-manager. I'm running version 0.13.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants