openebs / lvm-localpv

Dynamically provision Stateful Persistent Node-Local Volumes & Filesystems for Kubernetes that is integrated with a backend LVM2 data storage stack.
Apache License 2.0
235 stars 92 forks source link

refactor(lvm): continue refactoring of lvm command calling convention #261

Closed kro-cat closed 9 months ago

kro-cat commented 9 months ago

The number of this branch refers to pull request #250.

Why is this PR required? What issue does it fix?: The calling convention of lvm commands in pkg/lvm/lvm_util.go should all follow the same format, following issue #247. This reduces the impact of unforseen (non-critical) warnings being printed to the STDERR output stream, which CombinedOutput mixes with STDOUT.

What this PR does?: Refactor all occurrences of Commands using CombinedOutput with an lvm command to use RunCommandSplit, a function which splits the STDERR and STDOUT streams into separate return values. Additionally, RunCommandSplit prints any STDERR messages to the log as a warning so that these messages aren't lost.

Additionally, this PR refactors some changes in ci/ci-test.sh that were added in pull #250. During ci tests of these changes, I found that the tests were failing for an unknown reason. A small patch has been applied to ci/ci-test.sh which may help resolve that issue on some machines.

Does this PR require any upgrade changes?: Most likely not.

If the changes in this PR are manually verified, list down the scenarios covered:: No manual verification. The refactors in this PR are a follow-up to issue #247.

Any additional information for your reviewer? : This PR is a continuation of pull request #250.

Checklist: