Skip to content

Conversation

@kudla
Copy link
Contributor

@kudla kudla commented Dec 1, 2020

No description provided.

@kudla kudla requested review from akranga and arkadijs December 1, 2020 14:55
@kudla
Copy link
Contributor Author

kudla commented Dec 1, 2020

Should I better go with a new component or update exiting mysql?

@arkadijs
Copy link
Contributor

arkadijs commented Dec 1, 2020

This is wrong in so many ways it is not worth having.

Copy link
Contributor Author

@kudla kudla left a comment

Choose a reason for hiding this comment

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

This is wrong in so many ways it is not worth having.

Would you be more specific please?

# leads escaped quutes in the result command
# + helm --kube-context=busy-odilee-553.bubble.superhub.io --namespace=kubeflow-data upgrade mysql .charts/mysql --install --create-namespace --debug --wait --version 0.1.0 --values values.yaml \
# --set ''\''mysqlUser=mysql''\'' --set ''\''mysqlPassword=somepass'\''' --set ''\''mysqlRootPassword=somepass''\'' --set ''\''persistence.existingClaim=mysql''\''

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arkadijs Please do I follow the idea right on this change

And there's an issue I've faced with mentioned in TODO regarding quoting the values
What is the proper way for solving it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had an alternative way for exposing the exact values over HELM_OPTS just composing the extra --values set dynamically instead combined with hub templates interpolation.
I would be more safe but it seemed to be more messy same time regarding code structure.

Copy link

@akranga akranga left a comment

Choose a reason for hiding this comment

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

@kudla please rework this component in the way we agreed during our conversation earlier

- name: helm
parameters:
- name: repo
value: https://storage.googleapis.com/t3n-helm-charts
Copy link

Choose a reason for hiding this comment

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

@@ -0,0 +1,48 @@
#!/bin/sh -xe

HELM_OPTS=""
Copy link

Choose a reason for hiding this comment

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

How does HELM_OPTS used here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants