Merge pull request #1339 from bensmrs/main

incusd/scriptlet: Make set_target fail with invalid members
lxc · Oct 28, 2024 · f9e2022 · f9e2022
2 parents 99c8eb4 + ed4435f
commit f9e2022
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 9 deletions.
diff --git a/internal/server/scriptlet/instance_placement.go b/internal/server/scriptlet/instance_placement.go
@@ -47,8 +47,8 @@ func InstancePlacementRun(ctx context.Context, l logger.Logger, s *state.State,
 		}
 
 		if targetMember == nil {
-			l.Warn("Instance placement scriptlet set invalid member target", logger.Ctx{"member": memberName})
-			return starlark.String("Invalid member name"), nil
+			l.Error("Instance placement scriptlet set invalid member target", logger.Ctx{"member": memberName})
+			return starlark.String("Invalid member name"), fmt.Errorf("Invalid member name: %s", memberName)
 		}
 
 		l.Info("Instance placement scriptlet set member target", logger.Ctx{"member": targetMember.Name})

diff --git a/test/suites/clustering_instance_placement_scriptlet.sh b/test/suites/clustering_instance_placement_scriptlet.sh
@@ -120,7 +120,7 @@ EOF
   ! INCUS_DIR="${INCUS_ONE_DIR}" incus init testimage c1 || false
 
   # Set instance placement scriptlet to one that sets an invalid cluster member target.
-  # Check that instance placement uses Incus's built in logic instead (as if setTarget hadn't been called at all).
+  # Confirm that we get a placement error.
   cat << EOF | incus config set instances.placement.scriptlet=-
 def instance_placement(request, candidate_members):
         # Set invalid member target.
@@ -130,8 +130,11 @@ def instance_placement(request, candidate_members):
         return
 EOF
 
-  INCUS_DIR="${INCUS_ONE_DIR}" incus init testimage c1 -c cluster.evacuate=migrate
-  INCUS_DIR="${INCUS_ONE_DIR}" incus info c1 | grep -q "Location: node1"
+  ! INCUS_DIR="${INCUS_ONE_DIR}" incus init testimage c1 -c cluster.evacuate=migrate || false
+
+  # Create an instance
+  incus config unset instances.placement.scriptlet
+  INCUS_DIR="${INCUS_ONE_DIR}" incus init testimage c1 -c cluster.evacuate=migrate --target node1
 
   # Set basic instance placement scriptlet that statically targets to 3rd member.
   cat << EOF | incus config set instances.placement.scriptlet=-

diff --git a/test/suites/clustering_move.sh b/test/suites/clustering_move.sh
@@ -121,8 +121,7 @@ EOF
   INCUS_DIR="${INCUS_ONE_DIR}" incus move c2 --target @foobar3
   INCUS_DIR="${INCUS_ONE_DIR}" incus info c2 | grep -q "Location: node3"
 
-  # Ensure that setting an invalid target won't interrupt the move and fall back to the built in behavior.
-  # Equally distribute the instances beforehand so that node1 will get selected.
+  # Ensure that setting an invalid target causes the error to be raised.
   INCUS_DIR="${INCUS_ONE_DIR}" incus move c2 --target node2
 
   cat << EOF | INCUS_DIR="${INCUS_ONE_DIR}" incus config set instances.placement.scriptlet=-
@@ -134,8 +133,7 @@ def instance_placement(request, candidate_members):
         return
 EOF
 
-  INCUS_DIR="${INCUS_ONE_DIR}" incus move c1 --target @foobar1
-  INCUS_DIR="${INCUS_ONE_DIR}" incus info c1 | grep -q "Location: node1"
+  ! INCUS_DIR="${INCUS_ONE_DIR}" incus move c1 --target @foobar1 || false
 
   # If the scriptlet produces a runtime error, the move fails.
   cat << EOF | INCUS_DIR="${INCUS_ONE_DIR}" incus config set instances.placement.scriptlet=-