openshift-routes
openshift-routes copied to clipboard
Helm: add `omitHelmLabels` so that people can generate static manifests without the Helm-specific labels
In https://github.com/cert-manager/openshift-routes/issues/73, I wondered how users could continue using a static manifests (e.g., for ArgoCD using the "static manifests" pattern). In this PR, I propose to add a Helm value to help with generating a static manifest free of Helm-specific labels.
To generate a static manifest free of Helm-specific labels, you will now be able to run:
helm template deploy/charts/openshift-routes/ --set omitHelmLabels=true
The only two labels and labelSelectors left are the three non-Helm-specific labels:
labels:
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
app.kubernetes.io/version: v0.6.0-alpha.0
Here are the labels that get removed when using --set omitHelmLabels=true:
make helm-chart VERSION=v0.6.0-alpha.0
diff -u \
<(helm template --show-only templates/deployment.yaml _bin/scratch/image/openshift-routes-0.6.0-alpha.0.tgz) \
<(helm template --show-only templates/deployment.yaml _bin/scratch/image/openshift-routes-0.6.0-alpha.0.tgz --set omitHelmLabels=true)
--- helm template --show-only templates/deployment.yaml
+++ helm template --show-only templates/deployment.yaml --set omitHelmLabels=true
@@ -9,9 +9,6 @@
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
app.kubernetes.io/version: 0.6.0-alpha.0
- app.kubernetes.io/instance: release-name
- app.kubernetes.io/managed-by: Helm
- helm.sh/chart: openshift-routes-0.6.0-alpha.0
spec:
replicas: 1
selector:
@@ -19,14 +16,12 @@
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
app.kubernetes.io/version: 0.6.0-alpha.0
- app.kubernetes.io/instance: release-name
template:
metadata:
labels:
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
app.kubernetes.io/version: 0.6.0-alpha.0
- app.kubernetes.io/instance: release-name
spec:
automountServiceAccountToken: true
serviceAccountName: release-name-openshift-routes
Difference between the old v0.5.0 static manifest and the static manifest you would get using --set omitHelmLabels=true:
make helm-chart VERSION=v0.6.0-alpha.0
diff -u \
<(curl -sSL https://github.com/cert-manager/openshift-routes/releases/download/v0.5.0/cert-manager-openshift-routes.yaml) \
<(helm template openshift-routes -n cert-manager _bin/scratch/image/openshift-routes-0.6.0-alpha.0.tgz --set omitHelmLabels=true)
--- curl -sSL https://github.com/cert-manager/openshift-routes/releases/download/v0.5.0/cert-manager-openshift-routes.yaml
+++ helm template openshift-routes -n cert-manager _bin/scratch/image/openshift-routes-0.6.0-alpha.0.tgz --set omitHelmLabels=true
@@ -1,8 +1,25 @@
---
+# Source: openshift-routes/templates/serviceaccount.yaml
+apiVersion: v1
+kind: ServiceAccount
+metadata:
+ name: openshift-routes
+ namespace: cert-manager
+ labels:
+ app.kubernetes.io/name: openshift-routes
+ app.kubernetes.io/component: controller
+ app.kubernetes.io/version: 0.6.0-alpha.0
+automountServiceAccountToken: false
+---
+# Source: openshift-routes/templates/rbac.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRole
metadata:
- name: cert-manager-openshift-routes
+ name: openshift-routes
+ labels:
+ app.kubernetes.io/name: openshift-routes
+ app.kubernetes.io/component: controller
+ app.kubernetes.io/version: 0.6.0-alpha.0
rules:
- apiGroups:
- route.openshift.io
@@ -61,66 +78,71 @@
- list
- update
---
-apiVersion: v1
-kind: ServiceAccount
-metadata:
- name: cert-manager-openshift-routes
- namespace: cert-manager
-automountServiceAccountToken: false
----
+# Source: openshift-routes/templates/rbac.yaml
apiVersion: rbac.authorization.k8s.io/v1
kind: ClusterRoleBinding
metadata:
- name: cert-manager-openshift-routes
+ name: openshift-routes
+ labels:
+ app.kubernetes.io/name: openshift-routes
+ app.kubernetes.io/component: controller
+ app.kubernetes.io/version: 0.6.0-alpha.0
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: ClusterRole
- name: cert-manager-openshift-routes
+ name: openshift-routes
subjects:
- kind: ServiceAccount
- name: cert-manager-openshift-routes
+ name: openshift-routes
namespace: cert-manager
---
+# Source: openshift-routes/templates/deployment.yaml
apiVersion: apps/v1
kind: Deployment
metadata:
- name: cert-manager-openshift-routes
+ name: openshift-routes
namespace: cert-manager
labels:
- app.kubernetes.io/name: cert-manager-openshift-routes
- app.kubernetes.io/version: "0.5.0"
+ app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
- app.kubernetes.io/part-of: cert-manager
+ app.kubernetes.io/version: 0.6.0-alpha.0
spec:
replicas: 1
selector:
matchLabels:
- app.kubernetes.io/name: cert-manager-openshift-routes
- app.kubernetes.io/version: "0.5.0"
+ app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
- app.kubernetes.io/part-of: cert-manager
+ app.kubernetes.io/version: 0.6.0-alpha.0
template:
metadata:
labels:
- app.kubernetes.io/name: cert-manager-openshift-routes
- app.kubernetes.io/version: "0.5.0"
+ app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
- app.kubernetes.io/part-of: cert-manager
+ app.kubernetes.io/version: 0.6.0-alpha.0
spec:
- serviceAccountName: cert-manager-openshift-routes
automountServiceAccountToken: true
+ serviceAccountName: openshift-routes
+ securityContext:
+ runAsNonRoot: true
+ seccompProfile:
+ type: RuntimeDefault
containers:
- - name: cert-manager-openshift-routes
- image: "ghcr.io/cert-manager/cert-manager-openshift-routes:0.5.0"
+ - name: openshift-routes
+ securityContext:
+ allowPrivilegeEscalation: false
+ capabilities:
+ drop:
+ - ALL
+ readOnlyRootFilesystem: true
+ image: "ghcr.io/cert-manager/cert-manager-openshift-routes:v0.6.0-alpha.0"
+ imagePullPolicy: IfNotPresent
args:
- - -v=5
+ - "-v=5"
+ - "--leader-election-namespace=cert-manager"
ports:
- containerPort: 6060
name: readiness
protocol: TCP
- - containerPort: 9402
- name: metrics
- protocol: TCP
readinessProbe:
httpGet:
port: readiness
@@ -128,3 +150,7 @@
initialDelaySeconds: 3
periodSeconds: 5
timeoutSeconds: 3
+ resources:
+ {}
+ nodeSelector:
+ kubernetes.io/os: linux
cert-manager has a similar value: https://github.com/cert-manager/cert-manager/blob/master/deploy/charts/cert-manager/values.yaml#L1380-L1384
Talked during the open standup:
I noticed that the label
app.kubernetes.io/version: "0.5.0"
got removed. I will add it back using appVersion since it seems like a useful label.
Note that I noticed that some users prefer when the label selectors don't have a variable version: https://github.com/quarkusio/quarkus/issues/11070. But that seems to be only when a Pod is used directly since pod specs are immutable. In Openshift-routes, we use deployments which means this isn't a problem.
I noticed some discrepancies with the image tag pattern in v0.5.0 and in v0.6.0:
-image: "ghcr.io/cert-manager/cert-manager-openshift-routes:0.5.0"
+image: "ghcr.io/cert-manager/cert-manager-openshift-routes:v0.6.0"
It looks like the v was added to the image tags after #60 got merged:
Is that intended? I guess I'm OK with this change, but isn't this going to break anyone? @inteon
I noticed some discrepancies with the image tag pattern in v0.5.0 and in v0.6.0.
Thanks for noticing this. I did not notice that this was how the current images were versioned and that it differs now. I do believe it makes sense to continue using this new pattern. It only makes sense to use the non-v prefix for Helm charts, not for the OCI image.
I agree. I'll give notice to the users of openshift-routes just to make sure we are giving them a chance to complain before the change takes place. To that effect:
→ I created a GitHub issue and pinned it: https://github.com/cert-manager/openshift-routes/issues/75 → I published a message on Slack and pinged Vinny (only person I know who uses openshift-routes in the Kubernetes Slack). → I emailed Jack Henschel to ask what he thinks.
Talked during the open standup:
I noticed that the label
app.kubernetes.io/version: "0.5.0"got removed. I will add it back using
appVersionsince it seems like a useful label.Note that I noticed that some users prefer when the label selectors don't have a variable version: quarkusio/quarkus#11070. But that seems to be only when a Pod is used directly since pod specs are immutable. In Openshift-routes, we use deployments which means this isn't a problem.
The labelselector in a Deployment is also immutable:
# deployments.apps "openshift-routes" was not valid:
# * spec.template.metadata.labels: Invalid value: map[string]string{"app.kubernetes.io/component":"controller", "app.kubernetes.io/instance":"openshift-routes", "app.kubernetes.io/managed-by":"Helm", "app.kubernetes.io/name":"openshift-routes", "app.kubernetes.io/version":"0.6.0-alpha.0-20-gb36d5372ac760e", "helm.sh/chart":"openshift-routes-0.6.0-alpha.0-20-gb36d5372ac760e"}: `selector` does not match template `labels`
# * spec.selector: Invalid value: v1.LabelSelector{MatchLabels:map[string]string{"app.kubernetes.io/component":"controller88", "app.kubernetes.io/instance":"openshift-routes", "app.kubernetes.io/name":"openshift-routes"}, MatchExpressions:[]v1.LabelSelectorRequirement(nil)}: field is immutable
The labelselector in a Deployment is also immutable
You are right, I forgot about selector.matchLabels not being mutable on Deployment resources. I propose that we keep the version label anywhere else, and only skip it for the selector.matchLabels field.
The labelselector in a Deployment is also immutable
You are right, I forgot about
selector.matchLabelsnot being mutable on Deployment resources. I propose that we keep the version label anywhere else, and only skip it for theselector.matchLabelsfield.
Great, that is what I added here: https://github.com/cert-manager/openshift-routes/pull/74/commits/b36d5372ac760ee3a70414e02ce7abdcc3b0d0d1
Updated diff after applying the latest changes included in this PR:
make helm-chart VERSION=v0.6.0-alpha.0
diff -u \
<(helm template --show-only templates/deployment.yaml _bin/scratch/image/openshift-routes-0.6.0-alpha.0.tgz) \
<(helm template --show-only templates/deployment.yaml _bin/scratch/image/openshift-routes-0.6.0-alpha.0.tgz --set omitHelmLabels=true)
--- helm template --show-only templates/deployment.yaml
+++ helm template --show-only templates/deployment.yaml --set omitHelmLabels=true
@@ -8,26 +8,19 @@
labels:
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
- app.kubernetes.io/instance: release-name
app.kubernetes.io/version: 0.6.0-alpha.0
- app.kubernetes.io/managed-by: Helm
- helm.sh/chart: openshift-routes-0.6.0-alpha.0
spec:
replicas: 1
selector:
matchLabels:
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
- app.kubernetes.io/instance: release-name
template:
metadata:
labels:
app.kubernetes.io/name: openshift-routes
app.kubernetes.io/component: controller
- app.kubernetes.io/instance: release-name
app.kubernetes.io/version: 0.6.0-alpha.0
- app.kubernetes.io/managed-by: Helm
- helm.sh/chart: openshift-routes-0.6.0-alpha.0
spec:
automountServiceAccountToken: true
serviceAccountName: release-name-openshift-routes
Proof that we can upgrade using Helm from v0.5.0 to v0.6.0-...
$ helm install openshift-routes -n cert-manager oci://ghcr.io/cert-manager/charts/openshift-routes --version=0.5.0
$ make helm-chart VERSION=v0.6.0-alpha.1
$ helm upgrade openshift-routes -n cert-manager _bin/scratch/image/openshift-routes-0.6.0-alpha.1.tgz
Release "openshift-routes" has been upgraded. Happy Helming!
NAME: openshift-routes
LAST DEPLOYED: Mon Aug 12 16:53:08 2024
NAMESPACE: cert-manager
STATUS: deployed
REVISION: 5
TEST SUITE: None
Thanks for showing the diff output. I think this is good to go now.
/lgtm /approve /hold since there seems to be a discussion with Adam
@maelvls: you cannot LGTM your own PR.
In response to this:
Thanks for showing the diff output. I think this is good to go now.
/lgtm /approve /hold since there seems to be a discussion with Adam
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: maelvls
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [maelvls]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
I can't approve my own PR 😅 Can someone else LGTM, @ThatsMrTalbot maybe? Thanks.
/lgtm
/unhold