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

Add an option to set annotations on the StatefulSet #199

Merged
merged 8 commits into from
Aug 14, 2020

Conversation

cablespaghetti
Copy link
Contributor

I'm using this to add the annotation for Reloader to the StatefulSet. I've tried to follow the conventions currently used by the chart but let me know if anything needs changing.

@hashicorp-cla
Copy link

hashicorp-cla commented Feb 12, 2020

CLA assistant check
All committers have signed the CLA.

@jasonodonnell
Copy link
Contributor

Hi @cablespaghetti, this looks useful!

Can you add unit test coverage for this feature? You can find them in tests/unit.

@cablespaghetti
Copy link
Contributor Author

@jasonodonnell Sorry it took a while. Have some tests. 👍

@tvoran tvoran added chart Area: helm chart enhancement New feature or request labels Mar 4, 2020
@cablespaghetti
Copy link
Contributor Author

@jasonodonnell Don't forget about this one :)

@tvoran tvoran self-requested a review May 1, 2020 01:53
Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

Apologies, but here are some suggested changes since we've recently changed all the annotations to support both YAML and YAML-formatted strings, and I think this patch will need to follow that convention.

templates/_helpers.tpl Outdated Show resolved Hide resolved
test/unit/server-statefulset.bats Outdated Show resolved Hide resolved
test/unit/server-statefulset.bats Show resolved Hide resolved
values.yaml Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
@jasonodonnell
Copy link
Contributor

Bump on this @cablespaghetti

@jasonodonnell
Copy link
Contributor

Bump @cablespaghetti :)

Signed-off-by: Sam Weston <weston.sam@gmail.com>
Signed-off-by: Sam Weston <weston.sam@gmail.com>
Signed-off-by: Sam Weston <weston.sam@gmail.com>
cablespaghetti and others added 5 commits August 10, 2020 19:04
Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>
Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>
Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>
Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>
Co-authored-by: Theron Voran <tvoran@users.noreply.github.com>
@cablespaghetti
Copy link
Contributor Author

@tvoran @jasonodonnell I've committed the requested changes now. Sorry for the delay.

Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

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

It all checks out for me, thanks for the patch!

@tvoran tvoran merged commit ed0b918 into hashicorp:master Aug 14, 2020
@jasonodonnell jasonodonnell mentioned this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chart Area: helm chart enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants