Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Dec 1, 2020

Closes #154
Closes #98

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Dec 1, 2020
@ghost
Copy link
Author

ghost commented Dec 1, 2020

This likely needs a good test before its accepted into the branch, I have given it the best tests I can in the environments I have available to me

@arm4b arm4b added the enhancement New feature or request label Dec 6, 2020
Copy link
Member

@arm4b arm4b 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 contribution!
Great work with details: Helm values for dependencies and connection strings. 👍

I provided a few comments to address and let's see if we could get the CI e2e tests happy first.

Copy link
Member

@arm4b arm4b 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 changes!

Let's try to fix the CI so it works with Helm v3. I left a few pointers in the comments.

@ghost ghost requested a review from arm4b December 15, 2020 09:32
@ghost
Copy link
Author

ghost commented Dec 17, 2020

Found another potential issue running upgrade with helm.. I can't decide if this is an error or configuration, maybe config

Error: UPGRADE FAILED: template: stackstorm-ha/charts/mongodb/templates/NOTES.txt:192:4: executing "stackstorm-ha/charts/mongodb/templates/NOTES.txt" at <include "common.errors.upgrade.passwords.empty" (dict "validationErrors" (list $requiredPasswordValidationErrors) "context" $)>: error calling include: template: stackstorm-ha/charts/mongodb/charts/common/templates/_errors.tpl:18:48: executing "common.errors.upgrade.passwords.empty" at : error calling fail:
PASSWORDS ERROR: you must provide your current passwords when upgrade the release
'auth.rootPassword' must not be empty, please add '--set auth.rootPassword=$MONGODB_ROOT_PASSWORD' to the command. To get the current value:

    export MONGODB_ROOT_PASSWORD=$(kubectl get secret --namespace test stackstorm-ha-mongodb -o jsonpath="{.data.mongodb-root-password}" | base64 --decode)

@ghost
Copy link
Author

ghost commented Dec 17, 2020

Ok sorted that now

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

Oh wow, e2e CI tests are green under Helm v3 👍 @Christopher-Russell That's a ton of great work!

I left a few more minor comments below.

@arm4b
Copy link
Member

arm4b commented Dec 17, 2020

Also please check the https://github.com/StackStorm/stackstorm-ha#requirements README section, - that needs an update too following Helm v3 migration.

Chris Russell and others added 4 commits December 18, 2020 08:36
Co-authored-by: Eugen Cusmaunsa <[email protected]>
Co-authored-by: Eugen Cusmaunsa <[email protected]>
Co-authored-by: Eugen Cusmaunsa <[email protected]>
@ghost ghost requested a review from arm4b December 17, 2020 23:24
Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

@Christopher-Russell Thanks a lot for the contribution! It's massive effort and great work here! 👍

I'm approving this PR and planning to merge it closer to the v0.50.0 release due to its breaking change nature.
I'd also welcome more community testing for this PR.

Comment on lines +4 to +5
* Drop Helm `v2` support and fully migrate to Helm `v3` (#163)
* Switch dependencies from deprecated `helm/charts` to new Bitnami Subcharts (#163)
Copy link
Member

Choose a reason for hiding this comment

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

@Christopher-Russell Feel free to also include your name or nickname here as someone who contributed this work.

@arm4b arm4b changed the title change of dependencies from the official helm charts to bitnami charts (issue #98) Migrate to Helm v3, Change dependencies from deprecated official helm charts to Bitnami charts Dec 28, 2020
@arm4b arm4b merged commit e373c72 into StackStorm:master Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change enhancement New feature or request feature size/L PR that changes 100-499 lines. Requires some effort to review.

Projects

None yet

2 participants