Sidecar port default protocol are buggy

[haf]

This is a duplicate post of #1291 — which was never fixed. The core problem after investigation is that the protocol in the ports declaration of side-cars isn't compared in a way that counts protocol: null === protocol: TCP despite TCP being the default if protocol in the Go k8s struct is nil.

This is a good first issue:

1. Create a unit test that validates the bug (by failing) when comparing the protocol property of a ports array (of a side-car or otherwise)
2. Special-case the defaults: this could potentially include other defaults if they exist for this record

Or alternatively; semantic comparison with defaults is probably already implemented in the k8s go SDK: call this SDK's comparison function instead.

---

After getting advice, this is a cross post with zalando/spilo#536 — it would seem the fault is with the operator and not Spilo.

• Which image of the operator are you using? registry.opensource.zalan.do/acid/postgres-operator:v1.6.0
• Where do you run it - cloud or metal? Kubernetes or OpenShift? GCP
• Are you running Postgres Operator in production? yes
• Type of issue? bug report

For me it's enough to define a sidecar like this, to trigger the behaviour:

  sidecars:
  - name: exporter
    image: wrouesnel/postgres_exporter

    env:
    - name: "DATA_SOURCE_URI"
      value: app-analytics-db/analytics?sslmode=require

    - name: "DATA_SOURCE_USER"
      valueFrom:
        secretKeyRef:
          name: postgres.app-analytics-db.credentials
          key: username

    - name: "DATA_SOURCE_PASS"
      valueFrom:
        secretKeyRef:
          name: postgres.app-analytics-db.credentials
          key: password

    - name: PG_EXPORTER_WEB_LISTEN_ADDRESS
      value: ":9114"

    - name: PG_EXPORTER_CONSTANT_LABELS
      value: app=analytics-db,component=postgres

    ports:
    - name: http-prom
      containerPort: 9114

    resources:
      limits:
        cpu: 500m
        memory: 256M
      requests:
        cpu: 100m
        memory: 200M

Causes it to fail over every thirty minutes:

Some logs:

time="2021-01-06T17:24:36Z" level=debug msg="metadata.annotation are different" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:36Z" level=debug msg="-  zalando-postgres-operator-rolling-update-required: false" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:36Z" level=debug msg="+  zalando-postgres-operator-rolling-update-required: true" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:36Z" level=info msg="reason: new statefulset containers's exporter (index 1) ports do not match the current one" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:36Z" level=debug msg="updating statefulset" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:36Z" level=debug msg="patching statefulset annotations" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:37Z" level=debug msg="patching statefulset annotations" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:37Z" level=debug msg="calling Patroni API on a pod app/app-analytics-db-0 to set the following Postgres options: map[wal_level:logical]" cluster-name=app/app-analytics-db pkg=cluster worker=1
time="2021-01-06T17:24:37Z" level=debug msg="making PATCH http request: http://10.4.7.76:8008/config" cluster-name=app/app-analytics-db pkg=cluster worker=1

[FxKu]

This is the place where the sidecar ports are set. I don't know how enable internal K8s defaults here.

The comparison to the running statefulset is very basic atm. If somebody wants to add more logic there, PRs are welcome. Since we do not use sidecars ourselves there's no prio currently to fix this.

[jouve]

sidecar port is defined like this:

    ports:
    - containerPort: 9187
      name: metrics

time="2021-03-29T14:17:21Z" level=debug msg="-              containerPort: 9187," cluster-name=nextcloud/acid-nextcloud pkg=cluster
time="2021-03-29T14:17:21Z" level=debug msg="-              protocol: TCP" cluster-name=nextcloud/acid-nextcloud pkg=cluster
time="2021-03-29T14:17:21Z" level=debug msg="+              containerPort: 9187" cluster-name=nextcloud/acid-nextcloud pkg=cluster
...
time="2021-03-29T14:17:21Z" level=info msg="reason: new statefulset containers's exporter (index 1) ports do not match the current one" cluster-name=nextcloud/acid-nextcloud pkg=cluster

[toabi]

I found this because I have the same issue.

My sidecars config looks like this:

spec:
  databases: {}
  numberOfInstances: 2
  postgresql:
    version: "13"
  resources:
    limits:
      cpu: 2000m
      memory: 450Mi
    requests:
      cpu: 200m
      memory: 450Mi
  sidecars:
  - env:
    - name: DATA_SOURCE_URI
      valueFrom:
        fieldRef:
          fieldPath: metadata.name
    - name: DATA_SOURCE_USER
      valueFrom:
        secretKeyRef:
          key: username
          name: postgres.core-shared.credentials.postgresql.acid.zalan.do
    - name: DATA_SOURCE_PASS
      valueFrom:
        secretKeyRef:
          key: password
          name: postgres.core-shared.credentials.postgresql.acid.zalan.do
    - name: PG_EXPORTER_AUTO_DISCOVER_DATABASES
      value: "true"
    image: quay.io/prometheuscommunity/postgres-exporter:v0.10.0
    name: exporter
    ports:
    - containerPort: 9187
      name: exporter
      protocol: TCP
    resources:
      limits:
        cpu: 100m
        memory: 128Mi
      requests:
        cpu: 100m
        memory: 128Mi

But the whole set gets recreated every resync_period with this message:

time="2022-01-26T13:39:59Z" level=info msg="statefulset postgresql/core-shared is not in the desired state and needs to be updated" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-          terminationMessagePath: /dev/termination-log," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-          terminationMessagePolicy: File," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-                  apiVersion: v1," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-          terminationMessagePath: /dev/termination-log," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-          terminationMessagePolicy: File," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      restartPolicy: Always," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      dnsPolicy: ClusterFirst," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      serviceAccount: postgres-pod," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      }," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      schedulerName: default-scheduler" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="+      }" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      kind: PersistentVolumeClaim," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      apiVersion: v1," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      status: {" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-        phase: Pending" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-      }" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="+      status: {}" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-  }," cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="-  revisionHistoryLimit: 10" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="+  }" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=info msg="reason: new statefulset containers's exporter (index 1) environment does not match the current one" cluster-name=postgresql/core-shared pkg=cluster
time="2022-01-26T13:39:59Z" level=debug msg="updating statefulset" cluster-name=postgresql/core-shared pkg=cluster

It's not the ports in my case but the env… but I don't see what the difference could be. I also once added the default envs the operator adds to there but it didn't make a difference either.

[toabi]

Found out what the difference was for my issue: the apiVersion: v1 in that fieldRef. With this I don't have restarts anymore!

    - name: DATA_SOURCE_URI
      valueFrom:
        fieldRef:
          apiVersion: v1
          fieldPath: metadata.name