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

Adding an exhaustive list of Helm chart values to eck-elasticsearch Helm chart #6336

Merged
merged 11 commits into from
Feb 7, 2023

Conversation

naemono
Copy link
Contributor

@naemono naemono commented Jan 18, 2023

This change adds an exhaustive list of Helm chart values to the eck-elasticsearch Helm chart.

This expands the podTemplate field in the values file and includes all relevant options within the podTemplate field.

relates to #6187

Relates to: elastic#6187

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono naemono added >enhancement Enhancement of existing functionality >docs Documentation v2.7.0 labels Jan 18, 2023
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

I am not sure about this one. I am not sure duplicating the PodTemplateSpec is the right approach. How do we keep that updated etc. Would it not suffice to link to the k8s API docs?

deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Show resolved Hide resolved
@naemono
Copy link
Contributor Author

naemono commented Jan 25, 2023

I am not sure duplicating the PodTemplateSpec

How do you feel about me documenting the most commonly used/overridden options in the podtemplatespec? While also pointing to the documentation?

Review comments.

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
@naemono
Copy link
Contributor Author

naemono commented Jan 25, 2023

@pebrc I've trimmed down the number of fields shown in the podTemplateSpec section. Let me know what you think. Thanks.

@thbkrkr
Copy link
Contributor

thbkrkr commented Jan 25, 2023

Could you merge main to work with #6329 in this PR?

Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Copy link
Collaborator

@pebrc pebrc left a comment

Choose a reason for hiding this comment

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

LGTM, I think we really have to fix the Helm versioning. I just bumped the charts for a change that introduced the version to the next minor and here we are doing it again but to a patch version. @thbkrkr you have to probably consolidate these version bumps before creating the next release branch.

deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
deploy/eck-elasticsearch/values.yaml Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
deploy/eck-elasticsearch/values.yaml Outdated Show resolved Hide resolved
Signed-off-by: Michael Montgomery <mmontg1@gmail.com>
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

@naemono naemono merged commit c28889c into elastic:main Feb 7, 2023
@naemono naemono deleted the helm-eck-es-values-readme branch February 7, 2023 20:12
@thbkrkr thbkrkr changed the title Adding an exhaustive list of Helm chart values to eck-elasticsearch chart. Adding an exhaustive list of Helm chart values to eck-elasticsearch Helm chart Mar 21, 2023
@thbkrkr thbkrkr removed the >enhancement Enhancement of existing functionality label Mar 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>docs Documentation v2.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants