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

Cirrus: Improve CI VM image updates for EC2 #15215

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

cevich
Copy link
Member

@cevich cevich commented Aug 5, 2022

AWS EC2 keys VM images by an utterly unreadable and horrible to use "AMI
ID" value. This is very error prone for humans to use, since it's
impossible to tell one image from the next by eye. Worse, they permit
duplicate name-tag values (or any other tag), complicating image
selection further.

However, Cirrus-CI recently implemented a feature by which AMI's may be
referenced by name-tag search, where the most recent AMI is chosen.
Since the containers/automation_images build workflow always assigns
a unique $IMAGE_SUFFIX value (based on the $CIRRUS_BUILD_ID), we can
now simply re-use it for both AWS and GCP image specification. In other
words to update VM images, simply update the $IMAGE_SUFFIX value
and you're done.

Signed-off-by: Chris Evich [email protected]

Does this PR introduce a user-facing change?

None

@cevich cevich requested review from lsm5 and edsantiago August 5, 2022 15:22
@openshift-ci openshift-ci bot added do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None release-note-none and removed do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Aug 5, 2022
@cevich cevich force-pushed the use_image_search branch from 9d6b1b9 to d824b86 Compare August 8, 2022 14:28
@cevich
Copy link
Member Author

cevich commented Aug 8, 2022

force-push: Rebased on main. Removed the image-update, so this commit only does the ID-search in EC2.

@cevich
Copy link
Member Author

cevich commented Aug 8, 2022

Oh woops! This is going to break hack/get_ci_vm.sh

@cevich cevich marked this pull request as draft August 8, 2022 15:48
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 8, 2022
@cevich cevich changed the title Cirrus: Improve CI VM image updates for EC2 [WIP] Cirrus: Improve CI VM image updates for EC2 Aug 8, 2022
AWS EC2 keys VM images by an utterly unreadable, horrible to use,
generated "AMI ID" value.  This is very error prone for humans in
practice, since it's impossible to tell one image from the next by
eye.  Worse, EC2 permits duplicate name-tag values, complicating
image specification further.

However fortunately, Cirrus-CI recently implemented a feature by
which AMI's may be referenced by a name-tag search - choosing
the most recent AMI found.  Since the `containers/automation_images`
build workflow always assigns a unique name + `$IMAGE_SUFFIX` value,
we can simply re-use it for both AWS and GCP image specification.

In other words as of this commit, specifying new CI VM images can
be done by simply updating the `$IMAGE_SUFFIX` value as we've always
done.  No need to call out a specific AMI ID just for EC2 tasks.

Signed-off-by: Chris Evich <[email protected]>
@cevich cevich force-pushed the use_image_search branch from d824b86 to d2d7898 Compare August 9, 2022 15:22
@cevich cevich changed the title [WIP] Cirrus: Improve CI VM image updates for EC2 Cirrus: Improve CI VM image updates for EC2 Aug 9, 2022
@cevich
Copy link
Member Author

cevich commented Aug 9, 2022

Force-push: Rebased on main.

I fixed hack/get_ci_vm.sh to work with the new image search scheme.

@cevich cevich marked this pull request as ready for review August 9, 2022 15:24
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2022
@edsantiago
Copy link
Member

Does that mean this is ready for review? LGTM at first glance.

@rhatdan
Copy link
Member

rhatdan commented Aug 9, 2022

LGTM

@rhatdan
Copy link
Member

rhatdan commented Aug 9, 2022

/approve
/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 9, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 9, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 9, 2022
@openshift-merge-robot openshift-merge-robot merged commit c33dc90 into containers:main Aug 9, 2022
@cevich cevich deleted the use_image_search branch April 18, 2023 14:46
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Aug 30, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants