Fix storage filter restrictions (#545)

The storage selection filtering mechanism in preseed is meant as an
alternative to the explicitly defined disk set.

If all systems have supplied disks explicitly for ceph, then also using
the filter will now return a descriptive error, instead of simply
reporting that no more disks could be found. The same is true for zfs.

Disk filtering will apply for any system that did not explicitly define
disks. That is to say, as long as one system has not explicitly defined
disks, then including a filter will be valid. This is true for ceph/zfs
respectively.

cmd/microcloud/preseed.go

@@ -893,39 +893,50 @@ 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")
+	}
+
+	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.
-		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] {
+			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)
+			}
 		}
 	}
 
 	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))
@@ -951,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 {
@@ -991,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) {

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:

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
 }