Skip to content

Update centos distribution to scos as this prevents scos build testing#2915

Merged
HuijingHei merged 1 commit intocoreos:mainfrom
HuijingHei:distribution-scos
Jul 21, 2022
Merged

Update centos distribution to scos as this prevents scos build testing#2915
HuijingHei merged 1 commit intocoreos:mainfrom
HuijingHei:distribution-scos

Conversation

@HuijingHei
Copy link
Copy Markdown
Contributor

@HuijingHei HuijingHei commented Jun 10, 2022

See openshift/os#901, when running kola test for scos, get error, this is to fix it

$ cosa kola run ostree.basic
kola -p qemu-unpriv --output-dir tmp/kola run ostree.basic
=== RUN   ostree.basic
--- FAIL: ostree.basic (57.14s)
        harness.go:1443: mach.Start() failed: machine "ea43750e-e04b-4632-b966-caff659a5214" failed basic checks: not a supported instance: scos-coreos
FAIL, output in tmp/kola
Error: harness: test suite failed
2022-06-10T09:12:23Z cli: harness: test suite failed

@HuijingHei HuijingHei changed the title Update centos distribution to scos as this prevents exec testing Update centos distribution to scos as this prevents scos build testing Jun 10, 2022
@travier
Copy link
Copy Markdown
Member

travier commented Jun 10, 2022

Can you make it support all four:

  • scos-
  • scos-coreos
  • rhcos-
  • rhcos-coreos
    so that we can pick & choose freely later which one we want to keep?

@HuijingHei
Copy link
Copy Markdown
Contributor Author

Can you make it support all four:

  • scos-
  • scos-coreos
  • rhcos-
  • rhcos-coreos
    so that we can pick & choose freely later which one we want to keep?

Looks sane, update is done

cgwalters
cgwalters previously approved these changes Jun 10, 2022
Copy link
Copy Markdown
Member

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Personally I think we should just delete this distribution checking code, it's IMO an anti-pattern.

That said, where is scos-coreos and rhcos-coreos coming from? That seems redundant as "rhcos" is short for "rhel coreos" so this is like "rhel coreos coreos" right?

@HuijingHei
Copy link
Copy Markdown
Contributor Author

That said, where is scos-coreos and rhcos-coreos coming from? That seems redundant as "rhcos" is short for "rhel coreos" so this is like "rhel coreos coreos" right?

Refer to:

out, stderr, err := m.SSH(`. /etc/os-release && echo "$ID-$VARIANT_ID"`)

When $ID and $VARIANT_ID are both defined in /etc/os-release, the value will be like scos-coreos (for example in openshift/os#773).

Maybe checking $ID (rhcos or scos) is enough, but for fedora coreos the $ID is fedora which might cause confusion with original fedora

@travier
Copy link
Copy Markdown
Member

travier commented Jun 13, 2022

I'm looking at making this less redundant but changing the defaults here might have impacts in other places thus it's easier to add VARIANT_ID=coreos than change existing values.

travier
travier previously approved these changes Jun 13, 2022
Supported:
- scos-
- scos-coreos
- rhcos-
- rhcos-coreos

See openshift/os#773
Fix issue
```
$ cosa kola run ostree.basic
        harness.go:1443: mach.Start() failed: machine "ea43750e-e04b-4632-b966-caff659a5214" failed basic checks: not a supported instance: scos-coreos
```
@HuijingHei HuijingHei dismissed stale reviews from travier and cgwalters via af1ec01 July 21, 2022 08:13
@HuijingHei HuijingHei enabled auto-merge (rebase) July 21, 2022 10:42
@HuijingHei HuijingHei merged commit 4ce6255 into coreos:main Jul 21, 2022
@HuijingHei HuijingHei deleted the distribution-scos branch August 2, 2022 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants