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

Add image upload CI workflow #300

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add image upload CI workflow #300

wants to merge 2 commits into from

Conversation

sjpb
Copy link
Collaborator

@sjpb sjpb commented Aug 9, 2023

Allows images built on either CI cloud to be uploaded to Arcus S3. Either triggered manually, or via successful completion of main CI run after merging a PR to main branch.

TODO: add secrets to repo before merging

@sjpb
Copy link
Collaborator Author

sjpb commented Aug 9, 2023

Cancelled CI as doesn't test this PR.

@sjpb sjpb marked this pull request as ready for review August 9, 2023 14:03
@sjpb sjpb requested a review from a team as a code owner August 9, 2023 14:03
@sjpb sjpb requested a review from m-bull August 9, 2023 15:04
Copy link
Collaborator

@m-bull m-bull left a comment

Choose a reason for hiding this comment

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

I think this needs equivalent manifest build/upload and then manifest promote steps from azimuth-cloud/azimuth-images#28.

Builds on main may still fail the functional tests even if they have passed in a feature branch, because package versions can shift underneath. Adding the "promote-manifest-on-tag" functionality means that we can tag only commits on main that pass the functional tests, generate a manifest, then the manifest can be consumed by azimuth-ops.

I think its just a few more steps in the upload_image.yml playbook to checksum the downloaded image, write a manifest.json that contains the checksum and the image URL and upload the manifest to the same bucket.

Maybe @mkjpryor has thoughts too?

run: |
echo CI_CLOUD: ${{ vars.CI_CLOUD }}

- name: Add bastion's ssh key to known_hosts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this step needed?

@sjpb
Copy link
Collaborator Author

sjpb commented Aug 11, 2023

I think this needs equivalent manifest build/upload and then manifest promote steps from stackhpc/azimuth-images#28.

Builds on main may still fail the functional tests even if they have passed in a feature branch, because package versions can shift underneath. Adding the "promote-manifest-on-tag" functionality means that we can tag only commits on main that pass the functional tests, generate a manifest, then the manifest can be consumed by azimuth-ops.

I think its just a few more steps in the upload_image.yml playbook to checksum the downloaded image, write a manifest.json that contains the checksum and the image URL and upload the manifest to the same bucket.

Maybe @mkjpryor has thoughts too?

Ah - the entire point here is that we do NOT rerun the build on main. The sequence is:

  • In a PR branch:
    • optionally the fatimage workflow is run, manually triggered, to create a new image, and the image defined for the .stackhpc environment is manually bumped.
    • The stackhpc workflow runs on push and tests whatever image is defined
    • PR is approved and merges
  • On main, the stackhpc workflow runs again and tests the defined image
  • If that passes, the image_upload workflow runs and uploads the image, if not already present, to s3.

So the image build only ever runs in the branch, so it runs once, but it gets tested on main.

@sjpb
Copy link
Collaborator Author

sjpb commented Aug 11, 2023

I guess I should point out - the above / this PR is just replicating what I'm going to have to do manually before #298 is usable. So I'm open to making subsequent changes for better azimuth integration, but this seems like a good first step to me.

@mkjpryor
Copy link
Member

If that is your plan, i.e. not rebuilding and retesting images when pushed to main, then you have to be very disciplined about making sure your branches are always rebased to the latest main before they are merged. Otherwise the branch might be building something that doesn't match main after it is merged.

@sjpb
Copy link
Collaborator Author

sjpb commented Aug 11, 2023

Personally I think it's ok; the image build happens on a merge checkout; the image test happens both on the merge checkout (in the PR) and then again actually on main. And then that same image is used in "production". So we do test what we use. Its a tradeoff re. the already-lengthy 1hr stackhpc CI + whatever the image build is, and trying to avoid redoing the latter.

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.

3 participants