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: upload helm chart to GitHub release artifacts [INFENG-93] #5064

Merged
merged 4 commits into from
Sep 21, 2022

Conversation

asnelling
Copy link
Contributor

@asnelling asnelling commented Sep 19, 2022

Description

INFENG-93

Test Plan

I haven't found a good way to test without sweeping changes to the CI config. Will try to come up with something doable.

Commentary (optional)

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.
  • If modifying /webui/react/src/shared/ verify make -C webui/react test-shared passes.

Title

fix: upload helm chart to GitHub release artifacts

@cla-bot cla-bot bot added the cla-signed label Sep 19, 2022
@netlify
Copy link

netlify bot commented Sep 19, 2022

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit 75c8ff6
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/632b07c8ace5b00008ab2bbc
😎 Deploy Preview https://deploy-preview-5064--determined-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@asnelling asnelling self-assigned this Sep 19, 2022
@netlify
Copy link

netlify bot commented Sep 19, 2022

Deploy Preview for storybook-det ready!

Name Link
🔨 Latest commit 75c8ff6
🔍 Latest deploy log https://app.netlify.com/sites/storybook-det/deploys/632b07c8817c1800092a11b8
😎 Deploy Preview https://deploy-preview-5064--storybook-det.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@asnelling
Copy link
Contributor Author

For testing, I had to:

  1. Create a separate repo
  2. In ./.circleci/config.yml:
    • comment out the login-helm step under the publish-helm job
    • comment out all workflows except for release
  3. put a tag name in ./VERSION
  4. comment out in ./helm/Makefile under the release target:
    • helm repo add ...
    • helm cm-push ...
  5. set GITHUB_TOKEN environment variable in CircleCI
  6. Create a release tag, e.g. 0.19.4
  7. push above changes and tag to the separate repo

@dannysauer
Copy link
Member

For testing, I had to:

What were the results of this test? :)

@asnelling
Copy link
Contributor Author

The test run created this release with linked helm chart:
https://github.com/asnelling/determined/releases/tag/0.19.3-helm9

Here's the full set of changes for the test:
https://github.com/asnelling/determined/compare/master..0.19.3-helm9

Copy link
Member

@dannysauer dannysauer left a comment

Choose a reason for hiding this comment

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

LGTM

@dannysauer
Copy link
Member

I think I would've liked to make another symlink and install goreleaser in the makefile so the process would work outside of CircleCI, but this works. So maybe call it tech debt for later. 🤷‍♂️

@dannysauer dannysauer merged commit 1734a84 into determined-ai:master Sep 21, 2022
@asnelling
Copy link
Contributor Author

asnelling commented Sep 21, 2022

If we didn't already use goreleaser, I would've just:

  • curl .../tags/$TAG/release_id # get release id
  • curl -XPOST ../releases # create new release if needed
  • curl -XPOST .../$RELEASE_ID/artifacts, # upload

dzhu pushed a commit that referenced this pull request Sep 21, 2022
* fix: upload helm chart to GitHub release artifacts

* make sure repo is clean for goreleaser

* don't assume where GOBIN or GOPATH is

Co-authored-by: Danny Sauer <danny.sauer@hpe.com>
(cherry picked from commit 1734a84)
@asnelling
Copy link
Contributor Author

It worked... but it published a release (with helm chart) for 0.19.4-rc0. Started a fix in https://github.com/asnelling/determined/commits/helm-release-gh
Current status with testing:

release test: no -rc# suffix

rc test: with -rc# suffix

@dannysauer dannysauer modified the milestones: 0.0.102, 0.19.5 Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants