Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

incusd/scriptlet: Make set_target fail with invalid members #1339

Merged
merged 2 commits into from
Oct 28, 2024
Merged

incusd/scriptlet: Make set_target fail with invalid members #1339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8043469
incusd/scriptlet: Make set_target fail with invalid members
bensmrs Oct 28, 2024
ed4435f
tests: Update for scriptlet placement error handling
stgraber Oct 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/server/scriptlet/instance_placement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
9 changes: 6 additions & 3 deletions test/suites/clustering_instance_placement_scriptlet.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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=-
Expand Down
6 changes: 2 additions & 4 deletions test/suites/clustering_move.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=-
Expand All @@ -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=-
Expand Down
Loading