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.

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

NO-JIRA: variants: simplify #1502

Merged
merged 4 commits into from
May 9, 2024

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented May 6, 2024

This repo is really confusing to work with because of all the various
tiers of variants we have.

In practice, our production pipelines always specify a concrete variant
to build because the switchover between e.g. 9.2 and 9.4 happens on the
ART side, not RHCOS side. And even in CI, since the script that gets
called by Prow lives here, we can easily control which concrete variant
gets built.

So overall, we don't gain much from trying to have symbolic versionless
variants, but it adds cognitive overhead trying to understand it all.

This patch greatly simplifies things by getting rid of the scos and
rhel-coreos-9 variants. Now, we only have concrete variants.
Document them in the README.

The only symbolic links left are the canonical variantless ones, which
determine the default variant that gets built if no --variant switch
is passed to cosa init.

This is also prep for #799, which will add more concrete variants that
do not bake in the OpenShift components.

@openshift-ci openshift-ci bot requested review from marmijo and prestist May 6, 2024 19:04
@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 6, 2024
c9s.repo Show resolved Hide resolved
Copy link
Member

@travier travier left a comment

Choose a reason for hiding this comment

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

LGTM apart from the repo name change for C9S

@travier
Copy link
Member

travier commented May 7, 2024

CI fixes in openshift/release#51750

@jlebon jlebon mentioned this pull request May 7, 2024
jlebon added 3 commits May 7, 2024 20:41
At this point, we're pretty committed to RHEL 9.4 and it's very unlikely
that we'll revert back to RHEL 9.2. If we do, we can just revert this
PR. This variant isn't even building right now in the pipeline so
there's no attention paid to it and no point in carrying it.
Since we have multiple repos at play here, a naked `baseos` repo name
is ambiguous. Let's add a `c9s-` prefix to make it consistent with the
RHEL ones, e.g. `rhel-9.4-baseos`.
This repo is really confusing to work with because of all the various
tiers of variants we have.

In practice, our production pipelines always specify a concrete variant
to build because the switchover between e.g. 9.2 and 9.4 happens on the
ART side, not RHCOS side. And even in CI, since the script that gets
called by Prow lives here, we can easily control which concrete variant
gets built.

So overall, we don't gain much from trying to have symbolic versionless
variants, but it adds cognitive overhead trying to understand it all.

This patch greatly simplifies things by getting rid of the `scos` and
`rhel-coreos-9` variants. Now, we *only* have concrete variants.
Document them in the README.

The only symbolic links left are the canonical variantless ones, which
determine the default variant that gets built if no `--variant` switch
is passed to `cosa init`.

This is also prep for openshift#799, which will add more concrete variants that
do not bake in the OpenShift components.
@jlebon jlebon force-pushed the pr/c9s-cleanups branch from c1e7929 to 8644a08 Compare May 8, 2024 00:59
@jlebon
Copy link
Member Author

jlebon commented May 8, 2024

/retest

2 similar comments
@jlebon
Copy link
Member Author

jlebon commented May 8, 2024

/retest

@jlebon
Copy link
Member Author

jlebon commented May 8, 2024

/retest

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels May 8, 2024
@jlebon jlebon force-pushed the pr/c9s-cleanups branch from 33cc766 to cdc3cae Compare May 9, 2024 00:35
@jlebon jlebon changed the title variants: simplify NO-JIRA: variants: simplify May 9, 2024
@openshift-ci-robot
Copy link

@jlebon: This pull request explicitly references no jira issue.

In response to this:

This repo is really confusing to work with because of all the various
tiers of variants we have.

In practice, our production pipelines always specify a concrete variant
to build because the switchover between e.g. 9.2 and 9.4 happens on the
ART side, not RHCOS side. And even in CI, since the script that gets
called by Prow lives here, we can easily control which concrete variant
gets built.

So overall, we don't gain much from trying to have symbolic versionless
variants, but it adds cognitive overhead trying to understand it all.

This patch greatly simplifies things by getting rid of the scos and
rhel-coreos-9 variants. Now, we only have concrete variants.
Document them in the README.

The only symbolic links left are the canonical variantless ones, which
determine the default variant that gets built if no --variant switch
is passed to cosa init.

