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.

Sign up for GitHub

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

Jump to bottom

Added more flexible volume variables #151

Merged
merged 3 commits into from
Mar 18, 2022
Merged

Added more flexible volume variables #151

merged 3 commits into from
Mar 18, 2022

Conversation

TylerHelmuth
Copy link
Member

This PR adds new extraVolumes and extraVolumeMounts variable and deprecates extraConfigMapMounts, extraHostPathMounts, and secretMounts.

Going forward, extraVolumes and extraVolumeMounts will allow more flexible volumes and volumeMounts by putting the configuration in the hands of the user.

Fixes #133

@TylerHelmuth TylerHelmuth requested review from a team March 17, 2022 22:31
@TylerHelmuth
Copy link
Member Author

TylerHelmuth commented Mar 18, 2022 •
edited
Loading

I've realized that we have no automated test that check that changes to the values.yaml like these are actually valid when they are used instead of their defaults of empty. I tested locally but would be good to run it through the workflow with real values.

Should one of our CI scenarios actually fill out every possible values.yaml setting?

@dmitryax
Copy link
Member

Should one of our CI scenarios actually fill out every possible values.yaml setting?

That would be useful. But they have be different from the default values. Not sure it's possible to cover all options within one scenario

dmitryax
dmitryax approved these changes Mar 18, 2022
View reviewed changes
Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

LGTM. Please update the version. The previous PR got merged sooner

@TylerHelmuth
Copy link
Member Author

LGTM. Please update the version. The previous PR got merged sooner

Done

@dmitryax dmitryax merged commit 653d07c into open-telemetry:main Mar 18, 2022
@TylerHelmuth TylerHelmuth deleted the issue-133 branch March 18, 2022 17:31
dmitryax added a commit that referenced this pull request Mar 18, 2022
@dmitryax
Add Tyler Helmuth as an approver
50589f1 
Tyler has been actively involved in this repo recently. The following PR were submitted or reviewed:
- #129
- #131
- #139
- #147
- #151
- #142
- #143
- #148

cc @open-telemetry/helm-approvers
@dmitryax dmitryax mentioned this pull request Mar 18, 2022
Merged
dmitryax added a commit that referenced this pull request Mar 20, 2022
@dmitryax
Add Tyler Helmuth as an approver (#152)
49ecad4 
Tyler has been actively involved in this repo recently. The following PR were submitted or reviewed:
- #129
- #131
- #139
- #147
- #151
- #142
- #143
- #148

cc @open-telemetry/helm-approvers
@TylerHelmuth TylerHelmuth mentioned this pull request Mar 20, 2022
Closed
6 tasks
@hairmare hairmare mentioned this pull request Apr 4, 2022
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitryax dmitryax dmitryax approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make volume mounts configuration more flexible
2 participants
@TylerHelmuth @dmitryax