Change the check for the existence of cryptsetup command

[lucas-a-martins]

Description

Currently, when starting the Agent, ACS checks if the cryptsetup command can be used by using the command's help; the output of the command is printed in the logs. However, this output is too verbose and can be confusing.

This PR addresses this issue by using the utility version to check the command, instead of help, which is much less verbose.

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

Screenshots (if appropriate):

• Before changes:

• After changes:

How Has This Been Tested?

I checked the logs before and after changes. As shown in the screenshots above, the logs became more intuitive.

[DaanHoogland]

clgtm
@lucas-a-martins why did you close #8480 in favour of this PR? they seem to do the same.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d6ac91f) 30.74% compared to head (0f08216) 30.80%.
Report is 5 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8482      +/-   ##
============================================
+ Coverage     30.74%   30.80%   +0.05%     
- Complexity    33930    33990      +60     
============================================
  Files          5341     5341              
  Lines        374918   374918              
  Branches      54534    54534              
============================================
+ Hits         115286   115499     +213     
+ Misses       244374   244151     -223     
- Partials      15258    15268      +10     

Flag	Coverage Δ
simulator-marvin-tests	24.72% <0.00%> (+0.09%)	⬆️
uitests	4.39% <ø> (ø)
unit-tests	16.46% <100.00%> (ø)

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.

[sureshanaparti]

code lgtm

[sureshanaparti]

@blueorangutan package

[blueorangutan]

@sureshanaparti a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[weizhouapache]

code lgtm

[blueorangutan]

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8263

[lucas-a-martins]

clgtm @lucas-a-martins why did you close #8480 in favour of this PR? they seem to do the same.

@DaanHoogland thanks for your review. About the #8480, it was my first PR. I made some mistakes and didn't know how to fix them, so I thought it would be better just closing it and creating a new one, but now I know it was not a good decision. I will be more careful.

[DaanHoogland]

clgtm @lucas-a-martins why did you close #8480 in favour of this PR? they seem to do the same.

@DaanHoogland thanks for your review. About the #8480, it was my first PR. I made some mistakes and didn't know how to fix them, so I thought it would be better just closing it and creating a new one, but now I know it was not a good decision. I will be more careful.

no problem @lucas-a-martins and no harm done. Just wondering what happened.

[BryanMLima]

I manually tested this patch in a local environment, LGTM.

2024-01-12 13:59:23,198 DEBUG [utils.script.Script] (Agent-Handler-1:null) (logid:e1013b70) Executing command [cryptsetup --usage ].
2024-01-12 13:59:23,219 DEBUG [utils.script.Script] (Agent-Handler-1:null) (logid:e1013b70) Successfully executed process [12069] for command [cryptsetup --usage ].
2024-01-12 13:59:23,220 DEBUG [utils.script.Script] (Agent-Handler-1:null) (logid:e1013b70) Usage: cryptsetup [-?Vvyrq] [-?|--help] [--usage] [-V|--version]
        [-v|--verbose] [--debug] [--debug-json] [-c|--cipher=STRING]
        [-h|--hash=STRING] [-y|--verify-passphrase] [-d|--key-file=STRING]
--
2024-01-12 15:48:06,513 DEBUG [utils.script.Script] (Agent-Handler-1:null) (logid:) Executing command [cryptsetup --version ].
2024-01-12 15:48:06,525 DEBUG [utils.script.Script] (Agent-Handler-1:null) (logid:) Successfully executed process [15380] for command [cryptsetup --version ].
2024-01-12 15:48:06,527 DEBUG [utils.script.Script] (Agent-Handler-1:null) (logid:) cryptsetup 2.2.2

[BryanMLima]

Merging this based on four approvals and manual test.

[boring-cyborg]

Awesome work, congrats on your first merged pull request!

[DaanHoogland]

Hm @BryanMLima , we were still in freeze. don't think it will be a problem though. cc @shwstppr .

[BryanMLima]

@DaanHoogland @shwstppr, my mistake, I can revert the merge commit if it is required.

[weizhouapache]

@DaanHoogland @shwstppr, my mistake, I can revert the merge commit if it is required.

No need to revert, I think
@BryanMLima

[nvazquez]

Hi guys, I think in case 4.19.0 RC4 makes it to be the final release this fix won't be on 4.19.1 as the milestone states, but will be present only on the future 4.20. @BryanMLima @shwstppr in case you are reverting this fix please target it to the 4.19 branch

[shwstppr]

Good catch @nvazquez. If we want it in 4.19.1 I guess either we need to revert and merge again in 4.19 branch once 4.19 release is done or we would need to backport the commit. Any other suggestions @BryanMLima @DaanHoogland @weizhouapache ?
If there will be another RC then all good

[weizhouapache]

We can backport to 4.18, then merge forward to 4.19 and main
@shwstppr @nvazquez @DaanHoogland @JoaoJandre

[DaanHoogland]

@BryanMLima @lucas-a-martins , as @nvazquez said and hopefully it will be on 4.20 ;) but if you need it in a mainstream release before, please backport.
A revert will prevent merge forward conflicts if you do so, but is not strictly necessary. cc @shwstppr @JoaoJandre .
either way, no major harm done, just an inconvenience.

[weizhouapache]

@BryanMLima @lucas-a-martins , as @nvazquez said and hopefully it will be on 4.20 ;) but if you need it in a mainstream release before, please backport.
A revert will prevent merge forward conflicts if you do so, but is not strictly necessary. cc @shwstppr @JoaoJandre .
either way, no major harm done, just an inconvenience.

@DaanHoogland
I think this is a very small fix without any risk.
I have cherry-picked the commit to 4.18 branch and pushed to the github repository. It will be in 4.18.2.0/4.19.1.0/4.20.0.0
cc @BryanMLima @lucas-a-martins @shwstppr @nvazquez

[BryanMLima]

@DaanHoogland I think this is a very small fix without any risk. I have cherry-picked the commit to 4.18 branch and pushed to the github repository. It will be in 4.18.2.0/4.19.1.0/4.20.0.0 cc @BryanMLima @lucas-a-martins @shwstppr @nvazquez

Thanks, @weizhouapache! Sorry for any inconvenience guys, my mistake. I will pay more attention in the future.

[weizhouapache]

@DaanHoogland I think this is a very small fix without any risk. I have cherry-picked the commit to 4.18 branch and pushed to the github repository. It will be in 4.18.2.0/4.19.1.0/4.20.0.0 cc @BryanMLima @lucas-a-martins @shwstppr @nvazquez

Thanks, @weizhouapache! Sorry for any inconvenience guys, my mistake. I will pay more attention in the future.

no worries @BryanMLima