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

ZTS: add centos stream10 #16904

Merged
merged 1 commit into from
Jan 2, 2025
Merged

ZTS: add centos stream10 #16904

merged 1 commit into from
Jan 2, 2025

Conversation

hanthor
Copy link
Contributor

@hanthor hanthor commented Dec 26, 2024

Motivation and Context

CentOS-stream10 is out. It would be nice to have ZFS support

We will be adding stream9 and stream10 as optional runners for testing incoming EL changes. As CentOS Stream does not have official ZFS support

Description

I copied the work to add fedora 41 from here: #16700

How Has This Been Tested?

It has been built in CI on my fork, will be testing in CI with this draft PR

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
  • Documentation (a change to man pages or other documentation)

Checklist:

@github-actions github-actions bot added the Status: Work in Progress Not yet ready for general review label Dec 26, 2024
@hanthor hanthor marked this pull request as ready for review December 26, 2024 15:57
@github-actions github-actions bot added Status: Code Review Needed Ready for review and testing and removed Status: Work in Progress Not yet ready for general review labels Dec 26, 2024
@hanthor
Copy link
Contributor Author

hanthor commented Dec 26, 2024

Some tests failed:

 Results Summary
PASS	1792
FAIL	13
SKIP	12

Running Time:	05:11:29
Percent passed:	98.62%

Tests with results other than PASS that are expected:
    FAIL append/threadsappend_001_pos (https://github.com/openzfs/zfs/issues/6136)
    FAIL casenorm/mixed_formd_delete (https://github.com/openzfs/zfs/issues/7633)
    FAIL casenorm/mixed_formd_lookup (https://github.com/openzfs/zfs/issues/7633)
    FAIL casenorm/mixed_formd_lookup_ci (https://github.com/openzfs/zfs/issues/7633)
    FAIL casenorm/mixed_none_lookup_ci (https://github.com/openzfs/zfs/issues/7633)
    FAIL casenorm/sensitive_formd_delete (https://github.com/openzfs/zfs/issues/7633)
    FAIL casenorm/sensitive_formd_lookup (https://github.com/openzfs/zfs/issues/7633)
    FAIL cli_root/zpool_import/import_rewind_device_replaced (Arbitrary pool rewind is not guaranteed)
    FAIL fault/auto_spare_multiple (https://github.com/openzfs/zfs/issues/11889)
    FAIL refreserv/refreserv_004_pos (Known issue)
    FAIL vdev_zaps/vdev_zaps_007_pos (Known issue)
    SKIP cli_root/zfs_unshare/zfs_unshare_006_pos (Not applicable)
    SKIP cli_root/zpool_import/zpool_import_missing_003_pos (https://github.com/openzfs/zfs/issues/6839)
    SKIP pool_checkpoint/checkpoint_discard_busy (https://github.com/openzfs/zfs/issues/12053)
    SKIP removal/removal_with_zdb (Known issue)
    SKIP rsend/rsend_008_pos (https://github.com/openzfs/zfs/issues/6066)


Tests with result of PASS that are unexpected:


Tests with results other than PASS that are unexpected:
    FAIL direct/dio_async_fio_ioengines (expected PASS)
    FAIL io/io_uring (expected PASS)

Logs-functional-centos-stream10.zip

@tonyhutter
Copy link
Contributor

I like more targets to test on, but I'm reluctant to automatically test on CentOS Stream by default. Stream is not supported by OpenZFS since it's such a moving target. I would hate to have people's PRs suddenly have test failures simply because Stream changed something under the rug.

That said, would you consider making this an optional, workflow_dispatch based runner, that you could manually run? You could even add some checkboxes to select which optional OSs to test on. I can see this also being useful for adding in obscure OSs or architectures that we don't want to test on by default.

Example:

image

(code below is untested)

diff --git a/.github/workflows/zfs-qemu.yml b/.github/workflows/zfs-qemu.yml
index 4748e90db..1d72650de 100644
--- a/.github/workflows/zfs-qemu.yml
+++ b/.github/workflows/zfs-qemu.yml
@@ -3,6 +3,18 @@ name: zfs-qemu
 on:
   push:
   pull_request:
+  workflow_dispatch:
+    inputs:
+      include_stream9:
+        type: boolean
+        required: false
+        default: false
+        description: 'Test on CentOS 9 stream'
+      include_stream10:
+        type: boolean
+        required: false
+        default: false
+        description: 'Test on CentOS 10 stream'
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
@@ -37,6 +49,15 @@ jobs:
             os_selection="$FULL_OS"
           fi
           os_json=$(echo ${os_selection} | jq -c)
+
+          # Add on optional runners
+          if [ ${{ github.event.inputs.include_stream9 }} == "true" ] ; then
+                os_json="$(echo $os_json | jq -c '. += ["centos-stream9"]')"
+          fi
+          if [ ${{ github.event.inputs.include_stream10 }} == "true" ] ; then
+                os_json="$(echo $os_json | jq -c '. += ["centos-stream10"]')"
+          fi
+
           echo "os=$os_json" >> $GITHUB_OUTPUT
           echo "ci_type=$ci_type" >> $GITHUB_OUTPUT

NOTE! If you want these nice workflow_dispatch checkboxes you have to check the code in to the master branch first. If you just open a PR, they will not show up in the "Actions" tab in github. So what I do is temporarly check in my changes to my local zfs master repo, test it out, and then git revert --hard <last-good-commit> to get rid of the temporary changes I made to my master

@hanthor
Copy link
Contributor Author

hanthor commented Dec 27, 2024

So should I remove stream9 from FULL_OS? I went ahead and and removed stream9 from the FULL_OS matrix as almalinux9 is already in there. I'm assuming testing against stream periodically is good from a EL-next standpoint. Here is the workflow_dispatch you're code worked as-is:

image

See results of https://github.com/hanthor/zfs/actions/runs/12512921350/job/34906851668 for stream10 test results.

@tonyhutter
Copy link
Contributor

So should I remove stream9 from FULL_OS

Yes, I think that centos9-stream should be optionally run via workflow_dispatch as you have done. That will free up a runner to for the automatic runs too.

Added centos as optional runners via workflow_dispatch

removed centos-stream9 from the FULL_OS runner list as CentOS is not
officially support by ZFS. This commit will add preliminary support for
EL10 and allow testing ZFS ahead of EL10 codebase solidifying in ~6
months

Signed-off-by: James Reilly <[email protected]>
@tonyhutter tonyhutter merged commit 3c2267a into openzfs:master Jan 2, 2025
18 checks passed
@behlendorf behlendorf added Status: Accepted Ready to integrate (reviewed, tested) and removed Status: Code Review Needed Ready for review and testing labels Jan 2, 2025
@carlwgeorge
Copy link

Stream is not supported by OpenZFS since it's such a moving target. I would hate to have people's PRs suddenly have test failures simply because Stream changed something under the rug.

CentOS Stream is objectively less of a moving target than Fedora, yet Fedora releases are in FULL_OS list. Logically either CentOS Stream belongs in that list too, or Fedora releases should be optional like CentOS Stream.

@tonyhutter
Copy link
Contributor

@carlwgeorge ah that's true, and thinking about it Fedora would catch 99% of those issues first anyway. That said, I don't know what testing Stream by default really gets us that Fedora+Alma does not. Adding more runners isn't free since we're limited to ~20 concurrent or something by our GH account. There may be value in a build-only Stream runner that doesn't run the test suite though. We've run into a few instances where RHEL's Frankenstein kernel has pulled in code from newer kernels that broke our build, for example.

@carlwgeorge
Copy link

Testing on CentOS Stream would give you several months advance notice of changes coming to the RHEL kernel. CentOS Stream 9 has RHEL 9.6 kernel changes while RHEL 9 and RHEL 9 derivatives are still on 9.5. CentOS Stream 10 has RHEL 10.0 kernel changes before RHEL 10.0 is released (which are not part of RHEL 10 Beta). CentOS Stream changes are not just random things thrown at the wall, they are changes that have passed RHEL QA that are queued up for the next minor version of RHEL. If a change there is going to affect your builds you'll have to deal with it eventually, either when it's available in CentOS Stream or when it lands in RHEL. Only testing on Fedora and Alma leaves you with a significant blind spot where incoming RHEL changes are known publicly but not tested against. This blind spot results in longer delays in offering compatibility with new RHEL minor versions. See also #11320 (comment).

behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Jan 3, 2025
Added centos as optional runners via workflow_dispatch

removed centos-stream9 from the FULL_OS runner list as CentOS is not
officially support by ZFS. This commit will add preliminary support for
EL10 and allow testing ZFS ahead of EL10 codebase solidifying in ~6
months

Signed-off-by: James Reilly <[email protected]>
Reviewed-by: Tony Hutter <[email protected]>
Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Tino Reichardt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Accepted Ready to integrate (reviewed, tested)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants