-
Notifications
You must be signed in to change notification settings - Fork 5
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
Provide credentials for using during helm upgrade. #335
Conversation
🦋 Changeset detectedLatest commit: 1e8a479 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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 with minor comments
charts/kubernetes-agent/values.yaml
Outdated
@@ -322,7 +322,7 @@ persistence: | |||
initial_backoff_seconds: "" | |||
# -- The total time to retry failed NFS checks before giving up and deleting the pod | |||
# @section -- Persistence | |||
# @default 10 | |||
# @default 10:w! |
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.
# @default 10:w! | |
# @default 10 |
Looks like you accidentally stayed in insert mode 😅
charts/kubernetes-agent/values.yaml
Outdated
upgrade: | ||
username: "" | ||
password: "" | ||
registry: "" | ||
|
||
|
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.
upgrade: | |
username: "" | |
password: "" | |
registry: "" | |
upgrade: | |
dockerAuth: | |
username: "" | |
password: "" | |
registry: "" |
Consider changing the root from upgrade
- it's not very descriptive.
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.
yeah I figured agent.upgrade.dockerAuth was actually quite nice.
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.
Just 1 minor suggestion for the release notes, but LGTM!
Co-authored-by: Alastair Pitts <[email protected]>
Helm has a config file which contains credentials for the registries to which it is logged in (it can otherwise fallback on docker config).
This file is the same format as a docker config file.
Where registry (for dockerhub) should be "https://index.docker.io/v1".
And "auth" is a basic auth string - i.e. a base64 encoding of
<username>:<password>
While there are many more fields in this file, this is sufficient to get our e2e tests passing reliably.