authzed / spicedb-operator

Kubernetes controller for managing instances of SpiceDB
Apache License 2.0
63 stars 29 forks source link

extraPodAnnotations doesn't apply to the migration pod #146

Closed Bhashit closed 1 year ago

Bhashit commented 1 year ago

I deployed the following in a namespace that has istio injection enabled

apiVersion: authzed.com/v1alpha1
kind: SpiceDBCluster
metadata:
  name: dev
spec:
  config:
    datastoreEngine: postgres
    extraPodAnnotations: 
      sidecar.istio.io/inject: "false"
  secretName: dev-spicedb-config

I was trying to add the annotation with extraPodAnnotations to the generated pods so that the sidecar doesn't get injected

Istio adds a sidecar to every pod created in that namespace. Because the sidecar keeps running, the pod never reaches the Completed stage, preventing the operator from progressing further (and creating the spicedb pods)

Since this migration pod is also generated by the operator, shouldn't the additional annotations apply to the migration pod as well?

I know I can either deploy the spicedb cluster to a namespace that doesn't inject sidecars, or configure the Istio operator to never inject anything in the pods that match the labels for spicedb, But it may be simpler, and perhaps more consistent, to apply the same annotations to the migration pod as well.

Another possible solution could be for the operator to only check the status of the migration container within the pod, instead of checking the pod status before moving on

ecordell commented 1 year ago

Right now, extraPodAnnotations only applies to the deployment pods, not the jobs.

There's a PR in progress (https://github.com/authzed/spicedb-operator/pull/135) that attempts to address this generically - that would look something like:

apiVersion: authzed.com/v1alpha1
kind: SpiceDBCluster
metadata:
  name: dev
spec:
  config:
    datastoreEngine: postgres
  secretName: dev-spicedb-config
  patches:
    - kind: Job
      patch:
        spec:
          template:
            metadata:
              annotations:
                sidecar.istio.io/inject: "false"
Bhashit commented 1 year ago

Thanks for the very quick response (once again, since you did the same earlier this morning). Looking forward to it.

thomasklein94 commented 1 year ago

While the PR #135 looks exciting, until the work there has completed, consider merging PR #147.

Bhashit commented 1 year ago

@ecordell any plans to cut a new release with the #147 merged in?