-
Notifications
You must be signed in to change notification settings - Fork 37
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
add support for 1.25 #361
add support for 1.25 #361
Conversation
Nice, have you tested this change out by spinning a 1.25 ? Want to make sure images are published and accessible etc. |
Yup they are tested
|
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.
One minor comment, rest LGTM!
@@ -51,6 +51,7 @@ apply: ## Apply the controller into your ~/.kube/config cluster | |||
$(HELM_OPTS) \ | |||
--set controller.image=ko://github.com/awslabs/kubernetes-iteration-toolkit/operator/cmd/controller \ | |||
--set webhook.image=ko://github.com/awslabs/kubernetes-iteration-toolkit/operator/cmd/webhook \ | |||
--set serviceAccount.create=true \ |
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.
what happens if SA already exists in the cluster and someone runs make apply
?
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.
It will work if it already exists and was created by the helm chart. If it was manually created though then helm will have a conflict with it.
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!
Issue #, if available:
Description of changes:
Adding support for 1.25
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.