MCO-1100: enable RHEL entitlements in on-cluster layering #4312
Conversation
|
@cheesesashimi: This pull request references MCO-1100 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.16.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. |
openshift-ci-robot
added
the
jira/valid-reference
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-merge-robot
added
the
needs-rebase
openshift-ci
bot
added
the
approved
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
from
a2e8d5e to
77b9422
Compare
openshift-merge-robot
removed
the
needs-rebase
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
3 times, most recently
from
a38312d to
c16775a
Compare
|
Checking to see if my Dockerfile changes had anything to do with this: /test e2e-gcp-op-techpreview |
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
from
9c3aab3 to
e93591a
Compare
|
@cheesesashimi: This pull request references MCO-1100 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.16.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. |
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
4 times, most recently
from
b18108f to
7fd8a8b
Compare
|
@cheesesashimi: This pull request references MCO-1100 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.16.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. |
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
3 times, most recently
from
1739853 to
274e37f
Compare
|
@cheesesashimi: This pull request references MCO-1100 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.16.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. |
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
2 times, most recently
from
5614c7e to
10e78aa
Compare
|
@cheesesashimi: This pull request references MCO-1100 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.16.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. |
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
from
10e78aa to
b52f3f3
Compare
|
@cheesesashimi: This pull request references MCO-1100 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.16.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. |
This adds the capability for BuildController to use the RHEL entitlement secrets to allow cluster admins to inject RHEL content into their builds that they are entitled to receive. This also allows the injection / consumption of content into /etc/yum.repos.d as well as /etc/pki/rpm-gpg. There are a few notes about the implementation that I would like to have at a higher level: - Because we run rootless Buildah, we're more prone to running into SELinux complications. This makes it more difficult to directly mount the contents of /etc/yum.repos.d, /etc/pki/entitlement, and /etc/pki/rpm-gpg directly into the build context. With that in mind, we copy everything into a series of temp directories first, and then mount those temp directories into the build context as a volume. - We also create an emptyDir which is mounted into the build pod at /home/build/.local/share/containers. It is unclear why this is necessary, but as mentioned before, I suspect that this is due to SELinux issues. - The e2e test suite now has the capability to stream the container logs from the build pod to the filesystem as there is useful information contained within those logs if the e2e test fails. In OpenShift CI, this location will be determined by the ARTIFACT_DIR env var. If this env var is not present, it will default the current directory.
cheesesashimi
force-pushed
the
zzlotnik/rhel-entitlements
branch
from
b52f3f3 to
ee44666
Compare
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/refresh-jira |
|
/jira refresh |
|
@cheesesashimi: This pull request references MCO-1100 which is a valid jira issue. 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. |
|
@cheesesashimi: 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/test-infra repository. I understand the commands that are listed here. |
|
/test e2e-aws-ovn This was a transient failure due to AWS capacity constraints: level=error msg=Error: creating EC2 Instance: InsufficientInstanceCapacity: We currently do not have sufficient m6a.xlarge capacity in the Availability Zone you requested (us-east-2a). Our system will be working on provisioning additional capacity. You can currently get m6a.xlarge capacity by not specifying an Availability Zone in your request or choosing us-east-2b, us-east-2c. |
|
Tested locally on an AWS cluster and RHEL entitlement worked fine with this PR. nodes opted to layered pool got updated with |
| # If we have /etc/pki/entitlement certificates, commonly used with RHEL | ||
| # entitlements, copy them into a tempdir to avoid SELinux issues, and tell | ||
| # Buildah about them. | ||
| if [[ -n "$ETC_PKI_ENTITLEMENT_MOUNTPOINT" ]] && [[ -d "$ETC_PKI_ENTITLEMENT_MOUNTPOINT" ]]; then | ||
| configs="$(mktemp -d)" | ||
| cp -r -v "$ETC_PKI_ENTITLEMENT_MOUNTPOINT/." "$configs" | ||
| chmod -R 0755 "$configs" | ||
| build_args+=("--volume=$configs:$ETC_PKI_ENTITLEMENT_MOUNTPOINT:$mount_opts") | ||
| fi | ||
|
|
||
| # If we have /etc/yum.repos.d configs, commonly used with Red Hat Satellite | ||
| # subscriptions, copy them into a tempdir to avoid SELinux issues, and tell | ||
| # Buildah about them. | ||
| if [[ -n "$ETC_YUM_REPOS_D_MOUNTPOINT" ]] && [[ -d "$ETC_YUM_REPOS_D_MOUNTPOINT" ]]; then | ||
| configs="$(mktemp -d)" | ||
| cp -r -v "$ETC_YUM_REPOS_D_MOUNTPOINT/." "$configs" | ||
| chmod -R 0755 "$configs" | ||
| build_args+=("--volume=$configs:$ETC_YUM_REPOS_D_MOUNTPOINT:$mount_opts") | ||
| fi | ||
|
|
||
| # If we have /etc/pki/rpm-gpg configs, commonly used with Red Hat Satellite | ||
| # subscriptions, copy them into a tempdir to avoid SELinux issues, and tell | ||
| # Buildah about them. | ||
| if [[ -n "$ETC_PKI_RPM_GPG_MOUNTPOINT" ]] && [[ -d "$ETC_PKI_RPM_GPG_MOUNTPOINT" ]]; then | ||
| configs="$(mktemp -d)" | ||
| cp -r -v "$ETC_PKI_RPM_GPG_MOUNTPOINT/." "$configs" | ||
| chmod -R 0755 "$configs" | ||
| build_args+=("--volume=$configs:$ETC_PKI_RPM_GPG_MOUNTPOINT:$mount_opts") | ||
| fi |
There was a problem hiding this comment.
For modularity, you could make a function that encapsulates this, only if you wanted to. This isnt really a mandatory, as your code looks good! just a suggestion:
function prepare_and_mount_dir {
if [[ -n "$mount_point" ]] && [[ -d "$mount_point" ]]; then
configs=$(mktemp -d)
cp -r -v "$mount_point/." "$configs"
chmod -R 0755 "$configs"
build_args+=("--volume=$configs:$mount_point:$mount_opts")
fi
}
prepare_and_mount_dir "RHSM Certs" "$rhsm_path" "rhsm_certs"
prepare_and_mount_dir "RPM-GPG Configs" "$ETC_PKI_RPM_GPG_MOUNTPOINT" "rpm_gpg_configs"
There was a problem hiding this comment.
I tried to do that, but I couldn't get it to work quite the way that I wanted it to. Admittedly, my Bash is a little rusty. But I did think of two interesting paths forward for the future:
- There is a Python3 interpreter available in the official Buildah image, the MCO image, and the RHCOS image. So if I were so inclined, I could re-write this in Python. That would open the door to writing unit tests around the script. Although, one doesn't strictly need Python to do unit tests since Bats exists.
- Use Go instead of Bash to orchestrate things. Instead of this Bash script, we would add another binary to the MCO container which would get called instead. As a starting point, this binary could do what this Bash script does, but it could eventually do so much more.
|
|
||
| // Fetches an optional secret to inject into the build. Returns a nil error if | ||
| // the secret is not found. | ||
| func (ctrl *Controller) getOptionalSecret(secretName string) (*corev1.Secret, error) { | ||
| optionalSecret, err := ctrl.kubeclient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), secretName, metav1.GetOptions{}) | ||
| if err == nil { | ||
| klog.Infof("Optional build secret %q found, will include in build", secretName) | ||
| return optionalSecret, nil | ||
| } | ||
|
|
||
| if k8serrors.IsNotFound(err) { | ||
| klog.Infof("Could not find optional secret %q, will not include in build", secretName) | ||
| return nil, nil | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("could not retrieve optional secret: %s: %w", secretName, err) | ||
| } | ||
|
|
||
| // Fetches an optional ConfigMap to inject into the build. Returns a nil error if | ||
| // the ConfigMap is not found. | ||
| func (ctrl *Controller) getOptionalConfigMap(configmapName string) (*corev1.ConfigMap, error) { | ||
| optionalConfigMap, err := ctrl.kubeclient.CoreV1().ConfigMaps(ctrlcommon.MCONamespace).Get(context.TODO(), configmapName, metav1.GetOptions{}) | ||
| if err == nil { | ||
| klog.Infof("Optional build ConfigMap %q found, will include in build", configmapName) | ||
| return optionalConfigMap, nil | ||
| } | ||
|
|
||
| if k8serrors.IsNotFound(err) { | ||
| klog.Infof("Could not find ConfigMap %q, will not include in build", configmapName) | ||
| return nil, nil | ||
| } | ||
|
|
||
| return nil, fmt.Errorf("could not retrieve optional ConfigMap: %s: %w", configmapName, err) | ||
| } |
There was a problem hiding this comment.
another non mandatory suggestion. these can be combined into a "getOptionalResource" and just pass in a resource type, do something like, then combine the rest
if resourceType == "Secret" {
resource, err = ctrl.kubeclient.CoreV1().Secrets(ctrlcommon.MCONamespace).Get(context.TODO(), resourceName, metav1.GetOptions{})
} else if resourceType == "ConfigMap" {
resource, err = ctrl.kubeclient.CoreV1().ConfigMaps(ctrlcommon.MCONamespace).Get(context.TODO(), resourceName, metav1.GetOptions{})
}
There was a problem hiding this comment.
This is a good idea for the future, especially since both of these helpers only really concern themselves about the existence of the resource. That would blend nicely with some future refactoring ideas that I have.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cheesesashimi, dkhater-redhat 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 |
|
@cheesesashimi: This pull request references MCO-1100 which is a valid jira issue. 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. |
|
Verification steps:
Build was successfully executed, in the logs we can see the buildah installation: In the node we have access to buildah /label qe-approved |
openshift-ci
bot
added
the
qe-approved
|
@cheesesashimi: This pull request references MCO-1100 which is a valid jira issue. 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. |
|
These changes were incorporated into #4327, so this PR can be closed. |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. 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. |
- What I did
This adds the capability for BuildController to use the RHEL entitlement secrets to allow cluster admins to inject RHEL content into their builds that they are entitled to receive. This also allows the injection / consumption of content into
/etc/yum.repos.das well as/etc/pki/rpm-gpg. There are a few notes about the implementation that I would like to have at a higher level:/etc/yum.repos.d,/etc/pki/entitlement, and/etc/pki/rpm-gpgdirectly into the build context. With that in mind, we copy everything into a series of temp directories first, and then mount those temp directories into the build context as a volume.emptyDirwhich is mounted into the build pod at/home/build/.local/share/containers. It is unclear why this is necessary, but as mentioned before, I suspect that this is due to SELinux issues.ARTIFACT_DIRenv var. If this env var is not present, it will default the current directory.- How to verify it
Automated verification:
etc-pki-entitlementexists in theopenshift-config-managednamespace. If this secret is not present,TestEntitledBuildsandTestEntitledBuildsRollsOutImagewill be skipped.$ go test -count=1 -v ./test/e2e-techpreview/...(Note: Because we have not landed #4284, the cleanup / teardown will delete the node and its underlying machine, causing the Machine API to provision a replacement node.)
Semi-manual verification:
etc-pki-entitlementexists in theopenshift-config-managednamespace.onclustertestinghelper in your$PATH, run the following:$ onclustertesting setup in-cluster-registry --enable-featuregate --pool=layered --custom-dockerfile=./path/to/the/Dockerfilemachine-os-builderpod to start. Shortly afterward, the build pod should start. It should complete without any errors.- Description for the changelog
Enables RHEL entitlements in on-cluster layering