-
Notifications
You must be signed in to change notification settings - Fork 6
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
ci: Test pathogen repo CI builds using images on docker.io instead of ghcr.io #151
Merged
tsibley
merged 1 commit into
master
from
trs/pathogen-repo-ci/use-docker.io-instead-of-ghcr.io
May 10, 2023
Merged
ci: Test pathogen repo CI builds using images on docker.io instead of ghcr.io #151
tsibley
merged 1 commit into
master
from
trs/pathogen-repo-ci/use-docker.io-instead-of-ghcr.io
May 10, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… ghcr.io This should fix the currently-broken test-pathogen-repo-ci jobs when our CI runs on master² (and they should continue to work on branches too). It requires a little rearranging of jobs in the workflow, with a little additional and unfortunate complexity due to conditionals. This works around a Nextstrain CLI bug with registries other than docker.io¹ during `nextstrain update docker` (and `nextstrain setup docker`), which is run as part of our setup-nextstrain-cli action used by this workflow. Ideally we'll fix that bug and then be able to revert this, especially since we might actually want to condition the pushing to docker.io on the outcome of these test jobs in the future. ¹ <nextstrain/cli#279> ² <#148 (comment)>
victorlin
reviewed
May 10, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description and changes make sense 👍
tsibley
added a commit
to nextstrain/conda-base
that referenced
this pull request
May 11, 2023
[ Commit message based on that of 12000a20 in nextstrain/docker-base.¹ Code changes also based on that commit, plus subsequent commits.² ] A useful check for if new packages will break our pathogen builds. I included all pathogen repos that already use our pathogen-repo-ci reusable workflow. It should be minimal effort to maintain this list over time—I expect it to only grow—but perhaps in the future we will want to abstract it out into a shared list of known pathogen repos. I don't like that we have to copy the build-args for a few of the repos here since it'll be easy for this copy to diverge from the repo's authoritative build-args, but it's necessary for now. Over time as we work towards increased automation of pathogen builds, I think we can get rid of this build-args copy by further standardizing how each repo configures itself for automation. For example, instead of specifying build-args in a repo's CI workflow, the args for CI could be stored in a broader workflow metadata file (e.g. nextstrain-workflow.yaml) read by pathogen-repo-ci, or defined by some other convention. An alternative to directly running pathogen-repo-ci against each repo here would be instead triggering the CI workflows themselves within each repo. The downside to that is it would divorce the outcomes of those workflows from this one and render them not visible from PRs in this repo. It would also require updates to each repo to support triggering and passing in of additional parameters (i.e. for the package). And finally those CI workflows sometimes run other jobs, like linting and other integration tests (e.g. with Cram), that aren't always necessary to run with a new package. Related-to: <nextstrain/docker-base#148> Related-to: <nextstrain/docker-base#150> Related-to: <nextstrain/docker-base#151> ¹ <nextstrain/docker-base@12000a20> ² <nextstrain/docker-base@bc22a0bc> <nextstrain/docker-base@0a20a474> <nextstrain/docker-base@75254e92>
1 task
tsibley
added a commit
to nextstrain/conda-base
that referenced
this pull request
May 11, 2023
[ Commit message based on that of 12000a20 in nextstrain/docker-base.¹ Code changes also based on that commit, plus subsequent commits.² ] A useful check for if new packages will break our pathogen builds. I included all pathogen repos that already use our pathogen-repo-ci reusable workflow. It should be minimal effort to maintain this list over time—I expect it to only grow—but perhaps in the future we will want to abstract it out into a shared list of known pathogen repos. I don't like that we have to copy the build-args for a few of the repos here since it'll be easy for this copy to diverge from the repo's authoritative build-args, but it's necessary for now. Over time as we work towards increased automation of pathogen builds, I think we can get rid of this build-args copy by further standardizing how each repo configures itself for automation. For example, instead of specifying build-args in a repo's CI workflow, the args for CI could be stored in a broader workflow metadata file (e.g. nextstrain-workflow.yaml) read by pathogen-repo-ci, or defined by some other convention. An alternative to directly running pathogen-repo-ci against each repo here would be instead triggering the CI workflows themselves within each repo. The downside to that is it would divorce the outcomes of those workflows from this one and render them not visible from PRs in this repo. It would also require updates to each repo to support triggering and passing in of additional parameters (i.e. for the package). And finally those CI workflows sometimes run other jobs, like linting and other integration tests (e.g. with Cram), that aren't always necessary to run with a new package. Related-to: <nextstrain/docker-base#148> Related-to: <nextstrain/docker-base#150> Related-to: <nextstrain/docker-base#151> Related-to: <nextstrain/docker-base#154> ¹ <nextstrain/docker-base@12000a20> ² <nextstrain/docker-base@bc22a0bc> <nextstrain/docker-base@0a20a474> <nextstrain/docker-base@75254e92>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This should fix the currently-broken test-pathogen-repo-ci jobs when our CI runs on master² (and they should continue to work on branches too). It requires a little rearranging of jobs in the workflow, with a little additional and unfortunate complexity due to conditionals.
This works around a Nextstrain CLI bug with registries other than docker.io¹ during
nextstrain update docker
(andnextstrain setup docker
), which is run as part of our setup-nextstrain-cli action used by this workflow. Ideally we'll fix that bug and then be able to revert this, especially since we might actually want to condition the pushing to docker.io on the outcome of these test jobs in the future.¹ nextstrain/cli#279
² #148 (comment)
Testing