Skip to content

Updates for variant & yumrepo support in COSA#958

Merged
openshift-merge-robot merged 4 commits intoopenshift:masterfrom
travier:cosa-variant
Aug 31, 2022
Merged

Updates for variant & yumrepo support in COSA#958
openshift-merge-robot merged 4 commits intoopenshift:masterfrom
travier:cosa-variant

Conversation

@travier
Copy link
Copy Markdown
Member

@travier travier commented Aug 25, 2022

manifests: Rework symlinks to match variant support in COSA

To make the logic simpler, we're carrying the variant <-> version logic
with symlinks in this repo

See COSA support in coreos/coreos-assembler#2934


manifests: Fix 'distro' variable for C9S


kola-denylist & ci: Use COSA variant support


docs: Update for variant & yumrepo support

@openshift-ci openshift-ci Bot requested review from HuijingHei and aaradhak August 25, 2022 19:42
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2022
travier added a commit to travier/coreos-assembler that referenced this pull request Aug 25, 2022
On top of the defaut variant, a config repo may include aditional
optional variants.

The variant used is selected at `cosa init` with the `--variant` option
and the value is recorded locally in `src/config.json`.

See:
- openshift/os#852
- openshift/os#901
- openshift/os#958

Reworks: 480c239 init: Require specifying a config repository
@HuijingHei
Copy link
Copy Markdown
Contributor

I am OK with this, just confused about the manifest.yaml -> manifest-rhel-coreos-8.yaml -> manifest-rhel-8.6.yaml, is there any concern to do this? Why not ln manifest.yaml -> manifest-rhel-8.6.yaml directly?

@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 26, 2022

Agree that this is confusing. I'll update the docs and add a note like below to explain.

The idea is that we want to have a default variant (no suffix) that is the one we want to build by default when you clone the repo. Right now this is the one shipped in production (built by the pipelines) but we might want to switch to SCOS at some point.

We also want explicit options to be able to work on a specific variant by specifying exact RHEL or CentOS Stream versions as variants (rhel-8.6, rhel-9.0, c9s).

But we also want to be able to track major streams/versions as we will make two pipelines, one for RHEL 8 based RHCOS and one for RHEL 9 based RHCOS (plus one for SCOS elsewhere). So we need rhel-coreos-8, rhel-coreos-9 & scos. We could even have scos-9 but we don't really need it right now.

Thus the idea behind the chain of symlinks is that we make explicit those decisions:

  • default points to rhel-coreos-8 as that's what we have supported in the pipelines for now but we might switch to SCOS once we have the new pipelines ready
  • rhel-coreos-8 & rhel-coreos-9 point to the current minor version that we want to ship in the new pipelines

travier added a commit to travier/coreos-assembler that referenced this pull request Aug 26, 2022
On top of the defaut variant, a config repo may include aditional
optional variants.

The variant used is selected at `cosa init` with the `--variant` option
and the value is recorded locally in `src/config.json`.

See:
- openshift/os#852
- openshift/os#901
- openshift/os#958

Reworks: 480c239 init: Require specifying a config repository
@travier travier changed the title manifests: Rework symlinks to match variant support in COSA Update for variant & yumrepo support in COSA Aug 26, 2022
@travier travier changed the title Update for variant & yumrepo support in COSA Updatesfor variant & yumrepo support in COSA Aug 26, 2022
@travier travier changed the title Updatesfor variant & yumrepo support in COSA Updates for variant & yumrepo support in COSA Aug 26, 2022
travier added a commit to travier/coreos-assembler that referenced this pull request Aug 26, 2022
On top of the defaut variant, a config repo may include aditional
optional variants.

The variant used is selected at `cosa init` with the `--variant` option
and the value is recorded locally in `src/config.json`.

See:
- openshift/os#852
- openshift/os#901
- openshift/os#958

Reworks: 480c239 init: Require specifying a config repository
Copy link
Copy Markdown
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot for updating the docs.

Comment thread ci/prow-entrypoint.sh
"rhcos-cosa-prow-pr-ci")
setup_user
cosa_init
cosa_init "rhel-coreos-8"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Bikeshed: WDYT about rhcos8 and rhcos9 instead? Seems closer to what we're used to type and matches the other scos better too.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree typing rhcos is more convenient in quick emails and interactive chat. But for anything a bit more formal I'd like to move to being more explicit because the world can only have so many acronyms. "rhel-coreos" is not that much longer and is much, much more self-describing, even though it still contains one acronym.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I suppose a counterargument is the plethora of rhcos- in https://mirror.openshift.com/pub/openshift-v4/dependencies/rhcos/4.11/latest/ ...but...we can change that.

Copy link
Copy Markdown
Member Author

@travier travier Aug 26, 2022

Choose a reason for hiding this comment

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

We've started using rhel-coreos-8 & rhel-coreos-9 for the machine-os-content & container names somewhere but I don't remember exactly where thus the change here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think my core argument here is: With the introduction of RHEL9, the abbreviation "rhcos" has become ambiguous outside of the most generic contexts. Specifically, mirror.openshift.com for example can't just use rhcos - it has to become either rhcos8 and rhcos9 as you suggest, but I think while we are renaming things, let's use rhel-coreos-8 and rhel-coreos-9 there and in our disk images in general.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No strong argument against rhel-coreos-N, but note that RHCOS as an acronym is pretty established at this point, including in official OpenShift docs. We do need to start adding the version string, but I'm not sure it follows that we need to also change the prefix.

travier added a commit to travier/coreos-assembler that referenced this pull request Aug 26, 2022
On top of the defaut variant, a config repo may include aditional
optional variants.

The variant used is selected at `cosa init` with the `--variant` option
and the value is recorded locally in `src/config.json`.

See:
- openshift/os#852
- openshift/os#901
- openshift/os#958

Reworks: 480c239 init: Require specifying a config repository
@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 26, 2022

I think we need to force a rebuild of the layered cosa image.

/retest all

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 26, 2022

@jlebon: The /retest command does not accept any targets.
The following commands are available to trigger required jobs:

  • /test images
  • /test periodic-images
  • /test rhcos-86-build-test-metal
  • /test rhcos-86-build-test-qemu
  • /test rhcos-90-build-test-metal
  • /test rhcos-90-build-test-qemu
  • /test scos-9-build-test-metal
  • /test scos-9-build-test-qemu
  • /test validate

Use /test all to run all jobs.

Details

In response to this:

I think we need to force a rebuild of the layered cosa image.

/retest all

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.

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 26, 2022

/test images

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 26, 2022

/retest

1 similar comment
@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 26, 2022

/retest

@jlebon
Copy link
Copy Markdown
Member

jlebon commented Aug 26, 2022

CI failure looks legit now.

travier added a commit to coreos/coreos-assembler that referenced this pull request Aug 29, 2022
On top of the defaut variant, a config repo may include aditional
optional variants.

The variant used is selected at `cosa init` with the `--variant` option
and the value is recorded locally in `src/config.json`.

See:
- openshift/os#852
- openshift/os#901
- openshift/os#958

Reworks: 480c239 init: Require specifying a config repository
@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 29, 2022

/test images

@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 29, 2022

/retest

@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 29, 2022

/test images

@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 29, 2022

/retest

@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 29, 2022

/test images

@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 29, 2022

/test images
/retest

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 29, 2022
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 29, 2022
@travier
Copy link
Copy Markdown
Member Author

travier commented Aug 30, 2022

/test images
/retest

To make the logic simpler, we're carrying the variant <-> version logic
with symlinks in this repo

See COSA support in coreos/coreos-assembler#2934
@travier travier force-pushed the cosa-variant branch 2 times, most recently from 17ca171 to 60b1a8c Compare August 30, 2022 15:27
- Use a single kola denylist
- Remove setup variant workarounds
- Use an explicit variant for all test jobs
- Use the variant info to fetch the right repos
@cgwalters
Copy link
Copy Markdown
Member

The failing tests look like flakes to me offhand.
/retest

@cgwalters
Copy link
Copy Markdown
Member

/retest

@cgwalters
Copy link
Copy Markdown
Member

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 31, 2022
@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 31, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, jlebon, travier

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:
  • OWNERS [cgwalters,jlebon,travier]

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

@openshift-ci
Copy link
Copy Markdown
Contributor

openshift-ci Bot commented Aug 31, 2022

@travier: all tests passed!

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 01b7b0b into openshift:master Aug 31, 2022
@travier travier deleted the cosa-variant branch August 31, 2022 10:09
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.

5 participants