Conversation
) * settings-1-update.yml: Validate settings in parallel based on target * deploy-2-start.yml: Output modules and spack location from workflow * deploy-2-start.yml: Capitalize inputs.type values to keep in line with callers * deploy-1-setup.yml: Update inputs/outputs to be suitable for a matrix job. This matrix job must have the required I/O to do pre-deploy checks, as well as a deployment. * Move check-spack-yaml and check-config jobs from ci.yml to deploy-1-setup.yml * deploy-1-setup.yml: Use new inputs as inputs to deploy-2-start.yml * deploy-1-start.yml: Upload 'outputs' of workflow as an artifact. This is due to dynamic matrix job outputs not being collected appropriately. See https://github.com/orgs/community/discussions/17245 * cd.yml: Run checks of target config in parallel * ci.yml: Dynamically generate deployment comment, start matrix of deployment jobs, misc changes * cd.yml: Update deployment matrix jq * ci.yml: Add newline to deployment comment to fix formatting * Update job names to include deploy target
…general metadata artifact glob
…loy-2-start.yml` invocation
Multi-Target GitHub Releases
Co-authoured-by: Tom McAdam <[email protected]>
Jinja Templating for Complex Text
Replace `env.SPACK_YAML_MODEL_YQ` to accomodate for multi-target-format `spack.yaml`
This was
linked to
issues
Jan 29, 2025
Closed
This was referenced Jan 29, 2025
This was referenced Jan 29, 2025
aidanheerdegen
previously approved these changes
Jan 30, 2025
Member
aidanheerdegen
left a comment
aidanheerdegen
left a comment
There was a problem hiding this comment.
The vibe seems good. I have some questions, but am happy to merge if you think it appropriate.
This is because all environment are now of the form SUPERCOMPUTER TYPE
Update Acceptable Environments to the form `SUPERCOMPUTER TYPE`
Member
Author
|
The merged pull request should satisfy all your vibe-based review comments, @aidanheerdegen! |
aidanheerdegen
approved these changes
Feb 3, 2025
Member
aidanheerdegen
left a comment
aidanheerdegen
left a comment
There was a problem hiding this comment.
Happy to hit the big green button. Great work, thanks.
Member
Author
|
Thanks to @aidanheerdegen, @jo-basevi and @tmcadam for helping review the various PRs within this super-PR 🥳 |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Closes #121 super-issue
Close #213
Close #196
Important
This is a major update to build infrastructure. Therefore, there are changes required to Model Deployment Repositories before the
v4branch can be used. These are noted in the 'Major Model Deployment Repository Changes' sectionBackground
This super Pull Request contains a rework of
build-cdinfrastructure to support deployment to multiple HPCs at once. The major changes are noted below:spack.yamlUpdates:spack.yamlfile now allows for variations in compiler/target/variants based on HPC target via mutually-exclusivespack.definitions[].whenclauses, example here. Schema defined in1-0-4.json, implementation done in Support Multi-targetspack.yaml#208 and discussed in Investigation: Multi-target Manifest Files model-deployment-template#15Model Deployment Repository Choice In Deployment Targets: Since not all model deployment repositories may want to deploy to all valid deployment targets (namely, all the things defined in
config/settings.json), model deployment repositories can set the repo-levelvars.[PRE]RELEASE_DEPLOYMENT_TARGETSto choose Prerelease/Release targets. This is most useful for future Releases where we don't want to yet deploy officially to other HPC targets. Done in Support Deployment Target Choice For Deployment Repositories #211Release Notes and Deployment Comment Updates: Examples of the former and latter, the final update from Jinja Templating for Complex Text #207
Releases: A notable decision: a single deployment version encompasses multiple deployment targets, see Make
releaseJob Take Inputs From Multi-Target Deployments #200 (comment)Model DB: Notably, the model DB has been left unchanged, only adding
Gadi-specific information. This was because it'd be a fairly large piece of work in itself, and we probably won't be deploying official Releases to other targets for a while. The interim solution was done in Interimmodel-dbUpdate: Only IngestGadiMetadata #214 and the decision to wait on the larger update: Consider Multi-Target Paths For Model DB #201Updated workflow matrix structure: See the chicken-scratching picture regarding deployment workflow here Move jobs around so they can be part of a single parallel workflow #203, and release/release provenance workflow here Multi-Target GitHub Releases #205
Pull Requests Incorporated
This PR incorporates the following merged pull requests from
build-cd:env.SPACK_YAML_MODEL_YQto accomodate for multi-target-formatspack.yaml#210model-dbUpdate: Only IngestGadiMetadata #214SUPERCOMPUTER TYPE#221And the following into other related repositories:
spack.yamlschema#41spack.yamlmodel-deployment-template#18SUPERCOMPUTER ReleaseEnvironments model-deployment-template#22And the changes made solely in this PR (consolidated here):
deploy-2-start.ymldeployment-environmentinput intodeployment-targetanddeployment-typein beee2cfbuild-cdreferences to latest (future) default branch (v4) in 6299c48Major Model Deployment Repository Changes
Since this is a major
build-cdbump (due to changed entrypoint workflows, new vars and the overall size of the update), we need to do updates to all Model Deployment Repositories before we can use the newv4branch. Namely:vars.RELEASE_DEPLOYMENT_TARGETS,vars.PRERELEASE_DEPLOYMENT_TARGETSfor model-deployment-repository-level choice on deployment targets.vars.SPACK_YAML_SCHEMA_VERSIONto1-0-4ci.ymlspr-closedjob inputs fromnametoroot-sbd, due to 4d8a7c8, and give thecdjobpermissions.pull-requests:writedue to fa11ad3v3workflows tov4SUPERCOMPUTERwithSUPERCOMPUTER ReleaseTesting
E2E Testing was done primarily on
ACCESS-NRI/ACCESS_TEST, (Prerelease-related stuff in ACCESS-NRI/ACCESS-TEST#16, and Release-related stuff in ACCESS-NRI/ACCESS-TEST#20).We used a branch (
dev-121-multi-target-workflows_TEST, may be deleted now) off this one in which we add config for an additionalGadiTestenvironment, and call workflows from this branch. See branch comparison here.Testing Prereleases
✔️ With traditional, single-target
spack.yamlWorks.
✔️ With multi-target
spack.yamlWorks! See this commit, run and result.
✔️ Using
!bumpStill works, see:
✔️ Using
!redeployWorks: See invocation, run and result.
Closing PRs
Works! See run.
Testing Releases
✔️ Deployment
Succeeds, see run, and (ephemeral, may be gone later) deployment accessible via:
✔️ Release Artifact
Works, see run and (Un)official Realease.
✔️ Model DB Upload (stub)
Data for this side of the old DB API looks correct, see run.