Skip to content

Pre-loading container images into CRI-O#568

Merged
fzdarsky merged 5 commits intoopenshift:mainfrom
husky-parul:imageloader
Mar 4, 2022
Merged

Pre-loading container images into CRI-O#568
fzdarsky merged 5 commits intoopenshift:mainfrom
husky-parul:imageloader

Conversation

@husky-parul
Copy link
Copy Markdown
Contributor

@husky-parul husky-parul commented Jan 25, 2022

Signed-off-by: Parul parsingh@redhat.com

Which issue(s) this PR addresses:
Pre-loading container images into CRI-O

Closes #
#162

Should be merged after #593

@husky-parul husky-parul added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 25, 2022
@husky-parul husky-parul self-assigned this Jan 25, 2022
@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 Jan 25, 2022
@husky-parul husky-parul marked this pull request as draft January 25, 2022 14:14
@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 Jan 25, 2022
@openshift-ci openshift-ci Bot requested review from mangelajo and sallyom January 25, 2022 14:14
@husky-parul husky-parul changed the title image loader | initial drop [WIP] Pre-loading container images into CRI-O Jan 25, 2022
@husky-parul husky-parul force-pushed the imageloader branch 2 times, most recently from a1df000 to 8e44116 Compare February 1, 2022 15:07
@husky-parul husky-parul removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 1, 2022
@husky-parul husky-parul marked this pull request as ready for review February 1, 2022 15:51
@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 Feb 1, 2022
@openshift-ci openshift-ci Bot requested review from oglok and rootfs February 1, 2022 15:51
@husky-parul husky-parul force-pushed the imageloader branch 2 times, most recently from acbd9a0 to d6316a5 Compare February 1, 2022 15:57
@husky-parul husky-parul removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 1, 2022
@husky-parul
Copy link
Copy Markdown
Contributor Author

/retest

@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 Feb 1, 2022
@husky-parul husky-parul changed the title [WIP] Pre-loading container images into CRI-O Pre-loading container images into CRI-O Feb 1, 2022
@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 Feb 1, 2022
@husky-parul
Copy link
Copy Markdown
Contributor Author

/retest

Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
Comment thread pkg/controllers/image-loader-controller.go Outdated
@husky-parul husky-parul added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 1, 2022
@husky-parul
Copy link
Copy Markdown
Contributor Author

@fzdarsky please TAL

Comment thread packaging/rpm/microshift-images.spec Outdated
Release: 1
# this can be %{timestamp}.git%{short_hash} later for continous main builds
Summary: Create custom container storage
License: ASL 2.0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is ASL the correct abbreviation (as it's Apache License not Apache Software Licence or such?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ASL is Apache Software License. Going to change it to AL 2.0

%build


%install
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't this belong into the %build section, such that the RPM contains the images (rather than pulling them when installed)? Not sure the ImageBuilder can actually run stuff on podman when building ostrees.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As per the doc, https://rpm-packaging-guide.github.io/#what-is-a-spec-file

%build: Command or series of commands for actually building the software into machine code (for compiled languages) or byte code (for some interpreted languages).
%Install: Command or series of commands for copying the desired build artifacts from the %builddir (where the build happens) to the %buildroot directory (which contains the directory structure with the files to be packaged). This usually means copying files from ~/rpmbuild/BUILD to ~/rpmbuild/BUILDROOT and creating the necessary directories in ~/rpmbuild/BUILDROOT. This is only run when creating a package, not when the end-user installs the package. See [Working with SPEC files](https://rpm-packaging-guide.github.io/#working-with-spec-files) for details.

Since we are not building s/w and only using rpm to package files, I put it under %install

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it's .spec files just being confusing,

What you expect for install is %post

%install is the phase that takes the %build, and put's it into the fake root filesystem,
from where %file will gather stuff.

Comment thread packaging/rpm/microshift-images.spec Outdated
Comment thread packaging/rpm/microshift-images.spec Outdated
husky-parul and others added 4 commits March 2, 2022 11:09
Signed-off-by: Parul <parsingh@redhat.com>
Signed-off-by: Parul <parsingh@redhat.com>
Signed-off-by: Parul <parsingh@redhat.com>
Signed-off-by: Miguel Angel Ajo <majopela@redhat.com>
@husky-parul
Copy link
Copy Markdown
Contributor Author

@mangelajo @fzdarsky
I understand make-microshift-images-rpm.sh will not create the .rpm for users images. Are we planning to keep two specs, one for microshift-images and one for users images?

Comment thread packaging/rpm/microshift-images.spec

#Generate microshift-app-images.spec
touch ./microshift-app-images.spec
cat >./microshift-app-images.spec <<EOF
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should probably propagate the changes from the other spec here.

But I propose we merge, and then we keep working on the userland side.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@mangelajo good idea. Could you open an issue for that please so we don't forget?

Comment thread packaging/rpm/microshift-images.spec Outdated
URL: https://github.com/redhat-et/microshift

BuildRequires: podman
BuildRequires: crio
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
BuildRequires: crio
Requires: crio

@mangelajo
Copy link
Copy Markdown
Contributor

#611
this will be necessary but can be independent of this patch.

Signed-off-by: Parul <parsingh@redhat.com>
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Mar 3, 2022

@husky-parul: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-openshift-conformance-sig-network f12cc6a link false /test e2e-openshift-conformance-sig-network
ci/prow/e2e-openshift-conformance-sig-auth f12cc6a link false /test e2e-openshift-conformance-sig-auth
ci/prow/e2e-reboot f12cc6a link false /test e2e-reboot
ci/prow/e2e-openshift-conformance-sig-apps f12cc6a link false /test e2e-openshift-conformance-sig-apps
ci/prow/e2e-openshift-conformance-sig-scheduling f12cc6a link false /test e2e-openshift-conformance-sig-scheduling
ci/prow/e2e-openshift-conformance-sig-api-machinery f12cc6a link false /test e2e-openshift-conformance-sig-api-machinery
ci/prow/e2e-openshift-conformance-sig-node f12cc6a link false /test e2e-openshift-conformance-sig-node
ci/prow/e2e-openshift-conformance-sig-instrumentation f12cc6a link false /test e2e-openshift-conformance-sig-instrumentation
ci/prow/e2e-openshift-conformance-sig-storage f12cc6a link false /test e2e-openshift-conformance-sig-storage
ci/prow/e2e-openshift-conformance-sig-cli f12cc6a link false /test e2e-openshift-conformance-sig-cli
ci/prow/e2e-openshift-conformance-sig-arch f12cc6a link false /test e2e-openshift-conformance-sig-arch

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@mangelajo
Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ci openshift-ci Bot added lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 4, 2022
@mangelajo mangelajo requested review from fzdarsky and mangelajo March 4, 2022 09:42
@fzdarsky
Copy link
Copy Markdown
Contributor

fzdarsky commented Mar 4, 2022

/lgtm

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Mar 4, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fzdarsky, mangelajo

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

The pull request process is described here

Details 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants