From 87a8e5a5d73853d53bfe73aa2584dca0271e9942 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Fri, 13 Dec 2024 01:26:25 +0000 Subject: [PATCH 1/4] cmd/microcloud: Disallow filtering disks if all systems have specified disks Signed-off-by: Max Asnaashari --- cmd/microcloud/preseed.go | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/cmd/microcloud/preseed.go b/cmd/microcloud/preseed.go index a4bc3dff..3736ed85 100644 --- a/cmd/microcloud/preseed.go +++ b/cmd/microcloud/preseed.go @@ -893,31 +893,36 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map c.systems[peer] = system } - allResources := map[string]*lxdAPI.Resources{} - for peer, system := range c.systems { - // Skip any systems that had direct configuration. - if len(system.MicroCephDisks) > 0 || len(system.TargetStoragePools) > 0 || len(system.StoragePools) > 0 { - continue + checkFilterZFS := map[string]bool{} + checkFilterCeph := map[string]bool{} + for _, system := range p.Systems { + if (DirectStorage{} == system.Storage.Local) { + checkFilterZFS[system.Name] = true } - setupStorage := false - for _, cfg := range system.JoinConfig { - if cfg.Entity == "storage-pool" { - setupStorage = true - break - } + if len(system.Storage.Ceph) == 0 { + checkFilterCeph[system.Name] = true } + } - if setupStorage { - continue - } + if len(checkFilterCeph) == 0 && len(p.Storage.Ceph) > 0 { + return nil, fmt.Errorf("Ceph disk filter cannot be used. All systems have explicitly specified disks") + } + + if len(checkFilterZFS) == 0 && len(p.Storage.Local) > 0 { + return nil, fmt.Errorf("Local disk filter cannot be used. All systems have explicitly specified a disk") + } + allResources := map[string]*lxdAPI.Resources{} + for peer, system := range c.systems { cert := system.ServerInfo.Certificate // Fetch system resources from LXD to find disks if we haven't directly set up disks. - allResources[peer], err = s.Services[types.LXD].(*service.LXDService).GetResources(context.Background(), peer, system.ServerInfo.Address, cert) - if err != nil { - return nil, fmt.Errorf("Failed to get system resources of peer %q: %w", peer, err) + if checkFilterZFS[peer] || checkFilterCeph[peer] { + allResources[peer], err = s.Services[types.LXD].(*service.LXDService).GetResources(context.Background(), peer, system.ServerInfo.Address, cert) + if err != nil { + return nil, fmt.Errorf("Failed to get system resources of peer %q: %w", peer, err) + } } } From fe1b9312d03953ef1196b86c913ef9534794b25f Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Tue, 17 Dec 2024 00:31:01 +0000 Subject: [PATCH 2/4] cmd/microcloud: Split up resource collection for zfs/ceph Signed-off-by: Max Asnaashari --- cmd/microcloud/preseed.go | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/cmd/microcloud/preseed.go b/cmd/microcloud/preseed.go index 3736ed85..2cee2af0 100644 --- a/cmd/microcloud/preseed.go +++ b/cmd/microcloud/preseed.go @@ -913,13 +913,21 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map return nil, fmt.Errorf("Local disk filter cannot be used. All systems have explicitly specified a disk") } - allResources := map[string]*lxdAPI.Resources{} + allResourcesZFS := map[string]*lxdAPI.Resources{} + allResourcesCeph := map[string]*lxdAPI.Resources{} for peer, system := range c.systems { cert := system.ServerInfo.Certificate // Fetch system resources from LXD to find disks if we haven't directly set up disks. - if checkFilterZFS[peer] || checkFilterCeph[peer] { - allResources[peer], err = s.Services[types.LXD].(*service.LXDService).GetResources(context.Background(), peer, system.ServerInfo.Address, cert) + if checkFilterZFS[peer] { + allResourcesZFS[peer], err = s.Services[types.LXD].(*service.LXDService).GetResources(context.Background(), peer, system.ServerInfo.Address, cert) + if err != nil { + return nil, fmt.Errorf("Failed to get system resources of peer %q: %w", peer, err) + } + } + + if checkFilterCeph[peer] { + allResourcesCeph[peer], err = s.Services[types.LXD].(*service.LXDService).GetResources(context.Background(), peer, system.ServerInfo.Address, cert) if err != nil { return nil, fmt.Errorf("Failed to get system resources of peer %q: %w", peer, err) } @@ -927,10 +935,8 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map } cephMatches := map[string]int{} - zfsMatches := map[string]int{} cephMachines := map[string]bool{} - zfsMachines := map[string]bool{} - for peer, r := range allResources { + for peer, r := range allResourcesCeph { system := c.systems[peer] disks := make([]lxdAPI.ResourcesStorageDisk, 0, len(r.Storage.Disks)) @@ -956,6 +962,7 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map Encrypt: filter.Encrypt, }, ) + // There should only be one ceph pool per system. if !addedCephPool { if c.bootstrap { @@ -996,6 +1003,21 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map } } + c.systems[peer] = system + } + + zfsMatches := map[string]int{} + zfsMachines := map[string]bool{} + for peer, r := range allResourcesZFS { + system := c.systems[peer] + + disks := make([]lxdAPI.ResourcesStorageDisk, 0, len(r.Storage.Disks)) + for _, disk := range r.Storage.Disks { + if len(disk.Partitions) == 0 { + disks = append(disks, disk) + } + } + for _, filter := range p.Storage.Local { // No need to check filters anymore if each machine has a disk. if len(zfsMachines) == len(c.systems) { From b15ec2065b872a7194bb7f226f51d036dd2c8957 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Fri, 13 Dec 2024 01:27:19 +0000 Subject: [PATCH 3/4] test/suites: Add tests for filter restrictions Signed-off-by: Max Asnaashari --- test/suites/preseed.sh | 78 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/test/suites/preseed.sh b/test/suites/preseed.sh index 4e4cb077..4b727f30 100644 --- a/test/suites/preseed.sh +++ b/test/suites/preseed.sh @@ -282,4 +282,82 @@ EOF validate_system_microceph ${m} 1 1 disk1 disk1 fi done + + reset_systems 2 2 2 + echo "Fail to create a MicroCloud if all systems have defined disks and a filter for Ceph" + + preseed="$(cat << EOF +initiator: micro01 +lookup_subnet: 10.0.0.0/8 +session_passphrase: abcd +systems: + - name: micro01 + storage: + ceph: + - path: /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_disk1 + wipe: true + - name: micro02 + storage: + ceph: + - path: /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_disk1 + wipe: true +storage: + cephfs: true + ceph: + - find: device_id == *lxd_disk2 + find_min: 1 + wipe: true +EOF + )" + + lxc exec micro02 --env TEST_CONSOLE=0 -- sh -c 'microcloud preseed > out' <<< "$preseed" & + ! lxc exec micro01 --env TEST_CONSOLE=0 -- sh -c 'microcloud preseed 2>err > out' <<< "$preseed" || false + + lxc exec micro01 -- cat err | grep -q "Ceph disk filter cannot be used. All systems have explicitly specified disks" + + child_processes="$(jobs -pr)" + if [ -n "${child_processes}" ]; then + for p in ${child_processes}; do + kill -9 "${p}" + done + fi + + reset_systems 2 2 2 + echo "Fail to create a MicroCloud if all systems have defined disks and a filter for ZFS" + + preseed="$(cat << EOF +initiator: micro01 +lookup_subnet: 10.0.0.0/8 +session_passphrase: abcd +systems: + - name: micro01 + storage: + local: + path: /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_disk1 + wipe: true + - name: micro02 + storage: + local: + path: /dev/disk/by-id/scsi-0QEMU_QEMU_HARDDISK_lxd_disk1 + wipe: true +storage: + cephfs: true + local: + - find: device_id == *lxd_disk2 + find_min: 1 + wipe: true +EOF + )" + + lxc exec micro02 --env TEST_CONSOLE=0 -- sh -c 'microcloud preseed > out' <<< "$preseed" & + ! lxc exec micro01 --env TEST_CONSOLE=0 -- sh -c 'microcloud preseed 2>err > out' <<< "$preseed" || false + + lxc exec micro01 -- cat err | grep -q "Local disk filter cannot be used. All systems have explicitly specified a disk" + + child_processes="$(jobs -pr)" + if [ -n "${child_processes}" ]; then + for p in ${child_processes}; do + kill -9 "${p}" + done + fi } From c2f486442318cf6cab350810d11bb8341d1e7743 Mon Sep 17 00:00:00 2001 From: Max Asnaashari Date: Tue, 17 Dec 2024 00:32:50 +0000 Subject: [PATCH 4/4] doc/how-to: Add doc explanation of filter restriction Signed-off-by: Max Asnaashari --- doc/how-to/preseed.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/how-to/preseed.yaml b/doc/how-to/preseed.yaml index f85a5294..4d9e215a 100644 --- a/doc/how-to/preseed.yaml +++ b/doc/how-to/preseed.yaml @@ -77,6 +77,7 @@ ovn: dns_servers: 192.0.2.1,2001:db8:d:200::1 # `storage` is optional and is used as basic filtering logic for finding disks across all systems. +# Filters will only apply to systems which do not have an explicitly defined disk above for the corresponding storage type. # Filters are checked in order of appearance. # The names and values of each key correspond to the YAML field names for the `api.ResouresStorageDisk` # struct here: