Issue 4394 - custom filter for user labels

[echernous]

Type of change

• Enhancement / new feature

Description

Addresses the following issue #4394
This change allows strimzi users to control which parent CR labels they want to be used in subresources by setting the "STRIMZI_LABELS_EXCLUSION_PATTERN" environment variable (strimzi cluster operator deployment) with an exclusion regex pattern as its value, for example:

apiVersion: apps/v1
kind: Deployment
metadata:
  name: strimzi-cluster-operator
spec:
  template:
    spec:
      containers:
        - name: strimzi-cluster-operator
          env:
            - name: STRIMZI_LABELS_EXCLUSION_PATTERN
              value: "^key1.*"
....

The exclusion pattern will only be used to filter labels when parent CR labels are being assigned to its subresources. Strimzi or templates metadata such as spec.kafka.template.pod.metadata.labels labels won't be affected.
Default exclusion pattern is "^app.kubernetes.io/(?!part-of).*" which matches previously configured filtering to minimise impact.

Checklist

• Write tests
• Make sure all tests pass
• Update documentation
• Reference relevant issue(s) and close them after merging
• Update CHANGELOG.md

[strimzi-ci]

Can one of the admins verify this patch?

[scholzj]

@echernous We discussed this today at the Strimzi Community Call. The outcome was that this should be preferably controlled by an environment variable in the operator deployments and not through an annotation like you have it.

[echernous]

Thanks @scholzj, I will make the change and test it today, will let you know how it goes

[echernous]

@scholzj I applied the change and moved the filtering into fromResource so user specified regex won't affect non-CR labels, and the default exclusion pattern testing is covered in testFromResourceWithLabels

[scholzj]

Left few nits. Maybe you could also add the new variable to the docs: https://github.com/strimzi/strimzi-kafka-operator/blob/main/documentation/modules/operators/ref-operator-cluster.adoc

[echernous]

Hey @scholzj, thanks for the review, I fixed the filtering using matcher and updated the doc

[echernous]

/azp run regression

[azure-pipelines]

Commenter does not have sufficient privileges for PR 4791 in repo strimzi/strimzi-kafka-operator

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

@echernous Can you look at the regression test failures? It looks like KafkaST.testLabelsAndAnnotationForPVC might be related to this. KafkaST.testLabelModificationDoesNotBreakCluster was flaky - that might have been just coincidence.

[echernous]

Hi @scholzj , I ran the test locally and no issues found, also checked that it doesn't use the changed labels method. Pls could you trigger it again to see how it goes? Thank you!

Here are some logs (removed some long output):
2021-04-30 01:25:40 [main] INFO [TestSeparator:29] ############################################################################
2021-04-30 01:25:40 [main] INFO [TestSeparator:30] io.strimzi.systemtest.kafka.KafkaST.testLabelsAndAnnotationForPVC-STARTED
2021-04-30 01:25:40 [main] INFO [AbstractST:751] Not first test we are gonna generate cluster name
2021-04-30 01:25:40 [main] INFO [TestKafkaVersion:44] These are following supported Kafka versions:
[KafkaVersion{version='2.6.0', protocolVersion='2.6', messageVersion='2.6', zookeeperVersion='3.5.8', isDefault=false, isSupported=true}, KafkaVersion{version='2.6.1', protocolVersion='2.6', messageVersion='2.6', zookeeperVersion='3.5.8', isDefault=false, isSupported=true}, KafkaVersion{version='2.7.0', protocolVersion='2.7', messageVersion='2.7', zookeeperVersion='3.5.8', isDefault=true, isSupported=true}]
2021-04-30 01:25:40 [main] INFO [ResourceManager:145] Create/Update of Kafka my-cluster-1580438267 in namespace kafka-cluster-test
2021-04-30 01:25:40 [main] WARN [VersionUsageUtils:60] The client is using resource type 'kafkas' with unstable version 'v1beta2'
2021-04-30 01:25:40 [main] INFO [ResourceManager:309] Wait for Kafka: my-cluster-1580438267 will have desired state: Ready
2021-04-30 01:25:40 [main] INFO [TestUtils:134] Exception waiting for Wait for Kafka: my-cluster-1580438267 will have desired state: Ready, null
2021-04-30 01:28:41 [main] INFO [ResourceManager:320] Kafka: my-cluster-1580438267 is in desired state: Ready
2021-04-30 01:28:41 [main] INFO [ResourceManager:197] Resource my-cluster-1580438267 in namespace kafka-cluster-test is io.strimzi.systemtest.resources.ResourceManager$$Lambda$535/0x00000008007d7040@43f670f3!
2021-04-30 01:28:41 [main] INFO [KafkaST:1517] Check if Kubernetes labels are applied
2021-04-30 01:28:41 [main] INFO [KafkaST:1521] Kubernetes labels are correctly set and present
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-0-my-cluster-1580438267-kafka-0 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-0-my-cluster-1580438267-kafka-1 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-0-my-cluster-1580438267-kafka-2 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-1-my-cluster-1580438267-kafka-0 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-1-my-cluster-1580438267-kafka-1 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-1-my-cluster-1580438267-kafka-2 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1529] Verifying that PVC label data-my-cluster-1580438267-zookeeper-0 - testValue = testValue
2021-04-30 01:28:41 [main] INFO [KafkaST:1539] Replacing kafka && zookeeper labels and annotations from testKey to editedTestValue
2021-04-30 01:28:42 [main] INFO [PersistentVolumeClaimUtils:32] Wait until PVC labels will change {testKey=editedTestValue}
2021-04-30 01:29:08 [main] INFO [PersistentVolumeClaimUtils:48] PVC labels has changed {testKey=editedTestValue}
2021-04-30 01:29:08 [main] INFO [PersistentVolumeClaimUtils:52] Wait until PVC annotation will change {testKey=editedTestValue}
2021-04-30 01:29:08 [main] INFO [PersistentVolumeClaimUtils:68] PVC annotation has changed {testKey=editedTestValue}
2021-04-30 01:29:08 [main] INFO [ResourceManager:309] Wait for Kafka: my-cluster-1580438267 will have desired state: Ready
2021-04-30 01:29:08 [main] INFO [ResourceManager:320] Kafka: my-cluster-1580438267 is in desired state: Ready
2021-04-30 01:29:08 [main] INFO [KafkaST:1553] [PersistentVolumeClaim(apiVersion=v1, kind=PersistentVolumeClaim, ... removed as it's too long
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-0-my-cluster-1580438267-kafka-0 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-0-my-cluster-1580438267-kafka-1 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-0-my-cluster-1580438267-kafka-2 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-1-my-cluster-1580438267-kafka-0 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-1-my-cluster-1580438267-kafka-1 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-1-my-cluster-1580438267-kafka-2 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [KafkaST:1558] Verifying replaced PVC label data-my-cluster-1580438267-zookeeper-0 - testValue = editedTestValue
2021-04-30 01:29:08 [main] INFO [ResourceManager:238] ############################################################################
2021-04-30 01:29:08 [main] INFO [ResourceManager:239] Going to clear all resources for testLabelsAndAnnotationForPVC
2021-04-30 01:29:08 [main] INFO [ResourceManager:240] ############################################################################
2021-04-30 01:29:08 [main] INFO [ResourceManager:174] Delete of Kafka my-cluster-1580438267 in namespace kafka-cluster-test
2021-04-30 01:29:13 [main] INFO [ResourceManager:197] Resource my-cluster-1580438267 in namespace kafka-cluster-test is io.strimzi.systemtest.resources.ResourceManager$$Lambda$590/0x00000008008e7040@35d128fd!
2021-04-30 01:29:13 [main] INFO [ResourceManager:247] ############################################################################
2021-04-30 01:29:13 [main] WARN [VersionUsageUtils:60] The client is using resource type 'kafkatopics' with unstable version 'v1beta2'
2021-04-30 01:29:14 [main] INFO [TimeMeasuringSystem:123] End time of operation TEST_EXECUTION is correctly stored
2021-04-30 01:29:14 [main] INFO [TestSeparator:36] io.strimzi.systemtest.kafka.KafkaST.testLabelsAndAnnotationForPVC-FINISHED

[scholzj]

Ok, I restarted it, so lets see if it helps.

[echernous]

@scholzj , sorry I didn't run the test properly last time, now should be fixed. As statefulset labels specified under template are not being filtered anymore the test was failing.

[scholzj]

/azp run regression

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[scholzj]

Wait, so ... what was the problem? It looks like you just changed the test, but that means something changed in the code and it behaves differently now with the default settings?

[echernous]

Yes, it doesn't filter labels under template sections anymore (it still will throw an exception if labels start with strimzi domain), STRIMZI_LABELS_EXCLUSION_PATTERN is applied only to labels that are specified in metadata of a parent CR.

The current implementation
Kubernetes resources will have user labels from parent CR's metadata.labels (Labels.fromResource method is used here) and custom labels configured using templates such as spec.kafka.template.statefulset.metadata.labels (Labels.withAdditionalLabels is used here). CR's and template's labels are getting filtered by the following rules:

• deletes labels that start with app.kubernetes.io/ except app.kubernetes.io/part-of
• throws an exception if labels start with strimzi.io/ except strimzi.io/cluster

After this change
Users will be able to control parent CR's labels propagation, and use templates for custom labels without filtering.
Kubernetes resources will have user labels from parent CR's metadata.labels filtered by:

• deletes labels if they match the STRIMZI_LABELS_EXCLUSION_PATTERN regex
• throws an exception if labels start with strimzi.io/ except strimzi.io/cluster

Additionally kubernetes resources will have custom configured labels using templates which are filtered by:

• throws an exception if labels start with strimzi.io/ except strimzi.io/cluster

The testLabelsAndAnnotationForPVC system test includes verification of Statefulset labels assigned using template, these custom labels are not filtered anymore.

[scholzj]

So does that impact all templates? Or only the PVC template? I wonder if this new behaviour is ok for us or not ... it is a change from how it behaved before.

@strimzi/maintainers WDYT?

[echernous]

Hi @scholzj , yes, it will affect all templates. However, I believe impact on labels that are assigned through templates would be minimum as only app.kubernetes.io/* labels are filtered out right now, and users will be able to assign custom kubernetes labels to kafka subresources.

[Frawless]

so maybe the default regex for the filter should be empty? In that case, the behaviour should remain as it was if I understand correctly

[scholzj]

@Frawless No, if I understood it correctly, the previous behavior was that these were always filtered out? And now they are used regardless the regex?

[Frawless]

In that case, we should keep the behavior I think. Like always extend the regex with this default value for kube-specific labels.

[echernous]

In this change the regex doesn't affect labels in templates as they are custom labels specified per resource, there is no propagation to multiple resources so no need to filter (users can just remove unwanted label from template configuration). The regex is applied to Kafka metadata labels to control which labels should be applied to its subresources.