This is also prep for #799, which will add more concrete variants that
do not bake in the OpenShift components.

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 openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label May 9, 2024
@jlebon jlebon force-pushed the pr/c9s-cleanups branch from cdc3cae to 50330d4 Compare May 9, 2024 16:22
There's a messy situation right now where the containers-common package
is higher versioned in OCP than in c9s proper. And because we need the
OCP repo for now to compose SCOS, we get the OCP one, which causes
issues because unlike the c9s version, it doesn't ship the RHEL keys.

Work around this by pinning containers-common to the c9s-appstream repo.

While we're here, improve error-handling so that we output stderr if
podman fails.

See also: openshift#1505 (comment)

Fixes: openshift#1505
@jlebon jlebon force-pushed the pr/c9s-cleanups branch from 50330d4 to 76807f6 Compare May 9, 2024 16:22
Copy link
Contributor

openshift-ci bot commented May 9, 2024

@jlebon: all tests passed!

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.

@marmijo
Copy link
Contributor

marmijo commented May 9, 2024

/lgtm

Copy link
Contributor

openshift-ci bot commented May 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dustymabe, jlebon, marmijo

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:
  • OWNERS [dustymabe,jlebon,marmijo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label May 9, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 2a2ee08 into openshift:master May 9, 2024
7 checks passed
@dustymabe
Copy link
Member

some fallout from this is that older branches ext.config.rpm-ostree.replace-rt-kerneltest are pulling from the master branch and then sed replacing [baseos] to enable the repo and now they are failing:

=== RUN   ext.config.rpm-ostree.replace-rt-kernel
systemctl status kola-runext.service:
× kola-runext.service
     Loaded: loaded (/etc/systemd/system/kola-runext.service; static)
     Active: failed (Result: exit-code) since Thu 2024-05-09 22:09:20 UTC; 381ms ago
   Duration: 2.818s
    Process: 1939 ExecStart=/usr/local/bin/kola-runext-replace-rt-kernel (code=exited, status=1/FAILURE)
   Main PID: 1939 (code=exited, status=1/FAILURE)
        CPU: 154ms

May 09 22:09:17 qemu0 systemd[1]: Started kola-runext.service.
May 09 22:09:17 qemu0 kola-runext-replace-rt-kernel[1945]: + rm -rf '/etc/yum.repos.d/*'
May 09 22:09:17 qemu0 kola-runext-replace-rt-kernel[1947]: + curl -sSLf https://raw.githubusercontent.com/openshift/os/master/c9s.repo -o /etc/yum.repos.d/cs.repo
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1950]: + curl -sSLf https://centos.org/keys/RPM-GPG-KEY-CentOS-Official -o /etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-Official
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1953]: + sed -i 's|^gpgkey.*|gpgkey=file:///etc/pki/rpm-gpg/RPM-GPG-KEY-CentOS-Official|' /etc/yum.repos.d/cs.repo
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1939]: Testing overriding with CentOS Stream kernel
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1955]: + sed -i s/enabled=1/enabled=0/g /etc/yum.repos.d/cs.repo
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1957]: + sed -i '/\[baseos\]/,/^ *\[/ s/enabled=0/enabled=1/' /etc/yum.repos.d/cs.repo
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1959]: + sed -i '/\[appstream\]/,/^ *\[/ s/enabled=0/enabled=1/' /etc/yum.repos.d/cs.repo
May 09 22:09:18 qemu0 kola-runext-replace-rt-kernel[1961]: + rpm-ostree override replace --experimental --from repo=baseos kernel kernel-core kernel-modules kernel-modules-extra kernel-modules-core
May 09 22:09:20 qemu0 kola-runext-replace-rt-kernel[1962]: Checking out tree 2b62a0d...done
May 09 22:09:20 qemu0 kola-runext-replace-rt-kernel[1962]: No enabled rpm-md repositories.
May 09 22:09:20 qemu0 kola-runext-replace-rt-kernel[1962]: Importing rpm-md...done
May 09 22:09:20 qemu0 kola-runext-replace-rt-kernel[1962]: error: No matches for 'kernel' in repo 'baseos'
May 09 22:09:20 qemu0 systemd[1]: kola-runext.service: Main process exited, code=exited, status=1/FAILURE

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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants