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

Updating finding new dockerfiles and building them to test #499

Merged
merged 10 commits into from Dec 20, 2022

Conversation

erinyoung
Copy link
Contributor

@erinyoung erinyoung commented Nov 30, 2022

EDIT : NOW READY FOR REVIEW!!!

This is not a new docker container. There are some depreciation warnings to our github actions for new dockerfiles in PR. I need to test these, but this is what it may look like.

@erinyoung
Copy link
Contributor Author

For future reference (if it takes me forever to come back to this), this is how github actions is getting variables from actions: https://hynek.me/til/set-output-deprecation-github-actions/

@erinyoung
Copy link
Contributor Author

Also, https://github.com/ewels/MultiQC/blob/a8cbadef13bbc2201b54da072fd00be0c061f273/.github/workflows/multiqc_linux.yml has an awesome way to put a variable in the action name. This is helpful when looking for specific actions.

@erinyoung
Copy link
Contributor Author

@erinyoung erinyoung marked this pull request as ready for review December 1, 2022 22:58
@erinyoung
Copy link
Contributor Author

erinyoung commented Dec 1, 2022

Alright : this is now ready for review.

This is phase 1/n of the facelift of StaPH-B/docker-builds.

What this does :

  • adds more information to "manual deploy"
  • brings "manual deploy" up to date to remove all depreciation warnings
  • brings "manual test" up to date to remove all depreciation warnings
  • brings the current "find_dockerfiles" github actions workflow up-to-date to remove all depreciation warnings
    • looks for the following in either the dockerfile or built image:
    • labels
    • app and test layers
    • WORKDIR

What this PR does NOT do :

  • use a workflow in parallel (still not supported by github actions - even with version 3)

This is just an update to keep the versions in line. It simply builds the docker image to the 'test' stage.
@erinyoung
Copy link
Contributor Author

The build-to-test changes seem to work great (https://github.com/StaPH-B/docker-builds/actions/runs/3622848226/jobs/6108036499)

@erinyoung
Copy link
Contributor Author

erinyoung commented Dec 5, 2022

I've updated all the "manual deploy" steps, as well. There are still warnings, but they aren't something I can do something about unless we want to go with some other command than docker/build-push-action. https://github.com/StaPH-B/docker-builds/actions/runs/3622937452

Run docker/build-push-action@v2
Warning: The `save-state` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/
Docker info
Warning: The `save-state` command is deprecated and will be disabled soon. Please upgrade to using Environment Files. For more information see: https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

@kapsakcj kapsakcj self-requested a review December 13, 2022 22:38
@kapsakcj
Copy link
Collaborator

Apologies for the delay in reviewing, I'll review this PR ASAP

@@ -30,6 +30,8 @@ on:
description: "Repository name. <repository>/tool:tag (Usually staphb)"
default: "staphb"

run-name: Deploy ${{ github.event.inputs.tool }} version ${{ github.event.inputs.version }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love this addition, it makes the names of GH Action workflow much more descriptive than "Manual Deploy"

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 know! I saw it in multiqc's repo. I feel like it's going to make deploy better.

@kapsakcj
Copy link
Collaborator

OK, so here's what testing I've done:

First - I ran the workflow .github/workflows/manual-deploy.yml using SPAdes 3.15.4 as the guinea pig dockerfile. I did not ask it to overwrite the :latest image tag, just update the dockerhub and quay tags for :3.15.4

It failed due to the cab.spbu.ru server failling to properly serve up the files requested. SO I opted to edit the dockerfile on this PR to download from GitHub instead of the Russian server (9a7787a) and watch the GHActions cascade occur.

Second - The workflows (find_new_dockerfiles and build_to_test) passed as expected after I edited the spades 3.15.4 dockerfile ✔️ , so....

Third I ran the manual-deploy.yml workflow again to test now that it's switched to downloading from Github. log here: https://github.com/StaPH-B/docker-builds/actions/runs/3741894625

The deploy workflow ran as expected this time. The new checks for ps and ca-certificates worked as expected as well as the checks for app and test layers of dockerfiles. Also the check for WORKDIR label was successful

Lastly - I pushed a commit to update to docker/build-push-action@v3 for build-to-deploy.yml workflow (78d3abd)

^ After making this commit, I re-ran manual-deploy.yml with the same spades 3.15.4 dockerfile as a test, just to make sure things go as expected. They did 🎉 . Log here: https://github.com/StaPH-B/docker-builds/actions/runs/3742167477/jobs/6352761688

I'm good with merging this PR

@kapsakcj
Copy link
Collaborator

And lastly, regarding the save-state warning associated with docker/build-push-action: I did not see the warning after upgrading to the docker/build-push-action@v3. I believe the maintainers of that workflow resolved the issue docker/build-push-action#703 (comment)

@erinyoung
Copy link
Contributor Author

Yay!

@kapsakcj kapsakcj merged commit 378674d into master Dec 20, 2022
@kapsakcj
Copy link
Collaborator

I'll let you delete the dev branch if you're finished with it

@erinyoung erinyoung deleted the erin-find-new-dockerfiles branch December 20, 2022 16:24
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.

None yet

2 participants