-
Notifications
You must be signed in to change notification settings - Fork 413
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
MCO-1331: Install extensions via Containerfile for OCL #4705
Conversation
@umohnani8: This pull request references MCO-1331 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
c612203
to
58a3428
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great overall! I just have a few minor suggestions.
pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template
Outdated
Show resolved
Hide resolved
pkg/controller/build/buildrequest/assets/Containerfile.on-cluster-build-template
Outdated
Show resolved
Hide resolved
2581c8f
to
fff7b19
Compare
/retest |
abd2284
to
b4644d6
Compare
Add logic to the Containerfile used to build the new OS image to be able to install extensions when using OCL. Extensions are installed via rpm-ostree and commited to the container image. Signed-off-by: Urvashi <[email protected]>
b4644d6
to
0ce0cbc
Compare
Pre-merge verified :
For rhel-etitlement based MOSB it is failing and have created a ticket here OCPBUGS-44862 /label qe-approved |
@umohnani8: This pull request references MCO-1331 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
/lgtm |
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to ensure we cover cases where an extensions resolves to multiple RPMs
extensions="{{- range $index, $item := .Extensions }}{{- if $index }} {{ end }}{{$item}}{{- end }}" && \ | ||
echo "Installing packages: $extensions" && \ | ||
rpm-ostree install $extensions && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On RHCOS and SCOS one extension may resolve to multiple RPMs, or an RPM with a different name, which I don't think is currently accounted for, see
machine-config-operator/pkg/daemon/update.go
Lines 1765 to 1780 in 9a8ee32
func getSupportedExtensions() map[string][]string { | |
// In future when list of extensions grow, it will make | |
// more sense to populate it in a dynamic way. | |
// These are RHCOS supported extensions. | |
// Each extension keeps a list of packages required to get enabled on host. | |
return map[string][]string{ | |
"wasm": {"crun-wasm"}, | |
"ipsec": {"NetworkManager-libreswan", "libreswan"}, | |
"usbguard": {"usbguard"}, | |
"kerberos": {"krb5-workstation", "libkadm5"}, | |
"kernel-devel": {"kernel-devel", "kernel-headers"}, | |
"sandboxed-containers": {"kata-containers"}, | |
"sysstat": {"sysstat"}, | |
} | |
} |
machine-config-operator/pkg/daemon/update.go
Lines 1734 to 1739 in 9a8ee32
extensions := getSupportedExtensions() | |
for _, ext := range added { | |
for _, pkg := range extensions[ext] { | |
extArgs = append(extArgs, "--install", pkg) | |
} | |
} |
On FCOS, the relationship is 1:1, but I'm not sure whether we still need to handle that now that OKD is moving to SCOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened #4714 whose commits we could cherry-pick into this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @cheesesashimi, I have cherry-picked those commits here
name: "With extensions image and extensions", | ||
optsFunc: func() BuildRequestOpts { | ||
opts := getBuildRequestOpts() | ||
opts.MachineConfig.Spec.Extensions = []string{"usbguard"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usbguard is one of the more trivial extensions that contains only one RPM with the same name. Could we use e.g. the ipsec
extension (which resolves to NetworkManager-libreswan
and libreswan
RPMs) for the test case instead?
/hold |
I'm not sure if any extensions adds users. If that's the case, we're likely at risk of running into containers/bootc#673 here too. |
/hold cancel |
d2f009b
to
fee1b99
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheesesashimi, LorbusChris, umohnani8 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 |
@umohnani8: The following tests failed, say
Full PR test history. Your PR dashboard. 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-sigs/prow repository. I understand the commands that are listed here. |
c761b7c
into
openshift:master
[ART PR BUILD NOTIFIER] Distgit: ose-machine-config-operator |
- What I did
Add logic to the Containerfile used to build the new OS image to be able to install extensions when using OCL. Extensions are installed via rpm-ostree and commited to the container image.
- How to verify it
Enable OCL and create a MC with to install extensions, wait for the image to build and roll out and the extensions installed should be installed on the nodes.
- Description for the changelog
Install extension via Containerfile when using OCL.