Telegraf Operator prevents pods creation when the secret already exists

[sumo-drosiek]

If a secret for pod already exists, the exception is raised and pod is not being created (with or without sidecar)

telegraf-operator logs

2020-10-09T12:44:53.473Z        ERROR   setup.podInjector       unable to create secret {"error": "secrets \"telegraf-config-demo-redis-bitnami-slave-0\" already exists"}
github.com/go-logr/zapr.(*zapLogger).Error
        /go/pkg/mod/github.com/go-logr/zapr@v0.1.0/zapr.go:128
main.(*podInjector).Handle
        /workspace/handler.go:123
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).Handle
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.4.0/pkg/webhook/admission/webhook.go:135
sigs.k8s.io/controller-runtime/pkg/webhook/admission.(*Webhook).ServeHTTP
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.4.0/pkg/webhook/admission/http.go:87
sigs.k8s.io/controller-runtime/pkg/webhook.instrumentedHook.func1
        /go/pkg/mod/sigs.k8s.io/controller-runtime@v0.4.0/pkg/webhook/server.go:117
net/http.HandlerFunc.ServeHTTP
        /usr/local/go/src/net/http/server.go:2007
net/http.(*ServeMux).ServeHTTP
        /usr/local/go/src/net/http/server.go:2387
net/http.serverHandler.ServeHTTP
        /usr/local/go/src/net/http/server.go:2802
net/http.(*conn).serve
        /usr/local/go/src/net/http/server.go:1890

statefulset events

Events:
  Type     Reason        Age                From                    Message
  ----     ------        ----               ----                    -------
  Warning  FailedCreate  2s (x14 over 43s)  statefulset-controller  create Pod demo-redis-bitnami-slave-0 in StatefulSet demo-redis-bitnami-slave failed error: admission webhook "telegraf.influxdata.com" denied the request: secrets "telegraf-config-demo-redis-bitnami-slave-0" already exists

I expect to automatically reload the secret (if it was created using telegraf-operator) or to run pods without the sidecat

Relevant URLs

N/A

What products and version are you using?

• telegraf-operator helm chart - 1.1.4
• telegraf-operator - quay.io/influxdb/telegraf-operator:v1.1.0

[wojciechka]

I have reproduced it by deploying telegraf-operator, manually creating the secret with matching name and then deploying a statefulset (statefulset will cause the name of the secret to be predictable, hence this is how I could reproduce it).

However, I can see how this would happen when telegraf-operator was temporarily down when a pod in a statefulset was being deleted.

I think the best long term approach would be to support upgrading the secret when it already exists. This has the danger of overwriting an existing secret that just collided because of same name.

Ideally we should only update the secret if it has a valid annotation - i.e. app.kubernetes.io/managed-by set to telegraf-operator. However, this would break it when upgrading any existing telegraf-operator - since existing secrets would not have the annotation.

So, I propose we always add the annotation and regarding handling of existing secrets:

• by default, always update the secret when needed
• provide an optional CLI flag that will fail if the annotation is not present - the flag could be used for new deployments of telegraf-operator

Additional checks could be made regarding the secret - such as that the secret is of type Opaque and only has one key - telegraf.conf. This will allow us to be safe about accidentally updating an existing secret.

[BondAnthony]

@wojciechka I like the annotation approach of calling out telegraf-operator as the managed-by. I think this would be good regardless so we can easily link secrets with telegraf-operator.

Regards to secret update, the cli fail flag would make sense to help with the upgrade. Once users are beyond this change do we see a need for this going forward if the annotations are present?

[sumo-drosiek]

@wojciechka I tested the PR locally and it solves the issue for us 👍