Skip to content
This repository was archived by the owner on Jun 5, 2021. It is now read-only.

node-typescript-app skaffold and helm config #23

Merged
merged 13 commits into from
Sep 14, 2018

Conversation

enriched
Copy link
Contributor

@enriched enriched commented Sep 7, 2018

This currently does not work, but I created a issue on the skaffold repository:

GoogleContainerTools/skaffold#961

@CLAassistant
Copy link

CLAassistant commented Sep 7, 2018

CLA assistant check
All committers have signed the CLA.

Copy link
Owner

@fwouts fwouts 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 promising.

@@ -0,0 +1,3 @@
# my-service Helm Chart

This is a default chart created with `helm create`
Copy link
Owner

Choose a reason for hiding this comment

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

Since a lot of people (including myself) aren't familiar with Helm, would you mind including a very short description and link to https://helm.sh?

@enriched
Copy link
Contributor Author

I opened a separate pull request(#24) for the yarn cache so we can fasttrack that.

I have also been able to get helm and skaffold to work together. It's not great because the default hem configuration separates out the image name and tag, and skaffold relies on finding the full image-name:tag in the values so that it can identify the tag to associate with the image name. Probably won't be able to get around that till reusing tagPolicy variables is implemented. There is someone having a similar issue here.

@fwouts
Copy link
Owner

fwouts commented Sep 12, 2018

This PR looks good. I don't mind the workaround for now.

@enriched
Copy link
Contributor Author

I think this should be good to go for now :shipit:

@fwouts fwouts merged commit ff62b70 into fwouts:master Sep 14, 2018
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.

3 participants