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

Generation of Kubernetes Well-Known and Recommended labels #1700

Closed
manusa opened this issue Aug 11, 2022 · 8 comments · Fixed by #2534 or #2587
Closed

Generation of Kubernetes Well-Known and Recommended labels #1700

manusa opened this issue Aug 11, 2022 · 8 comments · Fixed by #2534 or #2587
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@manusa
Copy link
Member

manusa commented Aug 11, 2022

Component

JKube Kit

Is your enhancement related to a problem? Please describe

The generated YAML files should be (optionally) enriched with the:

Describe the solution you'd like

(High level)

  • Create a New (disableable) Enricher that adds the Recommended K8s labels
  • Make the existent ProjectLabelEnricher disableable (

Describe alternatives you've considered

Fragments

Additional context

No response

@manusa manusa added the enhancement New feature or request label Aug 11, 2022
@rohanKanojia rohanKanojia self-assigned this Dec 29, 2023
@rohanKanojia
Copy link
Member

I see a lot of labels and annotations on Well-Known Labels, Annotations and Taints page. Do we want to support all of them?

I think we can move forward with adding these:

Labels:

  • app.kubernetes.io/component
  • app.kubernetes.io/instance
  • app.kubernetes.io/managed-by
  • app.kubernetes.io/name
  • app.kubernetes.io/part-of
  • app.kubernetes.io/version

@manusa
Copy link
Member Author

manusa commented Jan 2, 2024

Do we want to support all of them?

At first, mostly those that we can infer. i.e. those that replace the jkube/fabric8-custom labels

@ajeans
Copy link

ajeans commented Jan 18, 2024

Good evening @rohanKanojia and @manusa

Unfortunately it seems that this change has regressed my maven jkube setup, I just noticed today after pulling the latest 1.16 snapshot.

I currently have the following in my pom.xml

            <plugin>
                <groupId>org.eclipse.jkube</groupId>
                <artifactId>kubernetes-maven-plugin</artifactId>
                <version>${jkube.version}</version>
                <configuration>
                    <resources>
                        <labels>
                            <all>
                                <team>rfn</team>
                                <language>java</language>
                                <property>
                                    <name>app.kubernetes.io/name</name>
                                    <value>${project.artifactId}</value>
                                </property>
                                <property>
                                    <name>app.kubernetes.io/version</name>
                                    <value>${project.version}</value>
                                </property>
                                <property>
                                    <name>app.kubernetes.io/component</name>
                                    <value>application</value>
                                </property>
                                <property>
                                    <name>app.kubernetes.io/managed-by</name>
                                    <value>Helm</value>
                                </property>
                                <property>
                                    <name>meta.helm.sh/release-name</name>
                                    <value>${project.artifactId}</value>
                                </property>
                            </all>
                        </labels>
                    </resources>
                    <helm>
                        <parameters>
                            <!-- Override with helm param -->
                            <parameter>
                                <name>common.namespace</name>
                                <value>{{ .Release.Namespace }}</value>
                            </parameter>
                        </parameters>
                    </helm>
                </configuration>
            </plugin>

Note how I override app.kubernetes.io/managed-by to have value "Helm".

Up until a few days ago, this generated a deployment file looking like

---
apiVersion: apps/v1
kind: Deployment
spec:
  selector:
    matchLabels:
      app: rfn-yellow-adapter-app
      group: com.rakuten
      provider: jkube
  template:
    metadata:
      labels:
        app: rfn-yellow-adapter-app
        app.kubernetes.io/component: application
        app.kubernetes.io/managed-by: Helm
        app.kubernetes.io/name: rfn-yellow-adapter-app
        app.kubernetes.io/version: 1.1.0
        group: com.rakuten
        language: java
        meta.helm.sh/release-name: rfn-yellow-adapter-app
        provider: jkube
        team: rfn
        version: 1.1.0

And everything worked fine when installing the helm chart.

However, with the latest snapshot I get

---
apiVersion: apps/v1
kind: Deployment
spec:
  selector:
    matchLabels:
      app: rfn-yellow-adapter-app
      app.kubernetes.io/managed-by: jkube
      app.kubernetes.io/name: rfn-yellow-adapter-app
      app.kubernetes.io/part-of: com.rakuten
      group: com.rakuten
      provider: jkube
  template:
    metadata:
      labels:
        app: rfn-yellow-adapter-app
        app.kubernetes.io/component: application
        app.kubernetes.io/managed-by: Helm
        app.kubernetes.io/name: rfn-yellow-adapter-app
        app.kubernetes.io/version: 1.1.0
        group: com.rakuten
        language: java
        meta.helm.sh/release-name: rfn-yellow-adapter-app
        provider: jkube
        team: rfn
        version: 1.1.0

And now helm installfails correctly with

Error: INSTALLATION FAILED: 1 error occurred:
        * Deployment.apps "rfn-yellow-adapter-app" is invalid: spec.template.metadata.labels: Invalid value: map[string]string{"app":"rfn-yellow-adapter-app", "app.kubernetes.io/component":"application", "app.kubernetes.io/managed-by":"Helm", "app.kubernetes.io/name":"rfn-yellow-adapter-app", "app.kubernetes.io/part-of":"com.rakuten", "app.kubernetes.io/version":"1.1.0", "group":"com.rakuten", "language":"java", "meta.helm.sh/release-name":"rfn-yellow-adapter-app", "provider":"jkube", "team":"rfn", "version":"1.1.0"}: `selector` does not match template `labels`

selector does not match template labels

Because there is a mismatch: the selector tries to match app.kubernetes.io/managed-by: jkube whereas the pods will have label app.kubernetes.io/managed-by: Helm.

My expectation would be that overriding the default value with <labels><all><property> should also propagate to the matching labels.

I can comment this override for now (I am not blocked), but I would rather have my helm-managed resources be marked as being managed by helm, if that works for you.

I commented directly in this closed issue because I think I tracked down the origin, but I can absolutely open a new issue if you prefer.

Best regards

CC @amihalfa

@manusa manusa reopened this Jan 18, 2024
@manusa
Copy link
Member Author

manusa commented Jan 18, 2024

Thanks for the detailed report 🙏
Indeed, it seems like a regression, awesome we got it before the stable release.
We'll try to fix ASAP.

@manusa
Copy link
Member Author

manusa commented Jan 19, 2024

By the way, in the meantime you can revert to the previous behavior by setting the following Maven property:

<jkube.enricher.jkube-well-known-labels.enabled>false</jkube.enricher.jkube-well-known-labels.enabled>

@rohanKanojia
Copy link
Member

WellKnownLabelsEnricher does not modify existing well-known labels/selectors if they already exist. Main difference I see between behavior of 1.15.0 and 1.16-SNAPSHOT is that now WellKnownLabelsEnricher adds selectors with app.kubernetes.io/ labels:
Screenshot_20240124_114059

When the user adds labels via XML configuration, they are not added to selectors. WellKnownEnricher during the create phase doesn't see any existing selector hence adds opinionated selectors.

We should modify WellKnownLabelsEnricher to also consider labels configured from XML configuration so that it doesn't collide with opinionated labels.

@rohanKanojia
Copy link
Member

@ajeans : Hello, we've merged a fix for the problem you were facing in #1700 (comment)

Could you please give 1.16-SNAPSHOT a try again and give feedback if it's working for your project now?

@ajeans
Copy link

ajeans commented Feb 8, 2024

@rohanKanojia Hello, I can confirm this fixes my set up, my label configuration overrides correctly.

Thanks for fixing this quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment