-
Notifications
You must be signed in to change notification settings - Fork 2
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
allow ownership job to fail #1201
Conversation
@@ -39,6 +39,6 @@ spec: | |||
name=$(kubectl get chart --namespace=giantswarm -l app.kubernetes.io/name=prometheus-meta-operator --output=jsonpath="{.items[0].spec.name}") | |||
namespace=$(kubectl get chart --namespace=giantswarm -l app.kubernetes.io/name=prometheus-meta-operator --output=jsonpath="{.items[0].spec.namespace}") | |||
kubectl patch --type=merge --record=false -p '{"metadata":{"annotations":{"meta.helm.sh/release-name": "'"${name}"'","meta.helm.sh/release-namespace": "'"${namespace}"'"},"labels":{"app.kubernetes.io/managed-by": "Helm"}}}' -n "${namespace}" alertmanager alertmanager | |||
kubectl patch --record=false -n "${namespace}" --type json --patch '[{ "op": "remove", "path": "/spec/alertmanagerConfigSelector/matchLabels/app.kubernetes.io~1managed-by", "value": "prometheus-meta-operator" }]' alertmanager alertmanager |
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.
Could you not check if the field exist instead?
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.
this will require either jq
or yq
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.
do you think it's installed in giantswarm/docker-kubect
?
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 honestly have no idea but I think the script would be more proper because this basically ignores this step even if it fails for other reasons
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.
Could you not use grep though?
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.
How is it now?
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.
LGTM if tested
13afb23
to
41b32d4
Compare
41b32d4
to
8e22097
Compare
The job also patch the Alertmanager resource to removes the label
managed-by: pmo
from
alertmanagerConfigSelector
.If the field doesn't has the label, the job will fail. so deployment will be stuck