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.

Sign up for GitHub

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

Jump to bottom

Copy check updates for quarterly check updates #1139

Merged
merged 10 commits into from
Jan 23, 2024
Merged

Copy check updates for quarterly check updates #1139

merged 10 commits into from
Jan 23, 2024

Conversation

rcrozean
Copy link
Member

Issue #, if available:

Description of changes:
Instead of rebuilding the minimal images every quarter regardless of the requirements to the images, we check the images for any updates the packages may require similar to how we check for security updates.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@rcrozean
Copy link
Member Author

/hold

@rcrozean rcrozean changed the title Copy check updates for quarterly check updates WIP: Copy check updates for quarterly check updates Aug 31, 2023
@rcrozean rcrozean force-pushed the quartlyMinimalImageUpdater branch from 02227cb to b9bb626 Compare December 13, 2023 19:49
@github-actions github-actions bot added the minimal-image Changes related to the minimal-images or tooling label Dec 13, 2023
@rcrozean rcrozean force-pushed the quartlyMinimalImageUpdater branch from b9bb626 to 0e4f0e8 Compare December 13, 2023 21:05
jaxesn
jaxesn reviewed Dec 14, 2023
View reviewed changes
Copy link
Member

@jaxesn jaxesn left a comment

Choose a reason for hiding this comment

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

This looks pretty good! One comment which I am not 100% sure on. Id probably try to test this somehow, maybe add echo in front of the actual builtkit command so you can run it quickly and just verify what it will do in different cases?

eks-distro-base/Makefile Outdated
@@ -746,6 +752,7 @@ test-all-minimal-images: $(addprefix test-minimal-images-, $(MINIMAL_VARIANTS))
standard-update: UPDATE_TARGET=standard-images
standard-update: IMAGE_NAME=eks-distro-base
standard-update: IMAGE_COMPONENT=$(IMAGE_NAME)
standard-update: SECURITY?=
Copy link
Member

Choose a reason for hiding this comment

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

I dont think you want/need this here since this target is called as a part of the update target which will it accordingly.

Copy link
Member Author

@rcrozean rcrozean Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

For some reason before this was added we were getting test failures, but can definitely test quickly to see how it runs in the different scenarios.

@rcrozean rcrozean changed the title WIP: Copy check updates for quarterly check updates Copy check updates for quarterly check updates Dec 14, 2023
@rcrozean rcrozean force-pushed the quartlyMinimalImageUpdater branch from 484863a to 0b50fc9 Compare December 18, 2023 20:58
jaxesn
jaxesn reviewed Jan 23, 2024
View reviewed changes
eks-distro-base/Makefile Outdated
create-rebuild-pr: IMAGE_REBUILD_BRANCH=rebuild-all-minimal-images
create-rebuild-pr:
$(MAKE_ROOT)/../pr-scripts/create_pr.sh eks-distro-build-tooling 'EKS_DISTRO_TAG_FILE' $(IMAGE_REBUILD_BRANCH)
.PHONY: echo
Copy link
Member

Choose a reason for hiding this comment

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

is this needed?

eks-distro-base/Makefile Outdated

.PHONY: update
update: buildkit-check open-pr-check $(UPDATE_TARGETS)
update: SECURITY?=--security
update: var-value-SECURITY \
Copy link
Member

Choose a reason for hiding this comment

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

why var-value-SECURITY as a prereq?

Copy link
Member Author

Choose a reason for hiding this comment

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

Left over from testing. Will remove.

@rcrozean rcrozean force-pushed the quartlyMinimalImageUpdater branch from f698e9f to 97f2a59 Compare January 23, 2024 20:38
@rcrozean
Copy link
Member Author

/approve

@eks-distro-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rcrozean

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

@rcrozean
Copy link
Member Author

/unhold

@eks-distro-bot eks-distro-bot merged commit b336a63 into aws:main Jan 23, 2024
38 checks passed
@rcrozean rcrozean deleted the quartlyMinimalImageUpdater branch January 23, 2024 22:32
@rcrozean rcrozean mentioned this pull request Jan 23, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaxesn jaxesn jaxesn approved these changes

Assignees

@jaxesn jaxesn

Labels
approved lgtm minimal-image Changes related to the minimal-images or tooling size/XXL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rcrozean @eks-distro-bot @jaxesn @eks-distro-pr-bot