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

fix annotation bug. Use active service on ingress when ha #270

Merged
merged 4 commits into from
May 28, 2020

Conversation

georgekaz
Copy link
Contributor

Annotations currently give a warning because they are not actually rendering as yaml like normal charts do.
"Warning: Merging destination map for chart 'vault'. Cannot overwrite table item 'annotations', with non table value: map[]"

changing from annotations: {} to annotations: [] removes this warning.

Additionally I've added some logic that points the ingress at the active server when in ha mode. There are times that pointing at the standby pods causes problems.

@hashicorp-cla
Copy link

hashicorp-cla commented Apr 21, 2020

CLA assistant check
All committers have signed the CLA.

@tvoran tvoran added bug Something isn't working chart Area: helm chart labels Apr 22, 2020
values.yaml Outdated Show resolved Hide resolved
@tvoran tvoran self-requested a review April 27, 2020 15:52
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.

Thanks for the patch! I think this needs a test, and the defaults for annotations should be left as they are.

I'm also wondering if the service type should be configurable for the active and standby services, since it's set to ClusterIP now.

values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
values.yaml Outdated Show resolved Hide resolved
templates/server-ingress.yaml Show resolved Hide resolved
fix a missing :

revert values as requested
@georgekaz
Copy link
Contributor Author

georgekaz commented May 27, 2020

I've reverted the annotations as requested and actually corrected a mistake in the serviceName. It prints as expected using helm template and helm lint. Not sure how to automate this test on here.

p.s. the force pushes were just to squash the commits

@tvoran tvoran added this to the v0.6.0 milestone May 27, 2020
@georgekaz georgekaz requested a review from tvoran May 28, 2020 09:56
@tvoran tvoran merged commit d755ad1 into hashicorp:master May 28, 2020
@georgekaz georgekaz deleted the chart-patches branch May 28, 2020 22:48
radudd pushed a commit to radudd/vault-helm that referenced this pull request Jun 5, 2020
Added some logic that points the ingress at the active server
when in ha mode. There are times that pointing at the standby
pods causes problems.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working chart Area: helm chart
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants