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

Modify labels configuration for litmus-core #363

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Calvinaud
Copy link
Contributor

  • Create seperate set of labels/labelSelector for the operator and exporter
  • Include the app label in the common label in helper.tpl
  • Update the helper.tpl so the label management is closer to the sub-chart of litmus-agent (the selector label are included in the list of label)

What this PR does / why we need it:

Update the labels configuration so that the labels for operator and exporter are not the same. The goal was that the service doesn't match both the exporter and operator at the same time. Which is the setup in the chaos-exporter sub-chart of litmus-agent.
I tried to match the configuration of the sub-chart of litmus-agent.

Which issue this PR fixes

(optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged)

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the README.md

Calvin Audier added 2 commits February 28, 2024 11:55
- Create seperate set of labels/labelSelector for the operator and exporter
- Include the app label in the common label in helper.tpl
- Update the helper.tpl so the label management is closer to the sub-chart of litmus-agent (the selector label are included in the list of label)

Signed-off-by: Calvin Audier <[email protected]>
@@ -2,7 +2,7 @@ apiVersion: v1
appVersion: "2.14.0"
description: A Helm chart to install litmus infra components on Kubernetes
name: litmus-core
version: 2.14.1
version: 2.14.2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

matchLabels changes are not backward compatibles

- Changed bump to 2.15.0 instead of 2.14.2 as per jouve comment
- Regenerating docs with helm-docs

Signed-off-by: Calvin Audier <[email protected]>
@v-vostretsov
Copy link

Hi guys
any updates on this one?

@uditgaurav
Copy link
Member

Hi @Calvinaud , Can you please update the version to fix conflicts.

@@ -23,8 +22,7 @@ kind: ClusterRoleBinding
metadata:
name: {{ include "litmus.fullname" . }}-admin
labels:
app: {{ template "litmus.name" . }}
{{- include "litmus.labels" . | indent 4 }}
{{- include "litmus.operatorLabels" . | indent 4 }}
Copy link
Member

@uditgaurav uditgaurav May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the labels on clusterrole and clusterrolebinding?

Copy link
Contributor Author

@Calvinaud Calvinaud May 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The labels were already present before this PR. Personally I still think that the labels are useful since it help to know at which application the clusterrole/clusterrolebinding are part of.

For the ****-admin, I think the question is more are they part of the "operator" but I would still said yes.

But if needed, I can remove them.

Bump version to 3.6.1

Signed-off-by: Calvin Audier <[email protected]>
@Calvinaud Calvinaud requested a review from Jonsy13 as a code owner May 16, 2024 07:15
Bump to version 3.7.1

Signed-off-by: Calvin Audier <[email protected]>
Signed-off-by: Calvin Audier <[email protected]>
@Calvinaud
Copy link
Contributor Author

Hi @Calvinaud , Can you please update the version to fix conflicts.

Updated by bumping version to 3.7.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ServiceMonitor misconfiguration
5 participants