project-machine / disko

Disk Operations API in Go
Apache License 2.0
13 stars 9 forks source link

Make ExtendLV call 'cryptsetup resize' if luks device is open. #85

Closed smoser closed 4 years ago

smoser commented 4 years ago

Previously ExtendLV(vg, lv) would not extend the luks device. Now call 'cryptsetup resize' to update the luks device after extending the LV.

Also update the demo program to extend an LV. I'm testing this on a loop device with:

i=/tmp/my.img;
rm -f $i;
truncate --size=2G $i;
lodev=$(losetup --find --show $i);
udevadm settle;
echo $lodev;
./demo misc updown $lodev;
udevadm settle;
cryptsetup close myvg0_mylv0_crypt;
udevadm settle;
vgremove myvg0 --force;
for p in ${lodev}p*; do [ -b "$p" ] || continue;
   (set -x; delpart $lodev ${p##*p} ); done;
losetup --detach=$lodev
codecov[bot] commented 4 years ago

Codecov Report

Merging #85 into master will decrease coverage by 0.25%. The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #85      +/-   ##
==========================================
- Coverage   54.51%   54.25%   -0.26%     
==========================================
  Files          16       16              
  Lines        1928     1937       +9     
==========================================
  Hits         1051     1051              
- Misses        776      785       +9     
  Partials      101      101              
Impacted Files Coverage Δ
linux/lvm.go 8.26% <0.00%> (-0.31%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d8696e2...e0d3690. Read the comment docs.

smoser commented 4 years ago

lgtm. Is this already covered by an existing test? If not, the shell snippet that you put in the PR message looks like it could be part of a bats test suite, is that worth looking into?

At this point, disko has only unit tests. I agree that we need some automated system test to actually test.

I'm not a fan of bats. But need for system test overrides my personal distaste for test frameworks that generate shell code.

mikemccracken commented 4 years ago

Heh, yeah I mainly just mentioned bats because I'm familiar with it and tycho uses it in Stacker. I do like being able to write tests as shell scripts, but if there's something you like better, I won't argue for bats

smoser commented 4 years ago

@mikemccracken, @tych0 Please see https://github.com/anuvu/disko/pull/86. That adds some integration tests that will be the basis of the tests you asked for.

So a.) lets land that (c-i is happy with it, and it only adds tests. b.) We can land this one on its own and i can add tests in another PR.