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: Fix summary page creation #16599

Merged
merged 1 commit into from
Oct 3, 2024
Merged

Conversation

mcmilk
Copy link
Contributor

@mcmilk mcmilk commented Oct 3, 2024

Motivation and Context

There are cases, where some needed files for the summary page aren't created. Currently the whole Summary Page creation will fail then. Sample run: https://github.com/openzfs/zfs/actions/runs/11148248072/job/30999748588

Fix this, by properly checking for existence of the needed files.

How Has This Been Tested?

The ZTS will do.

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:

There are cases, where some needed files for the summary page aren't
created. Currently the whole Summary Page creation will fail then.
Sample run: https://github.com/openzfs/zfs/actions/runs/11148248072/job/30999748588

Fix this, by properly checking for existence of the needed files.

Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Copy link
Member

@robn robn left a comment

Choose a reason for hiding this comment

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

Two earnest and dumb questions:

  • is a missing file interesting? Would it be better to put [uname.txt not found] in the report instead?
  • -s will skip zero-byte files too. Should it be -e? and/or, put [uname.txt empty] in the report?

@mcmilk
Copy link
Contributor Author

mcmilk commented Oct 3, 2024

Two earnest and dumb questions:

  • is a missing file interesting? Would it be better to put [uname.txt not found] in the report instead?

No, we have the red light that indicates that e.g. the module build has failed.

  • -s will skip zero-byte files too. Should it be -e? and/or, put [uname.txt empty] in the report?

When uname.txt exists, then there should be some content in it, thats why I used -s :)

@behlendorf behlendorf added the Status: Accepted Ready to integrate (reviewed, tested) label Oct 3, 2024
@behlendorf behlendorf merged commit 3d0175d into openzfs:master Oct 3, 2024
13 of 14 checks passed
@robn
Copy link
Member

robn commented Oct 3, 2024

Told you they were dumb 😊

tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Oct 3, 2024
There are cases, where some needed files for the summary page aren't
created. Currently the whole Summary Page creation will fail then.
Sample run: https://github.com/openzfs/zfs/actions/runs/11148248072/job/30999748588

Fix this, by properly checking for existence of the needed files.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rob Norris <robn@despairlabs.com>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16599
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Oct 3, 2024
There are cases, where some needed files for the summary page aren't
created. Currently the whole Summary Page creation will fail then.
Sample run: https://github.com/openzfs/zfs/actions/runs/11148248072/job/30999748588

Fix this, by properly checking for existence of the needed files.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rob Norris <robn@despairlabs.com>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16599
tonyhutter pushed a commit to tonyhutter/zfs that referenced this pull request Oct 3, 2024
There are cases, where some needed files for the summary page aren't
created. Currently the whole Summary Page creation will fail then.
Sample run: https://github.com/openzfs/zfs/actions/runs/11148248072/job/30999748588

Fix this, by properly checking for existence of the needed files.

Reviewed-by: Tony Hutter <hutter2@llnl.gov>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Rob Norris <robn@despairlabs.com>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16599
@mcmilk mcmilk deleted the fix-zts-uname branch October 4, 2024 05:09
mcmilk added a commit to mcmilk/zfs that referenced this pull request Oct 6, 2024
In PR openzfs#16599 I used 'return' like in C - which is wrong :/
This fix generates the summary as needed.

Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
behlendorf pushed a commit that referenced this pull request Oct 6, 2024
In PR #16599 I used 'return' like in C - which is wrong :/
This fix generates the summary as needed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes #16611
ixhamza pushed a commit to truenas/zfs that referenced this pull request Oct 8, 2024
In PR openzfs#16599 I used 'return' like in C - which is wrong :/
This fix generates the summary as needed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16611
ixhamza pushed a commit to truenas/zfs that referenced this pull request Oct 9, 2024
In PR openzfs#16599 I used 'return' like in C - which is wrong :/
This fix generates the summary as needed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16611
behlendorf pushed a commit to behlendorf/zfs that referenced this pull request Oct 9, 2024
In PR openzfs#16599 I used 'return' like in C - which is wrong :/
This fix generates the summary as needed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16611
ixhamza pushed a commit to truenas/zfs that referenced this pull request Oct 9, 2024
In PR openzfs#16599 I used 'return' like in C - which is wrong :/
This fix generates the summary as needed.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Tino Reichardt <milky-zfs@mcmilk.de>
Closes openzfs#16611
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.

4 participants