-
Notifications
You must be signed in to change notification settings - Fork 1
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
Annotations and Labels #4
Changes from all commits
3b1092d
80bdf5b
03ca741
8beb52e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,16 +2,22 @@ apiVersion: apps/v1 | |
kind: Deployment | ||
metadata: | ||
name: {{ include "workload.name" . }} | ||
annotations: | ||
{{- toYaml .Values.workload.annotations | nindent 4 }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We really should add these to the values.yaml file as sample defaults.
Why no annotations on the service? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. RE: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
No reasons, I missed it, just added, thanks for catching this. |
||
labels: | ||
{{- include "workload.labels" . | nindent 4 }} | ||
{{- include "workload.commonLabels" . | nindent 4 }} | ||
{{- toYaml .Values.workload.labels | nindent 4 }} | ||
spec: | ||
selector: | ||
matchLabels: | ||
{{- include "workload.selectorLabels" . | nindent 6 }} | ||
template: | ||
metadata: | ||
annotations: | ||
{{- toYaml .Values.workload.containers.annotations | nindent 8 }} | ||
labels: | ||
{{- include "workload.selectorLabels" . | nindent 8 }} | ||
{{- toYaml .Values.workload.containers.labels | nindent 8 }} | ||
spec: | ||
containers: | ||
{{- range $name, $container := .Values.containers }} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you provide more context on this change? Why is it needed? what does it benefit?
Are you going to add to the conversion function in https://github.com/score-spec/score-helm/blob/main/internal/helm/convert.go#L42C8-L42C8 to enable this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just updated the main description of this PR for the use cases. Let me know if you need more information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nop, it's not related to that. These
labels
andannotations
are not part of the Score spec anyway.This PR is about this workflow:
So just about the second command (
helm install
), not the first one (score-helm run
).Let me know if you need more information around this.