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

Use reusable image deploy #510

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from

Conversation

kathy-t
Copy link
Contributor

@kathy-t kathy-t commented Nov 19, 2024

Description
This PR uses the reusable deploy_artifacts workflow from dockstore/workflow-actions#3 that deploys a branch/tag to artifactory and uploads an image to quay. This happens for every push of a branch and tag and does not need to be manually invoked.

The settings.xml file is deleted because the settings are now specified in the reusable deploy_artifacts workflow.

Review Instructions
Builds should pass for the develop branch. Create a new tagged release and verify that the Deploy artifacts GitHub action works.

Issue
https://ucsc-cgl.atlassian.net/browse/SEAB-6771

Security
If there are any concerns that require extra attention from the security team, highlight them here.

Please make sure that you've checked the following before submitting your pull request. Thanks!

  • Check that you pass the basic style checks and unit tests by running mvn clean install in the project that you have modified (until https://ucsc-cgl.atlassian.net/browse/SEAB-5300 adds multi-module support properly)
  • Ensure that the PR targets the correct branch. Check the milestone or fix version of the ticket.
  • If you are changing dependencies, check with dependabot to ensure you are not introducing new high/critical vulnerabilities
  • If this PR is for a user-facing feature, create and link a documentation ticket for this feature (usually in the same milestone as the linked issue). Style points if you create a documentation PR directly and link that instead.

Copy link

codecov bot commented Nov 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 20.37%. Comparing base (d7f8b3b) to head (1a0369e).

❗ There is a different number of reports uploaded between BASE (d7f8b3b) and HEAD (1a0369e). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (d7f8b3b) HEAD (1a0369e)
toolbackup 1 0
Additional details and impacted files
@@              Coverage Diff              @@
##             develop     #510      +/-   ##
=============================================
- Coverage      27.08%   20.37%   -6.72%     
+ Complexity       150      110      -40     
=============================================
  Files             44       44              
  Lines           2322     2322              
  Branches         196      196              
=============================================
- Hits             629      473     -156     
- Misses          1646     1809     +163     
+ Partials          47       40       -7     
Flag Coverage Δ
metricsaggregator 20.37% <ø> (ø)
toolbackup ?
tooltester 9.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@denis-yuen
Copy link
Member

I have similar changes in the webservice and dockstore-deploy repos to use the reusable image deploy workflow, but the above question can be applied for those repos as well, so I would like to get some opinions before I request reviews for the other repos.

Ah, you can ignore my question in the other PR.

I think it makes sense, we may want to keep an eye on artifactory (e.g. if i gets too big, or I may have set a limit on how many snapshot versions it can store, so the old ones may drop out sooner) but I think this is something @svonworl requested at some point to work with feature branches or something

Copy link
Contributor

@coverbeck coverbeck left a comment

Choose a reason for hiding this comment

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

How do we feel about creating the image for every branch?

I'm OK with creating it for every branch. Most of the time we won't need it, but when we do, it's useful. Unless we run into some quota limits on Quay, might as well.


jobs:
call-reusable-tagged-release:
uses: dockstore/workflow-actions/.github/workflows/deploy_tagged.yaml@main
uses: dockstore/workflow-actions/.github/workflows/deploy_tagged.yaml@seab-6771/reusable-image-deploy
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably you'll change this to main after your other PR is merged?

deploy_tagged:
if: github.ref_type == 'tag'
uses: dockstore/workflow-actions/.github/workflows/deploy_tagged.yaml@seab-6771/reusable-image-deploy
secrets: inherit

Choose a reason for hiding this comment

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

Little bit confused by deploy_image and deploy_tagged, they seem similar. Why not just [the functionality of] deploy_image on its own?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

deploy_image deploys the image to quay and uploads the image digest to S3.

deploy_tagged uploads the tagged release to artifactory.

They could arguably be combined (at least from the POV of this repo), but not all of our repositories require uploading the artifacts to artifactory, which is why I separated out deploy_image and deploy_tagged.

Copy link
Member

Choose a reason for hiding this comment

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

Would be good code comment

@svonworl svonworl self-requested a review November 20, 2024 16:08
Copy link

sonarcloud bot commented Nov 26, 2024

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