-
Notifications
You must be signed in to change notification settings - Fork 205
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
[KEDA] Update helm values to work from v2.14.0 #1024
Conversation
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.
@vumdao Thanks for the PR. Have couple of comments.
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 this contribution!
podSecurityContextFsGroup
as well as security context fields: they are not supported by the version that is now the default version, let's add deprecation to these fields and a note that these have no effect for version 2.14 and above.
There is also impact to docs/addons/keda.md
, but you can defer it.
Other than that looks great.
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.
@vumdao Looks great.
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
/do-e2e-tests |
1 similar comment
/do-e2e-tests |
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.
end to end tests failed. A maintainer can provide more details.
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.
end to end tests failed. A maintainer can provide more details.
Failure in e2e unrelated to the changes here. I will create a PR to fix them and rerun. |
/do-e2e-tests |
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.
end to end tests passed
Issue:
Since v2.14.0, #625 updates some Helm chart values that are not backward compatible with the old settings. The serviceAccount has been separated per service, such as operator, metricServer, and webhooks.This causes the upgrade to v2.14.0+ to recreate the new operator serviceAccount without the IAM role annotation
Description of changes:
irsaRole
condition check to avoid CDK deletekeda
namespace if we change irsaRole[] to emptyserviceAccount
toserviceAccount.operator
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.