-
Notifications
You must be signed in to change notification settings - Fork 275
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
helm: make all images configurable #260
Conversation
Welcome @Annegies! |
Hi @Annegies. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign @aramase |
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.
Thank you for the PR!
Added few comments. Could you also add the new fields to Configuration table in the readme?
charts/secrets-store-csi-driver/templates/secrets-store-csi-driver-windows.yaml
Outdated
Show resolved
Hide resolved
/retitle helm: make all images configurable |
Thanks for the review, resolved your comments |
This PR might make sense to be part of the |
I don't mind, we'll use our own chart until then! |
@Annegies #258 has been merged. Can you update the PR to make all the changes for helm chart in manifest_staging? |
Applied the changes to the chart in manifest_staging |
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.
@Annegies Added few small nits. PTAL when you get a chance. Could you also squash to a single commit once the changes are done?
Resolved your comments and squashed the commit |
/ok-to-test |
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.
@Annegies Just one small nit and then we are good to merge
author Annegies van 't Zand <ace.vtzand@gmail.com> 1595940905 +0200 committer Annegies van 't Zand <ace.vtzand@gmail.com> 1596521061 +0200 Make all images configurable in order to use private registries fix references Rename livenessProbeImage and add to configuration table Apply changes in the manifest_staging Add fields to configuration Make private registries possible Make all images configurable in order to use private registries rename register Make all images configurable in order to use private registries
Again, resolved |
/test pull-secrets-store-csi-driver-e2e-windows |
1 similar comment
/test pull-secrets-store-csi-driver-e2e-windows |
The windows test job template needs to be updated. Will re-run the windows test once that update is complete. |
/test pull-secrets-store-csi-driver-e2e-windows |
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
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Annegies, ritazh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/test pull-secrets-store-csi-driver-e2e-azure |
What this PR does / why we need it:
This PR allows to configure all the image repositories if needed. For our container platfom only containers from private registries are allowed.
fixes #255