-
-
Notifications
You must be signed in to change notification settings - Fork 117
support for custom st2actionrunner images via values.yaml #141
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
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.
👍
Note that a Changelog update is also needed to merge this.
|
@armab this is ready for review the following todo's
|
templates/_helpers.tpl
Outdated
| {{- end -}} | ||
|
|
||
| # Generate Docker image repository for st2actionrunner: Private 'docker.stackstorm.com' for Enterprise vs Public Docker Hub 'stackstorm' for FOSS version | ||
| {{- define "st2actionrunnerImageRepository" -}} |
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.
For simplicity reasons, it would be easier to avoid custom template for st2actionrunner, if possible. I saw the previous version, wondering if that worked good?
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.
Is this PR still needed? If so, can you rebase it or merge in master?
Enterprise stuff has been removed, so at least some of this can be simplified.
|
@cognifloyd will take a look at this, this week. |
|
#200 was just merged, so you can have custom image tags for any of the pods, including |
|
@Sheshagiri I looked into resolving the conflicts for you to merge master into your branch, but Maintainer edits are not turned on. Would you please merge in master? I'd love to merge this once we get it ready. |
|
@Sheshagiri I merged in master and addressed @armab's feedback by dropping the additional helper. |
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.
K. I added unit tests.
@armab would you please review this?
I changed quite a bit, so I need another pair of eyes before it gets merged.
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.
Nice!
I guess st2actionrunner is the image adopters would want to override more than other components.
|
Yeah that's what I figure. Thanks! |
This pr adds support for providing custom st2actionrunner-specific docker repository, image name, pull policy, and pull secret via
values.yaml.closes: #104