Add support for deployment annotations #582
Conversation
|
Refactored the helper method to the new approach introduced in #584. Also added a test. Noted that a new line is added in front of the annotations (both on deployment and pod level), but did not manage to remove it although the setup looks the same as for labels. Maybe you can take a quick look @robholland ? |
| @@ -3,6 +3,10 @@ apiVersion: apps/v1 | |||
| kind: Deployment | |||
| metadata: | |||
| name: {{ include "temporal.componentname" (list $ "web") }} | |||
| {{- if or $.Values.additionalAnnotations $.Values.admintools.deploymentAnnotations }} | |||
There was a problem hiding this comment.
| {{- if or $.Values.additionalAnnotations $.Values.admintools.deploymentAnnotations }} | |
| {{- if or $.Values.additionalAnnotations $.Values.web.deploymentAnnotations }} |
There was a problem hiding this comment.
On seconds thoughts lets remove the gating.
There was a problem hiding this comment.
Removed the gating.
| {{- end -}} | ||
|
|
||
| {{/* | ||
| Additonal user specified annotations for all resources |
There was a problem hiding this comment.
Couldn't this just be inlined above? It's never called on it's own.
There was a problem hiding this comment.
Just noticed the label version is split out. That should be inlined also, but don't worry about that for this PR.
There was a problem hiding this comment.
It was indeed intended to align with the approach on the labels. But agree that inline is better, because it's not called on it's own.
| @@ -3,6 +3,10 @@ apiVersion: apps/v1 | |||
| kind: Deployment | |||
| metadata: | |||
| name: {{ include "temporal.componentname" (list $ "admintools") }} | |||
| {{- if or $.Values.additionalAnnotations $.Values.admintools.deploymentAnnotations }} | |||
There was a problem hiding this comment.
Let's remove the gating, we can just allow annotations: to be empty, it's valid syntax.
There was a problem hiding this comment.
Removed the gating.
|
|
||
| func TestTemplateServerDeploymentAnnotations(t *testing.T) { | ||
| // t.Parallel() | ||
|
|
||
| helmChartPath, err := filepath.Abs("../") | ||
| releaseName := "temporal" | ||
| require.NoError(t, err) | ||
|
|
||
| namespaceName := "temporal-" + strings.ToLower(random.UniqueId()) | ||
|
|
||
| var deployment appsv1.Deployment | ||
|
|
||
| options := &helm.Options{ | ||
| SetValues: map[string]string{ | ||
| "server.frontend.deploymentAnnotations.one": "three", | ||
| "server.frontend.deploymentAnnotations.four": "four", | ||
| "server.deploymentAnnotations.one": "one", | ||
| "server.deploymentAnnotations.two": "two", | ||
| }, | ||
| KubectlOptions: k8s.NewKubectlOptions("", "", namespaceName), | ||
| BuildDependencies: true, | ||
| } | ||
|
|
||
| output := helm.RenderTemplate(t, options, helmChartPath, releaseName, []string{"templates/server-deployment.yaml"}) | ||
|
|
||
| helm.UnmarshalK8SYaml(t, output, &deployment) | ||
|
|
||
| require.Equal(t, "three", deployment.ObjectMeta.Annotations["one"]) | ||
| require.Equal(t, "two", deployment.ObjectMeta.Annotations["two"]) | ||
| require.Equal(t, "four", deployment.ObjectMeta.Annotations["four"]) | ||
| } |
There was a problem hiding this comment.
Did add a value test on the additional annotations as well.
|
Please merge master to get the CI fix. |
* feat: add deployment annotations support
What was changed
This PR adds support to define deployment annotations for the
server,admintoolsandwebcomponents. The setup is similar to the recent implemented resource labels.NOTE: this PR partly replaces #537, however this PR needs revision because of the templatization in PR #539.
Why?
Currently there is no option to define deployment annotations.
Checklist
helm install -f values.yaml --set server.deploymentAnnotations.deployAnnotation1="serverDeploymentAnnotation" --set admintools.deploymentAnnotations.deployAnnotation1="adminDeploymentAnnotation" --set web.deploymentAnnotations.deployAnnotation1="webDeploymentAnnotation" debug . --dry-run --debugdeployAnnotation1is added to all server services and admintools and web components.helm install -f values.yaml --set server.deploymentAnnotations.serverAnnotation1="serverDeploymentAnnotation" --set server.frontend.deploymentAnnotations.resourceAnnotation1="resourceDeploymentAnnotation" debug . --dry-run --debugfrontendservice.resourceAnnotation1is added to the serverfrontendservice. The deployment annotations on server level are ignored correctly.serverAnnotation1is added to the other server services (history,matching,worker).No.