This repository has been archived by the owner on Aug 25, 2021. It is now read-only.
Add server-acl-init-cleanup job #246
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lkysow
force-pushed
the
acl-hook
branch
3 times, most recently
from
66cea28 to
fb36532
Compare
ishustava
reviewed
Oct 11, 2019
Comment on lines
30
to
34
| - apiGroups: [""] | ||
| resources: | ||
| - services | ||
| verbs: | ||
| - get |
There was a problem hiding this comment.
I'm curious why do we need services here as well?
There was a problem hiding this comment.
I totally don't remember adding this so I'll delete it. Good catch. But it's interesting that we actually do make one call for Services:
kubeSvc, err = c.clientset.CoreV1().Services("default").Get("kubernetes", metav1.GetOptions{})
However that must have been working before. Looking that code now, I'm not sure why it doesn't just use kubernetes.default.svc.cluster.local instead of the IP address (which is the whole reason it's calling this in the first place).
There was a problem hiding this comment.
Yep, it's calling it inside the configureConnectInject function, but the helm chart already adds services in case connect is enabled.
This job deletes the server-acl-init job when it completes successfully. This keeps things clean and more importantly fixes the issue where if you try to make a change to the helm values that result in updating the spec for the Job that you get an error because the Job spec is immutable. If the Job is deleted then this isn't a problem. Also upgrades consul-k8s to the version with the new delete-completed-job command.
ishustava
approved these changes
Oct 15, 2019
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This job deletes the server-acl-init job when it completes successfully.
This keeps things clean and more importantly fixes the issue where if
you try to make a change to the helm values that result in updating the
spec for the Job that you get an error because the Job spec is
immutable. If the Job is deleted then this isn't a problem.
Also upgrades consul-k8s to the version with the new
delete-completed-job command.
Fixes #227
Needs hashicorp/consul-k8s#152