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

Allow setting of volumeClaimDeletePolicy in eck-elasticsearch Helm chart #6409

Merged
merged 5 commits into from
Feb 23, 2023
Merged

Allow setting of volumeClaimDeletePolicy in eck-elasticsearch Helm chart #6409

merged 5 commits into from
Feb 23, 2023

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Feb 13, 2023

closes #6249

Expose missing volumeClaimDeletePolicy for ECK Elasticsearch Helm Chart.

Also adds tests for success/failure.

Manual testing:

❯ helm template eck-elasticsearch . --values ./examples/hot-warm-cold.yaml -s templates/elasticsearch.yaml --set volumeClaimDeletePolicy="DeleteOnScaledownOnly" | yq e '.spec.volumeClaimDeletePolicy' -
DeleteOnScaledownOnly

❯ helm template eck-elasticsearch . --values ./examples/hot-warm-cold.yaml -s templates/elasticsearch.yaml --set volumeClaimDeletePolicy="bad"
Error: execution error at (eck-elasticsearch/templates/elasticsearch.yaml:46:7): volumeClaimDeletePolicy can only be one of 'DeleteOnScaledownOnly' or 'DeleteOnScaledownAndClusterDeletion'

Use --debug flag to render out invalid YAML

Signed-off-by: Michael Montgomery mmontg1@gmail.com

@naemono
Allow setting of volumeClaimDeletePolicy for eck-elasticsearch.
2aef96a 
Add test for setting of volumeclaimDeletePolicy.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono
Assert failure in tests as well.
4a84e35 
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono added >bug Something isn't working >enhancement Enhancement of existing functionality labels Feb 13, 2023
thbkrkr
thbkrkr reviewed Feb 22, 2023
View reviewed changes
Copy link
Contributor

@thbkrkr thbkrkr left a comment

Choose a reason for hiding this comment

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

LGTM except for the version.

@naemono
Revert version increment.
8b86367 
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono
Copy link
Contributor Author

naemono commented Feb 22, 2023

LGTM except for the version.

@thbkrkr version increment removed.

@thbkrkr thbkrkr mentioned this pull request Feb 23, 2023
Closed
naemono and others added 2 commits February 23, 2023 09:53
@naemono @thbkrkr
Update deploy/eck-stack/Chart.yaml
e753d70 
Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>
@naemono @thbkrkr
Update deploy/eck-elasticsearch/templates/tests/elasticsearch_test.yaml
e11b3a8 
Co-authored-by: Thibault Richard <thbkrkr@users.noreply.github.com>
@thbkrkr thbkrkr added v2.7.0 and removed >bug Something isn't working labels Feb 23, 2023
@naemono naemono merged commit 02a8a32 into elastic:main Feb 23, 2023
@naemono naemono deleted the 6249-addvolumeClaimDeletePolicy-es-helm-chart branch February 23, 2023 16:15
@thbkrkr thbkrkr changed the title Allow setting of volumeClaimDeletePolicy for eck-elasticsearch Helm chart. Allow setting of volumeClaimDeletePolicy in eck-elasticsearch Helm chart Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thbkrkr thbkrkr thbkrkr approved these changes

Assignees
No one assigned
Labels
>enhancement Enhancement of existing functionality v2.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[deploy/eck-elasticsearch] Expose volumeClaimDeletePolicy
2 participants
@naemono @thbkrkr