Skip to content

Conversation

@fidencio
Copy link
Contributor

@fidencio fidencio commented Mar 9, 2021

- What I did
This adds support for a new RHCOS extension called
"sandboxed-containres". It will install kata-containers and its
dependencies, 9 RPMs with a total size of ~55MB when downloaded, ~150MB
when installed.

It will allow users to run kernel isolated containers via the Sandboxed
Containers operator.

- How to verify it
TBD

- Description for the changelog
Added support for a new RHCOS extension to install sandboxed-containers.

IMPORTANT:
Let me add a big note here that I'm experimenting with the fact of having a kata-containers extension, superseding #2376, and it comes from a suggestion made by @cgwalters while reviewing openshift/enhancements#677 (comment) (also, refer to: https://coreos.slack.com/archives/CK1AE4ZCK/p1614974860056400).

While modifying openshift/enhancements#677 according to the reviewers suggestions, I'm experimenting, I'm trying to have all the pieces together, so we can make the proposal solid and based on work that's either done or close to be done, rather than basing it on what we think that should be done.

@fidencio
Copy link
Contributor Author

fidencio commented Mar 9, 2021

/hold

@fidencio
Copy link
Contributor Author

fidencio commented Mar 9, 2021

NOTE: As it's right now, this is a test to see whether we can easiler supersede #2376, if needed.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 9, 2021
@fidencio fidencio changed the title Add new extension for qemu-kiwi Add new extension for sandboxed-containers Mar 9, 2021
@fidencio fidencio force-pushed the wip/add-sandboxed-containers-extensions branch 3 times, most recently from 198e081 to 70b4912 Compare March 9, 2021 12:25
@fidencio fidencio force-pushed the wip/add-sandboxed-containers-extensions branch from 70b4912 to d404023 Compare March 11, 2021 11:26
@fidencio
Copy link
Contributor Author

I've updated the PR according to @sinnykumari's review. (thanks!)

@fidencio fidencio force-pushed the wip/add-sandboxed-containers-extensions branch from d404023 to 0b06a80 Compare March 11, 2021 11:33
@fidencio
Copy link
Contributor Author

@sinnykumari, as this one supersedes #2376, can we have the 4.8 label added to this one?

@jensfr, can we have #2376 closed?

@fidencio
Copy link
Contributor Author

/retest

@jensfr
Copy link

jensfr commented Mar 25, 2021

@jensfr, can we have #2376 closed?

done

@sinnykumari sinnykumari added the 4.8 Work deferred for 4.8 label Mar 25, 2021
@sinnykumari
Copy link
Contributor

@sinnykumari, as this one supersedes #2376, can we have the 4.8 label added to this one?

added 4.8 label. Once you have machine-os-content updated with correct set of packages, let us know in the PR.

fidencio added a commit to fidencio/os that referenced this pull request Mar 25, 2021
This reworks af93514 as, since it was
merged, a few changes happened in the enhancement proposal and it's been
decided to go with a "sandboxed-containers" extensions instead.

As the "qemu-kiwi" extension is no longer needed, we're removing it
(well, reworking it) and adding the "sandboxed-containers" one instead.

Last but not least, I've explicitly mentioned where each dependency is
coming from (either advanced-virt, or appstream), so we know all the
packages we're dealing with and where those come from.

The sandboxed-containers extension is added to the MCO with by the
following PR:
openshift/machine-config-operator#2456

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
fidencio added a commit to fidencio/os that referenced this pull request Mar 25, 2021
This reworks af93514 as, since it was
merged, a few changes happened in the enhancement proposal and it's been
decided to go with a "sandboxed-containers" extensions instead.

As the "qemu-kiwi" extension is no longer needed, we're removing it
(well, reworking it) and adding the "sandboxed-containers" one instead.

Last but not least, I've explicitly mentioned where each dependency is
coming from (either advanced-virt, or appstream), so we know all the
packages we're dealing with and where those come from.

The sandboxed-containers extension is added to the MCO with by the
following PR:
openshift/machine-config-operator#2456

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
fidencio added a commit to fidencio/os that referenced this pull request Mar 25, 2021
This reworks af93514 as, since it was
merged, a few changes happened in the enhancement proposal and it's been
decided to go with a "sandboxed-containers" extensions instead.

