Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Allow customization of the Statefulset #70

Merged
merged 4 commits into from
Apr 15, 2019

Conversation

tproenca
Copy link
Contributor

@tproenca tproenca commented Mar 2, 2019

I added extraVolumes, extraVolumesMounts, extraInitContainers and persistence.annotations to the helm chart.

The extraInitContainers is useful in a situation that you want to add an init-container to get the data from a backup copy for example. You can mount the volume from the volumeClaimTemplate, and copy the data there.

The extraVolumes, extraVolumesMounts can be used to mount the logs folder in a particular volume, and use a sidecar container like fluentd to send the logs elsewhere.

The persistence annotations is a good addition for clusters that don't support the storageClassName yet.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link
Contributor

@Crazybus Crazybus left a comment

Choose a reason for hiding this comment

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

This looks great! Thanks for adding these in :)

Could you also please adding the following:

  1. Add the new values into the README
  2. Add some tests for testing the templating logic. You can base it off the example for extraEnvs. Just a single test to ensure that it works correctly and has the right indentation level is enough.

@Crazybus
Copy link
Contributor

As another note it also seems like you still need to sign the CLA.

@tproenca
Copy link
Contributor Author

This looks great! Thanks for adding these in :)

Could you also please adding the following:

  1. Add the new values into the README
  2. Add some tests for testing the templating logic. You can base it off the example for extraEnvs. Just a single test to ensure that it works correctly and has the right indentation level is enough.

All done. I'm waiting for my company approve the CLA on my side.

@tproenca
Copy link
Contributor Author

I signed the CLA, but the merging is still blocked.

@Crazybus
Copy link
Contributor

jenkins test this please

Copy link
Contributor

@Crazybus Crazybus left a comment

Choose a reason for hiding this comment

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

Thanks so much for adding in the docs and testing 🥇 This is great!

@Crazybus Crazybus merged commit 6d2c953 into elastic:master Apr 15, 2019
@tproenca
Copy link
Contributor Author

Np... I'm glad to contribute. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants