search

oVirt / vdsm

The Virtual Desktop Server Manager
GNU General Public License v2.0
161 stars 202 forks source link

tests: Test builtin lvmlocal.conf #332

Closed nirs closed 2 years ago

nirs commented 2 years ago

Recent change adding spdx headers broke OST. Add simple test ensuring that the builtin file can be parsed.

Signed-off-by: Nir Soffer nsoffer@redhat.com

mz-pdm commented 2 years ago

Recent change adding spdx headers broke OST.

Really, or did you get confused by my PR #331? OST eventually passed on master for me although there had been previous repeated failure related LVM in https://github.com/oVirt/ovirt-system-tests/pull/270 .

Regardless, adding this test is good of course.

nirs commented 2 years ago

Recent change adding spdx headers broke OST.

Really, or did you get confused by my PR #331? OST eventually passed on master for me although there had been previous repeated failure related LVM in oVirt/ovirt-system-tests#270 .

Regardless, adding this test is good of course.

Yes, we added empty line before lvmconf header and this breaks the code parsing metadata from the header. This breaks "vdsm-tool is-configured" since the file will never have the expected metadata.

mz-pdm commented 2 years ago

Ah, yes, I missed the PR, OK.

nirs commented 2 years ago

@mz-pdm please ack and merge, I cannot merge now without another review.

mz-pdm commented 2 years ago

@mz-pdm please ack and merge, I cannot merge now without another review.

@michalskrivanek , requiring two maintainers to merge a PR is impractical. Would you change the setting in Branch protection rules, please?