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

fix: make network-info jobs require ethtool to be available (bugfix) #492

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

kissiel
Copy link
Contributor

@kissiel kissiel commented May 24, 2023

Description

Make network-info jobs require ethtool.
See bug link for details.

Resolved issues

Fixes: : CHECKBOX-626 (#488)

Testing

Tested on a system without the ethtool and now it yields:

==============[ Running job 1 / 1. Estimated time left: 0:00:01 ]===============
-------------------[ Network Information of device 1 (eno1) ]-------------------
ID: com.canonical.certification::networking/info_device1_eno1
Category: com.canonical.plainbox::networking
Job cannot be started because:
 - resource expression "executable.name == 'ethtool'" evaluates to false
Outcome: job cannot be started
Finalizing session that hasn't been submitted anywhere: checkbox-run-2023-05-24T07.16.45
==================================[ Results ]===================================
 ☑ : Enumerate available system executables
 ☑ : Collect information about hardware devices (udev)
 ☐ : Network Information of device 1 (eno1)

And with it installed it proceeds to run.

Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

Thanks for that fix. A few comments:

  • the bug happens with the snap version of Checkbox, even though it seems that ethtool is pulled at build time. Does modifying the packaging meta-data unit helps in that regard?
  • the job is skipped (providing zero info about the network) instead of running and providing partial information about the network. Not sure if this is what QA team need.

@kissiel
Copy link
Contributor Author

kissiel commented May 24, 2023

Thanks for that fix. A few comments:

I'll make sure it is so.

  • the job is skipped (providing zero info about the network) instead of running and providing partial information about the network. Not sure if this is what QA team need.

I asked Doug that @ the original bug

@kissiel kissiel marked this pull request as draft May 24, 2023 08:04
@pieqq
Copy link
Collaborator

pieqq commented May 26, 2023

Since the original issue was marked as closed, we might want to close this associated PR.

@kissiel
Copy link
Contributor Author

kissiel commented May 26, 2023

Yeah, although it still makes sense to not crash on missing ethtool. Recent releases don't include it by default.

@pieqq
Copy link
Collaborator

pieqq commented Nov 7, 2023

Any plan for this? Do we close it or not?

@kissiel
Copy link
Contributor Author

kissiel commented Jan 5, 2024

The patch still makes sense IMHO..
The job runs ethtool, but doesn't officially require it.

@kissiel kissiel marked this pull request as ready for review January 5, 2024 22:05
@Hook25 Hook25 changed the title fix: make network-info jobs require ethtool to be available fix: make network-info jobs require ethtool to be available (bugfix) Jan 9, 2024
@Hook25 Hook25 enabled auto-merge (squash) January 9, 2024 16:22
Copy link
Collaborator

@Hook25 Hook25 left a comment

Choose a reason for hiding this comment

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

+1, this job should require ethtool as it uses it

@kissiel kissiel force-pushed the add-requirement-for-ethtool branch from e9cf826 to 908c351 Compare January 9, 2024 17:21
@Hook25 Hook25 merged commit b0c9796 into main Jan 9, 2024
5 of 6 checks passed
@Hook25 Hook25 deleted the add-requirement-for-ethtool branch January 9, 2024 17:22
LiaoU3 pushed a commit to LiaoU3/checkbox that referenced this pull request Mar 20, 2024
…nical#492)

fix: make network-info jobs require ethtool to be available

Fixes: canonical#488

Co-authored-by: Devices Certification Bot <robot@canonical.com>
binli pushed a commit to binli/checkbox that referenced this pull request Mar 22, 2024
…nical#492)

fix: make network-info jobs require ethtool to be available

Fixes: canonical#488

Co-authored-by: Devices Certification Bot <robot@canonical.com>
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