[checkbox-ce-oem] Modify dbus warm cold boot job (BugFix)

[rickwu666666]

• Modify the command called in the job
  Since we change the command name in the test-strict-confinement snap.
• Fix the wrong job name in test-plan
• Change job structure
  Make the job easier to read

Description

We modify the command in the test-strict-confinement snap. Therefore, we modify the command that is called in the job.
Also, fix the wrong job ID that is included in the test plan.

Resolved issues

N/A

Documentation

N/A

Tests

Warm-boot
https://certification.canonical.com/hardware/202304-31485/submission/359313/

Cold-boot
https://certification.canonical.com/hardware/202304-31485/submission/359311/

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.52%. Comparing base (84878c2) to head (9324e7c).
Report is 3 commits behind head on main.

❗ Current head 9324e7c differs from pull request most recent head d3e2ae4. Consider uploading reports for the commit d3e2ae4 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1038      +/-   ##
==========================================
- Coverage   43.21%   40.52%   -2.69%     
==========================================
  Files         356      335      -21     
  Lines       38662    37365    -1297     
  Branches     6561     6348     -213     
==========================================
- Hits        16706    15143    -1563     
- Misses      21293    21585     +292     
+ Partials      663      637      -26     

Flag	Coverage Δ
contrib-provider-ce-oem	29.59% <ø> (-0.89%)	⬇️
provider-genio	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kissiel]

Race condition with the background process scheduling found.

[LiaoU3]

+1

[baconYao]

Race condition with the background process scheduling found.

@rickwu666666 , I think you can reference this PR
#979

[rickwu666666]

I've pushed a fix for the race condition.

[stanley31huang]

In test plan, we are going to compare the logs between the reboot test, so we need to add a job to collect logs before reboot test.

[rickwu666666]

@stanley31huang , Yes you are correct. I've pushed the fix. Thanks for the review.

[kissiel]

Going through an extra snap seems unnecessary. It:

• obfuscates what is being done
• reduces clarity and makes the whole thing less readable and debuggable
• creates unnecessary dependencies

Also there's a serious race condition with Checkbox being created here that may lead to breakage of other jobs that may follow the one modified here.

[kissiel]

Why is going through additional snap needed here?
As far as I understand it, this just reboots the machine, why introduce extra steps, that involve more ways the job can go wrong (calling a snap from a snap is problematic, also debugging becomes more difficult).

[rickwu666666]

Hi @kissiel,
Introducing extra snap is because we want to test those commands in a strict confinement environment. This requirement is from the project Shiner and it is due to a snap issue. The Shiner team requires us to run some test cases that they are concerned about in a strict confinement environment, and that means a snap that is not installed in devmode and also connected necessary interfaces.
And this is kind of a POC and also a short-term solution for the shiner team.

[kissiel]

There are two problems with this line:

• This command creates a race against checkbox. As soon as the dbus call is made to Poweroff, Checkbox proceeds to the next job (and writes progress to the disk, which leads to other IO problems, btw.), and if Checkbox starts the next job before the actual poweroff happens, the next job will be interrupted by the system.

|| exit 1 is redundant, if the first command failed then the error code will be preserved (also if the second one fails).

[rickwu666666]

Hi @kissiel,
Does this race condition happen because the dbus command is called by another SNAP which is outside of the checkbox instead of calling dbus itself? If so, do you have any recommendations for ways to avoid this kind of issue?