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

Setting custom labels via PodTemplate.metadata.labels doesn't work #575

Closed
mglotov opened this issue Oct 2, 2023 · 8 comments · Fixed by #649
Closed

Setting custom labels via PodTemplate.metadata.labels doesn't work #575

mglotov opened this issue Oct 2, 2023 · 8 comments · Fixed by #649
Labels
bug Something isn't working done Issues in the state 'done'

Comments

@mglotov
Copy link

mglotov commented Oct 2, 2023

What happened?

We are trying to set custom pod labels to inject Istio-sidecar into Cassandra's pods.
This is our CassandraDatacenter manifest:

apiVersion: cassandra.datastax.com/v1beta1
kind: CassandraDatacenter
metadata:
  name: dc1
  namespace: cass-operator
spec:
  clusterName: cluster1
  size: 3
  resources:
   requests:
     memory: 12Gi
     cpu: 3
   limits:
     memory: 12Gi
     cpu: 3
  storageConfig:
    cassandraDataVolumeClaimSpec:
      storageClassName: server-storage
      accessModes:
        - ReadWriteOnce
      resources:
        requests:
          storage: 512Gi
  serverType: "cassandra"
  serverVersion: "3.11.7"

  managementApiAuth:
    insecure: {}
    serviceAccount: "default"

  config:

    # See http://cassandra.apache.org/doc/latest/configuration/cassandra_config_file.html
    cassandra-yaml:
      num_tokens: 8
      file_cache_size_in_mb: 1000
      authenticator: org.apache.cassandra.auth.PasswordAuthenticator
      authorizer: org.apache.cassandra.auth.CassandraAuthorizer
      role_manager: org.apache.cassandra.auth.CassandraRoleManager
      read_request_timeout_in_ms: 15000
    jvm-options:

      # Set the database to use 14 GB of Java heap
      initial_heap_size: "4G"
      max_heap_size: "4G"

      additional-jvm-opts:

        # As the database comes up for the first time, set system keyspaces to RF=3
        - "-Ddse.system_distributed_replication_dc_names=dc1"
        - "-Ddse.system_distributed_replication_per_dc=3"

  podTemplateSpec:
    metadata:
      labels:
        istio.io/rev: stable

However, label isn't set.

$ kdp -n cass-operator cluster1-dc1-rack1-sts-0
Name:             cluster1-dc1-rack1-sts-0
Namespace:        cass-operator
Priority:         0
Service Account:  default
Start Time:       Mon, 02 Oct 2023 15:00:18 +0600
Labels:      app.kubernetes.io/created-by=cassandradatacenter_controller
                  app.kubernetes.io/instance=cassandra-cluster1
                  app.kubernetes.io/managed-by=cass-operator
                  app.kubernetes.io/name=cassandra
                  app.kubernetes.io/version=3.11.7
                  cassandra.datastax.com/cluster=cluster1
                  cassandra.datastax.com/datacenter=dc1
                  cassandra.datastax.com/node-state=Started
                  cassandra.datastax.com/rack=rack1
                  cassandra.datastax.com/seed-node=true
                  controller-revision-hash=cluster1-dc1-rack1-sts-6b94844bb4
                  statefulset.kubernetes.io/pod-name=cluster1-dc1-rack1-sts-0

We tried to manually kill pods, but it didn't help. Pods have only default labels.
Setting custom annotations via podTemplateSpec works well at the same time.

What did you expect to happen?

Our extra label is set to Cassandra pods

How can we reproduce it (as minimally and precisely as possible)?

Apply the manifest above

cass-operator version

1.12

Kubernetes version

v1.26.5-gke.2100

Method of installation

Kustomize

Anything else we need to know?

No response

@mglotov mglotov added the bug Something isn't working label Oct 2, 2023
@burmanm
Copy link
Contributor

burmanm commented Oct 2, 2023

Killing pods is not the correct way as cass-operator does not itself recreate the pods, the StatefulSet controller does. The StatefulSet should get updated first (and should have the labels) and then that one will apply the pods.

That said, Kubernetes itself prevents changing the metadata of PodTemplateSpec in StatefulSets, so this will require a bit of trickery. You need to delete the StatefulSet to get those changes applied:

kubectl delete statefulset.apps/cluster2-dc2-r1-sts --cascade=orphan

The cascade is important, otherwise Kubernetes will delete all the pods.

@mglotov
Copy link
Author

mglotov commented Oct 2, 2023

Killing pods is not the correct way as cass-operator does not itself recreate the pods, the StatefulSet controller does. The StatefulSet should get updated first (and should have the labels) and then that one will apply the pods.

That said, Kubernetes itself prevents changing the metadata of PodTemplateSpec in StatefulSets, so this will require a bit of trickery. You need to delete the StatefulSet to get those changes applied:

kubectl delete statefulset.apps/cluster2-dc2-r1-sts --cascade=orphan

The cascade is important, otherwise Kubernetes will delete all the pods.

@burmanm Thank you for a workaround.
Could you add this information in the doc, please. It's completely unclear that we must do extra manual steps (delete statefulsets), especially in case when the same changes work well with annotations

@burmanm
Copy link
Contributor

burmanm commented Oct 2, 2023

Yeah, I must add at some point the limitations of StatefulSets in some docs (such as PVC changes & these metadata and few other fields). It's only vaguely mentioned in the Kubernetes docs if one understand the basics:

https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-selector

(the label change prevention is tied to the selector). The annotations are allowed to be modified.

@mglotov
Copy link
Author

mglotov commented Oct 2, 2023

Yeah, I must add at some point the limitations of StatefulSets in some docs (such as PVC changes & these metadata and few other fields). It's only vaguely mentioned in the Kubernetes docs if one understand the basics:

https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#pod-selector

(the label change prevention is tied to the selector). The annotations are allowed to be modified.

You are right, but the doc doesn't say that .spec.selector must be completely the same as .spec.template.metadata.labels
Just apply this manifest to test:

apiVersion: apps/v1
kind: StatefulSet
metadata:
  name: web
spec:
  selector:
    matchLabels:
      app: nginx
  serviceName: "nginx"
  replicas: 1
  minReadySeconds: 10
  template:
    metadata:
      labels:
        app: nginx
        extra_label: test
    spec:
      terminationGracePeriodSeconds: 10
      containers:
      - name: nginx
        image: registry.k8s.io/nginx-slim:0.8
        ports:
        - containerPort: 80
          name: web
        volumeMounts:
        - name: www
          mountPath: /usr/share/nginx/html
  volumeClaimTemplates:
  - metadata:
      name: www
    spec:
      accessModes: [ "ReadWriteOnce" ]
      storageClassName: "standard"
      resources:
        requests:
          storage: 1Gi

This is a result:

$ k describe sts web
Name:               web
Namespace:          default
Selector:           app=nginx
Labels:             <none>
Annotations:        <none>
Replicas:           1 desired | 1 total
Update Strategy:    RollingUpdate
  Partition:        0
Pods Status:        1 Running / 0 Waiting / 0 Succeeded / 0 Failed
Pod Template:
  Labels:  app=nginx
           extra_label=test
$ kubectl get sts web
NAME   READY   AGE
web    1/1     5m41s
$ kdp web-0
Name:             web-0
Namespace:        default
Priority:         0
Service Account:  default
Labels:           app=nginx
                  controller-revision-hash=web-5786c6d9d6
                  extra_label=test
                  statefulset.kubernetes.io/pod-name=web-0

As you can see it isn't necessary to have the same set of labels.
In my case it will allow me to set the Istio specific label without changing the labels of .spec.selector

@mglotov
Copy link
Author

mglotov commented Oct 3, 2023

Also, if you run the next command to patch the statefulset:

kubectl patch statefulset web \
  --type merge \
  -p '{"spec":{"template":{"metadata":{"labels":{"extra-label2": "my_value"}}}}}'

And it works well:

$ kubectl patch statefulset web \
  --type merge \
  -p '{"spec":{"template":{"metadata":{"labels":{"extra-label2": "my_value"}}}}}'
statefulset.apps/web patched


$ k describe sts web
Name:               web
Namespace:          default
Selector:           app=nginx
Labels:             <none>
Annotations:        <none>
Replicas:           1 desired | 1 total
Update Strategy:    RollingUpdate
  Partition:        0
Pods Status:        1 Running / 0 Waiting / 0 Succeeded / 0 Failed
Pod Template:
  Labels:  app=nginx
           extra-label2=my_value
           extra_label=test
  Containers:

@mglotov
Copy link
Author

mglotov commented Oct 6, 2023

@burmanm Hello. Any updates here?

@burmanm
Copy link
Contributor

burmanm commented Oct 6, 2023

Not really, the only ticket I found from Kubernetes is that it shouldn't work which makes me wonder if there's a difference between different Kubernetes versions or if the merge strategy is different in our client (but I don't have time today to test that).

We don't have different code for annotations and labels processing in PodTemplateSpec (so we would apply those changes the same way). So for whatever reason the Kubernetes didn't pick up the change.

@mglotov
Copy link
Author

mglotov commented Oct 6, 2023

Ok. Thank you for your reply.
Just to clarify. I tested the code above in the same k8s cluster where we run cass-operator. So, it works with my test STS, but doesn't with cass-operator

@adejanovski adejanovski added ready-for-review Issues in the state 'ready-for-review' review Issues in the state 'review' and removed ready-for-review Issues in the state 'ready-for-review' labels May 3, 2024
@adejanovski adejanovski added done Issues in the state 'done' and removed review Issues in the state 'review' labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working done Issues in the state 'done'
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants