Skip to content
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

fix: extra pod specs for cra #116

Merged
merged 2 commits into from
Apr 17, 2024
Merged

fix: extra pod specs for cra #116

merged 2 commits into from
Apr 17, 2024

Conversation

soniqua
Copy link
Contributor

@soniqua soniqua commented Apr 11, 2024

Allows extraPodSpecs to be specified for Container Registry Agent

@CLAassistant
Copy link

CLAassistant commented Apr 11, 2024

CLA assistant check
All committers have signed the CLA.

@soniqua soniqua marked this pull request as ready for review April 11, 2024 15:27
@soniqua soniqua requested review from a team as code owners April 11, 2024 15:27
@pavel-snyk
Copy link
Contributor

@soniqua, could you add an helm unittest to verify usage of extraPodSpecsCr value (so the template is correctly rendered)?

Comment on lines +15 to +16
path: spec.template.spec
template: broker_deployment.yaml
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soniqua, nice usage instead of giga-snapshot :)

Copy link

@jonnyowenpowell jonnyowenpowell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Not a huge fan of the naming, this allows for extra fields in the PodSpec, but the name could be interpreted as extra distinct PodSpecs. However, appreciate it's existing and would be a breaking change to modify.

@soniqua soniqua merged commit 44efe7d into main Apr 17, 2024
4 checks passed
@soniqua soniqua deleted the fix/extra-pod-specs-cra branch April 17, 2024 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants