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

DT-454: Add GAR specific build action #1747

Closed
wants to merge 14 commits into from

Conversation

rushtong
Copy link
Contributor

@rushtong rushtong commented Jan 29, 2025

Addresses

https://broadworkbench.atlassian.net/browse/DT-454

Summary

We take inspiration from the existing build job and from an existing Terra build process. We have an existing Trivy action, so that is not included in this PR. We pull in the Terra bumper action which provides semantic versioning. This will supercede/replace the existing TDR helm_tag_bump functionality.

Images can be accessed via a suitable firecloud.org account:
Screenshot 2025-01-30 at 11 17 21 AM

Future work

When this is complete and pushing images to GAR, we will need to:

  1. Update the deployments to look for the GAR image
  2. Retire image publishing to GCR
  3. Retire the existing version bump process, i.e. helmtagbump.yaml
  4. Retire the cherry-pick process.

Copy link

cypress bot commented Jan 29, 2025

jade-data-repo-ui    Run #4021

Run Properties:  status check passed Passed #4021  •  git commit e59bb7f1aa ℹ️: Merge 78d3abc6e3630629d6482acb9c918941b942a850 into f04b7f0be34d214499a60c11c7bd...
Project jade-data-repo-ui
Branch Review gr-DT-454-artifact-registry
Run status status check passed Passed #4021
Run duration 03m 06s
Commit git commit e59bb7f1aa ℹ️: Merge 78d3abc6e3630629d6482acb9c918941b942a850 into f04b7f0be34d214499a60c11c7bd...
Committer Gregory Rushton
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 17
View all changes introduced in this branch ↗︎

Copy link
Contributor

@fboulnois fboulnois left a comment

Choose a reason for hiding this comment

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

Early feedback:

@rushtong rushtong marked this pull request as ready for review January 31, 2025 15:02
@rushtong rushtong requested a review from a team as a code owner January 31, 2025 15:02
@rushtong rushtong requested review from s-rubenstein and fboulnois and removed request for a team January 31, 2025 15:02
Copy link
Contributor

@fboulnois fboulnois left a comment

Choose a reason for hiding this comment

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

minor suggestions, otherwise lgtm:

- name: Set up Cloud SDK
uses: google-github-actions/setup-gcloud@v2
- name: Explicitly auth Docker for Artifact Registry
run: gcloud auth configure-docker $GOOGLE_DOCKER_REPOSITORY --quiet
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
run: gcloud auth configure-docker $GOOGLE_DOCKER_REPOSITORY --quiet
run: gcloud auth configure-docker "$GOOGLE_DOCKER_REPOSITORY" --quiet

BUMP_TAG: ${{ steps.tag.outputs.tag }}
- name: Build image
run: |
docker build -t $TAGGED .
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
docker build -t $TAGGED .
docker build -t "$TAGGED" .

# Publish images
- name: Push image
run: |
docker push $TAGGED
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
docker push $TAGGED
docker push "$TAGGED"

@rushtong
Copy link
Contributor Author

rushtong commented Feb 3, 2025

Closing this PR. We've applied the Google Migration tool to all affected repos so this action isn't necessary now. Additionally, this approach will not work with the current umbrella deployment process in place, we would need to update all paths in all repos at the same time.

@rushtong rushtong closed this Feb 3, 2025
@rushtong rushtong deleted the gr-DT-454-artifact-registry branch February 6, 2025 13:00
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.

2 participants