As the "qemu-kiwi" extension is no longer needed, we're removing it
(well, reworking it) and adding the "sandboxed-containers" one instead.

Last but not least, I've explicitly mentioned where each dependency is
coming from (either advanced-virt, or appstream), so we know all the
packages we're dealing with and where those come from.

The sandboxed-containers extension is added to the MCO with by the
following PR:
openshift/machine-config-operator#2456

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
fidencio added a commit to fidencio/os that referenced this pull request Mar 26, 2021
This reworks af93514 as, since it was
merged, a few changes happened in the enhancement proposal and it's been
decided to go with a "sandboxed-containers" extensions instead.

As the "qemu-kiwi" extension is no longer needed, we're removing it
(well, reworking it) and adding the "sandboxed-containers" one instead.

Last but not least, I've explicitly mentioned where each dependency is
coming from (either advanced-virt, or appstream), so we know all the
packages we're dealing with and where those come from.

The sandboxed-containers extension is added to the MCO with by the
following PR:
openshift/machine-config-operator#2456

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
@fidencio fidencio force-pushed the wip/add-sandboxed-containers-extensions branch from 0b06a80 to 3625f08 Compare March 26, 2021 12:13
@fidencio
Copy link
Contributor Author

This PR has been updated, and depends on openshift/os#525 (which depends on a downstream PR).

fidencio added a commit to fidencio/os that referenced this pull request Mar 31, 2021
This reworks af93514 as, since it was
merged, a few changes happened in the enhancement proposal and it's been
decided to go with a "sandboxed-containers" extensions instead.

As the "qemu-kiwi" extension is no longer needed, we're removing it
(well, reworking it) and adding the "sandboxed-containers" one instead.

Last but not least, I've explicitly mentioned where each dependency is
coming from (either advanced-virt, or appstream), so we know all the
packages we're dealing with and where those come from.

The sandboxed-containers extension is added to the MCO with by the
following PR:
openshift/machine-config-operator#2456

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
@fidencio fidencio force-pushed the wip/add-sandboxed-containers-extensions branch from 3625f08 to 5d9dbeb Compare March 31, 2021 20:06
fidencio added a commit to fidencio/os that referenced this pull request Apr 1, 2021
This reworks af93514 as, since it was
merged, a few changes happened in the enhancement proposal and it's been
decided to go with a "sandboxed-containers" extensions instead.

As the "qemu-kiwi" extension is no longer needed, we're removing it
(well, reworking it) and adding the "sandboxed-containers" one instead.

The sandboxed-containers extension is added to the MCO with by the
following PR:
openshift/machine-config-operator#2456

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
@fidencio
Copy link
Contributor Author

fidencio commented Apr 2, 2021

/retest

4 similar comments
@fidencio
Copy link
Contributor Author

fidencio commented Apr 2, 2021

/retest

@fidencio
Copy link
Contributor Author

fidencio commented Apr 2, 2021

/retest

@fidencio
Copy link
Contributor Author

fidencio commented Apr 2, 2021

/retest

@fidencio
Copy link
Contributor Author

fidencio commented Apr 5, 2021

/retest

Copy link
Contributor

@sinnykumari sinnykumari left a comment

Choose a reason for hiding this comment

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

looks clean, thanks for the PR!
/lgtm

@openshift-ci-robot openshift-ci-robot 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 Apr 6, 2021
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@fidencio
Copy link
Contributor Author

fidencio commented Apr 6, 2021

/retest

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fidencio, JAORMX, sinnykumari

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

@fidencio
Copy link
Contributor Author

fidencio commented Apr 6, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@fidencio
Copy link
Contributor Author

fidencio commented Apr 6, 2021

/test e2e-agnostic-upgrade

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@fidencio
Copy link
Contributor Author

fidencio commented Apr 6, 2021

/test e2e-aws-serial

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Apr 7, 2021

@fidencio: The following test failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/e2e-aws-workers-rhel7 8d27bf8 link /test e2e-aws-workers-rhel7

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.

@openshift-merge-robot openshift-merge-robot merged commit ca8fcb8 into openshift:master Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4.8 Work deferred for 4.8 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.

7 participants