From ea4a4501899a15d996281c0de10e8532f408e990 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Sat, 22 Sep 2018 23:50:03 -0700 Subject: [PATCH] machines/libvirt/worker.machineset.yaml: Drop /var/lib/libvirt/images The actuator looks up the baseVolumeID in the configured pool [1]. That lookup works with both the volume name and full volume path: $ virsh -c qemu:///system vol-info --vol coreos_base --pool default Name: coreos_base Type: file Capacity: 16.00 GiB Allocation: 1.55 GiB $ virsh -c qemu:///system vol-info --vol /home/trking/VirtualMachines/coreos_base --pool default Name: coreos_base Type: file Capacity: 16.00 GiB Allocation: 1.55 GiB But it fails if you use the wrong full path: $ virsh -c qemu:///system vol-info --vol /var/lib/libvirt/images/coreos_base --pool default error: failed to get vol '/var/lib/libvirt/images/coreos_base' error: Storage volume not found: no storage vol with matching path '/var/lib/libvirt/images/coreos_base' My default pool happens to be in my home directory: $ virsh -c qemu:///system pool-dumpxml default default c20a2154-aa60-44cf-bf37-cd8b7818a4e4 105554829312 44134699008 61420130304 /home/trking/VirtualMachines 0777 114032 114032 This commit allows configutions like mine by dropping our opinions about the default pool location and just using the volume names: $ virsh -c qemu:///system vol-list --pool default Name Path ------------------------------------------------------------------------------ bootstrap /home/trking/VirtualMachines/bootstrap bootstrap.ign /home/trking/VirtualMachines/bootstrap.ign coreos_base /home/trking/VirtualMachines/coreos_base master-0.ign /home/trking/VirtualMachines/master-0.ign master0 /home/trking/VirtualMachines/master0 worker.ign /home/trking/VirtualMachines/worker.ign We've been using the full-path approach since the templates landed in 2522d0f2 (add libvirt support, 2018-08-30, #35), but there's no discussion there about why the path approach was chosen instead of the name approach I'm switching to here. Longer-term, it would be nice to pull both the pool and volume names from information pushed by the installer [2]. But I'm punting on *that* for this commit. Reported-by: Matt Rogers [1]: https://github.com/openshift/cluster-api-provider-libvirt/blob/2e5a516afc704c6c94d7b7cde74e78c43bbfeaa5/cloud/libvirt/actuators/machine/utils/volume.go#L174 [2]: https://github.com/openshift/installer/blob/dc4764dc603cea5da0e54f575b7ae1a2c26d3102/pkg/types/machinepools.go#L53-L58 --- machines/libvirt/worker.machineset.yaml | 6 +++--- pkg/render/render_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/machines/libvirt/worker.machineset.yaml b/machines/libvirt/worker.machineset.yaml index 5b5ef3e5b..21068329d 100644 --- a/machines/libvirt/worker.machineset.yaml +++ b/machines/libvirt/worker.machineset.yaml @@ -30,14 +30,14 @@ spec: kind: LibvirtMachineProviderConfig domainMemory: 2048 domainVcpu: 2 - ignKey: /var/lib/libvirt/images/worker.ign + ignKey: worker.ign volume: poolName: default - baseVolumeID: /var/lib/libvirt/images/coreos_base + baseVolumeID: coreos_base networkInterfaceName: {{.Libvirt.NetworkName}} networkInterfaceAddress: {{.Libvirt.IPRange}} autostart: false uri: {{.Libvirt.URI}} versions: kubelet: "" - controlPlane: "" \ No newline at end of file + controlPlane: "" diff --git a/pkg/render/render_test.go b/pkg/render/render_test.go index a7c4fab5e..b82092da6 100644 --- a/pkg/render/render_test.go +++ b/pkg/render/render_test.go @@ -199,10 +199,10 @@ spec: kind: LibvirtMachineProviderConfig domainMemory: 2048 domainVcpu: 2 - ignKey: /var/lib/libvirt/images/worker.ign + ignKey: worker.ign volume: poolName: default - baseVolumeID: /var/lib/libvirt/images/coreos_base + baseVolumeID: coreos_base networkInterfaceName: testNet networkInterfaceAddress: 192.168.124.0/24 autostart: false