mirror of
https://github.com/mulgadc/spinifex.git
synced 2026-10-15 17:11:30 +00:00
test: gate image shell scripts in CI and pin the spot vCPU quota ordering (#641)
* test: gate scripts/images/ shell suites + shellcheck in CI The eks-node/rds-postgres *_test.sh suites and shellcheck only ever ran by hand: spinifex.yml filters pushes on Go-only paths and no job invoked them, so a broken helper could land unnoticed. Add a `make test-images` target and a path-filtered image-scripts.yml workflow so both run automatically on scripts/images/** and images/** changes, kept separate from preflight so image-asset churn doesn't run on every Go contributor's commit. Also clears the shellcheck findings that would have started this gate red (SC1090 on a runtime-resolved source path, SC2154 x2 on eval-assigned variables shellcheck can't trace). * test: pin vCPU quota gate ordering in RequestSpotInstances Nothing asserted that RequestSpotInstances runs the vCPU quota gate before dispatching capacity or launching a VM, so a regression that dropped or reordered EnforceLaunch would pass silently. Add an ordering test using a real quota.Service (embedded JetStream KV) and real NATS responders standing in for the capacity and launch daemons, recording a global call sequence and asserting the gate's index precedes both. Verified by removing the gate: the first quota-KV read then comes from the post-launch charge instead, landing after capacity dispatch and launch, and the ordering assertion fails as expected.
This commit is contained in:
Josh Siviour
@@ -0,0 +1,39 @@
|
||||
name: Image Scripts
|
||||
|
||||
# Separate from spinifex.yml on purpose: scripts/images/ and images/ are shell
|
||||
# and mkosi assets baked into system images, not Go, so they get their own
|
||||
# path-filtered gate instead of running on every Go contributor's push (see
|
||||
# ui.yml for the same pattern applied to the frontend).
|
||||
on:
|
||||
push:
|
||||
branches:
|
||||
- main
|
||||
- dev
|
||||
paths:
|
||||
- scripts/images/**
|
||||
- images/**
|
||||
- .github/workflows/image-scripts.yml
|
||||
pull_request:
|
||||
branches:
|
||||
- main
|
||||
- dev
|
||||
paths:
|
||||
- scripts/images/**
|
||||
- images/**
|
||||
- .github/workflows/image-scripts.yml
|
||||
workflow_dispatch:
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
image_scripts:
|
||||
name: Image Scripts
|
||||
runs-on: ubuntu-26.04
|
||||
|
||||
steps:
|
||||
- name: Checkout Code
|
||||
uses: actions/checkout@v7
|
||||
|
||||
- name: Run image script tests + shellcheck
|
||||
run: make test-images
|
||||
@@ -190,6 +190,20 @@ test-actions:
|
||||
@echo -e "\n....Running action tests...."
|
||||
LOG_IGNORE=1 go test -timeout 60s ./.github/actions/...
|
||||
|
||||
# Shell suites + shellcheck for scripts/images/ helpers baked into system
|
||||
# images. Kept out of `preflight` (a dedicated CI job gates it on
|
||||
# scripts/images/** changes instead) so image-asset churn doesn't run on
|
||||
# every Go contributor's commit.
|
||||
test-images:
|
||||
@echo -e "\n....Running scripts/images/**/*_test.sh...."
|
||||
@for t in $$(find scripts/images -name '*_test.sh' | sort); do \
|
||||
echo "-- $$t"; \
|
||||
bash "$$t" || exit 1; \
|
||||
done
|
||||
@echo -e "\n....Running shellcheck over scripts/images/**/*.sh...."
|
||||
shellcheck -S warning $$(find scripts/images -name '*.sh' | sort)
|
||||
@echo " test-images ok"
|
||||
|
||||
# Check that new/changed code meets coverage threshold (runs tests first)
|
||||
diff-coverage: test-cover
|
||||
@QUIET=$(QUIET) scripts/diff-coverage.sh $(COVERPROFILE)
|
||||
@@ -319,7 +333,7 @@ distro-arm64:
|
||||
distro-clean:
|
||||
rm -rf dist/
|
||||
|
||||
.PHONY: build build-ui build-installer build-lb-agent build-ecs-agent build-system-image build-eks-node-image import-eks-node-image publish-eks-node-image build-ecs-node-image import-ecs-node-image build-rds-postgres-image import-rds-postgres-image build-microvm-image install-microvm go_build preflight test test-cover test-race diff-coverage bench test-actions test-harness test-integration manifest-check manifest-lint manifest-lint-update \
|
||||
.PHONY: build build-ui build-installer build-lb-agent build-ecs-agent build-system-image build-eks-node-image import-eks-node-image publish-eks-node-image build-ecs-node-image import-ecs-node-image build-rds-postgres-image import-rds-postgres-image build-microvm-image install-microvm go_build preflight test test-cover test-race diff-coverage bench test-actions test-images test-harness test-integration manifest-check manifest-lint manifest-lint-update \
|
||||
deploy reinstall clean \
|
||||
install-system install-go install-aws quickinstall \
|
||||
lint fix govulncheck nilaway \
|
||||
|
||||
@@ -100,6 +100,8 @@ fi
|
||||
# (K3S_URL/K3S_TOKEN live there).
|
||||
ROLE=""
|
||||
if [ -f "${ENVFILE}" ]; then
|
||||
# ENVFILE is a runtime-resolved, test-overridable path.
|
||||
# shellcheck disable=SC1090
|
||||
ROLE=$(. "${ENVFILE}"; printf '%s' "${SPINIFEX_K3S_ROLE:-}")
|
||||
fi
|
||||
if [ -z "${ROLE}" ] && [ -f "${AGENT_ENVFILE}" ]; then
|
||||
|
||||
@@ -82,6 +82,8 @@ for n in 0 1 2 3 4 5; do
|
||||
eval "cidr=\${NIC${n}_CIDR:-}"
|
||||
ip link set "$iface" up
|
||||
|
||||
# dhcp is assigned above via eval; shellcheck can't trace dynamic names.
|
||||
# shellcheck disable=SC2154
|
||||
if [ "$dhcp" = "1" ]; then
|
||||
if dhcp_acquire "$iface"; then
|
||||
echo "[mulga-mgmt-net] data NIC $iface ($mac) up via DHCP"
|
||||
@@ -119,6 +121,8 @@ for n in 0 1 2 3 4 5; do
|
||||
# egress and (with a link-local /16) hijack IMDS. Enforce it — a DHCP client
|
||||
# racing this NIC may have added one before we set it static.
|
||||
eval "isdefault=\${NIC${n}_DEFAULT:-}"
|
||||
# isdefault is assigned above via eval; shellcheck can't trace dynamic names.
|
||||
# shellcheck disable=SC2154
|
||||
if [ "$isdefault" != "1" ]; then
|
||||
ip route del default dev "$iface" 2>/dev/null || true
|
||||
fi
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
package gateway_ec2_spotinstance
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/aws/aws-sdk-go/aws"
|
||||
"github.com/aws/aws-sdk-go/service/ec2"
|
||||
handlers_quota "github.com/mulgadc/spinifex/spinifex/handlers/quota"
|
||||
"github.com/mulgadc/spinifex/spinifex/testutil"
|
||||
"github.com/mulgadc/spinifex/spinifex/types"
|
||||
"github.com/nats-io/nats.go"
|
||||
"github.com/nats-io/nats.go/jetstream"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// callOrder records the arrival order of named events from concurrent NATS
|
||||
// callbacks and the quota KV, giving a single global sequence to assert on.
|
||||
type callOrder struct {
|
||||
mu sync.Mutex
|
||||
events []string
|
||||
}
|
||||
|
||||
func (o *callOrder) record(name string) {
|
||||
o.mu.Lock()
|
||||
defer o.mu.Unlock()
|
||||
o.events = append(o.events, name)
|
||||
}
|
||||
|
||||
// indexOf returns the first position of name in the recorded sequence, or -1
|
||||
// if it never happened.
|
||||
func (o *callOrder) indexOf(name string) int {
|
||||
o.mu.Lock()
|
||||
defer o.mu.Unlock()
|
||||
for i, e := range o.events {
|
||||
if e == name {
|
||||
return i
|
||||
}
|
||||
}
|
||||
return -1
|
||||
}
|
||||
|
||||
// recordingKV wraps a real jetstream.KeyValue and records every Get as a
|
||||
// "quota-check" event. EnforceLaunch's only externally observable action is
|
||||
// CheckVCPU's read of the counter, so instrumenting Get alone pins the moment
|
||||
// the quota gate actually ran, using the real gate rather than a stand-in.
|
||||
type recordingKV struct {
|
||||
jetstream.KeyValue
|
||||
|
||||
order *callOrder
|
||||
}
|
||||
|
||||
func (r *recordingKV) Get(ctx context.Context, key string) (jetstream.KeyValueEntry, error) {
|
||||
r.order.record("quota-check")
|
||||
return r.KeyValue.Get(ctx, key)
|
||||
}
|
||||
|
||||
// TestRequestSpotInstances_QuotaGateRunsBeforeDispatchAndLaunch pins the
|
||||
// ordering the vCPU quota gate depends on: EnforceLaunch must run before the
|
||||
// capacity query (spinifex.node.status) and before the per-node VM launch
|
||||
// (ec2.RunInstances.<type>.<node>), not merely "at some point" during the
|
||||
// request. All three legs run for real: a real quota.Service backed by an
|
||||
// embedded JetStream KV, and real NATS responders standing in for a capacity
|
||||
// daemon and a launch daemon.
|
||||
func TestRequestSpotInstances_QuotaGateRunsBeforeDispatchAndLaunch(t *testing.T) {
|
||||
_, nc, js := testutil.StartTestJetStream(t)
|
||||
|
||||
kv, err := js.CreateKeyValue(t.Context(), jetstream.KeyValueConfig{
|
||||
Bucket: handlers_quota.KVBucketAccountUsage,
|
||||
History: 1,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
order := &callOrder{}
|
||||
quota := handlers_quota.New(handlers_quota.Limits{Enabled: true, VCPUs: 1000}, &recordingKV{KeyValue: kv, order: order})
|
||||
|
||||
const instanceType = "t3.micro"
|
||||
const node = "node-1"
|
||||
|
||||
// Stand-in capacity daemon: replies with one node that has room.
|
||||
statusSub, err := nc.Subscribe("spinifex.node.status", func(msg *nats.Msg) {
|
||||
order.record("capacity-dispatch")
|
||||
resp := types.NodeStatusResponse{
|
||||
Node: node,
|
||||
InstanceTypes: []types.InstanceTypeCap{{Name: instanceType, Available: 5}},
|
||||
}
|
||||
data, _ := json.Marshal(resp)
|
||||
_ = nc.Publish(msg.Reply, data)
|
||||
})
|
||||
require.NoError(t, err)
|
||||
defer statusSub.Unsubscribe()
|
||||
|
||||
// Stand-in launch daemon on the node the capacity daemon advertised.
|
||||
launchSub, err := nc.Subscribe(fmt.Sprintf("ec2.RunInstances.%s.%s", instanceType, node), func(msg *nats.Msg) {
|
||||
order.record("vm-launch")
|
||||
reservation := ec2.Reservation{
|
||||
ReservationId: aws.String("r-test"),
|
||||
Instances: []*ec2.Instance{{InstanceId: aws.String("i-test"), InstanceType: aws.String(instanceType)}},
|
||||
}
|
||||
data, _ := json.Marshal(reservation)
|
||||
_ = msg.Respond(data)
|
||||
})
|
||||
require.NoError(t, err)
|
||||
defer launchSub.Unsubscribe()
|
||||
|
||||
// Stand-in SIR-persist daemon so the request completes cleanly.
|
||||
putSub, err := nc.Subscribe("ec2.PutSpotInstanceRequests", func(msg *nats.Msg) {
|
||||
_ = msg.Respond([]byte("{}"))
|
||||
})
|
||||
require.NoError(t, err)
|
||||
defer putSub.Unsubscribe()
|
||||
|
||||
time.Sleep(50 * time.Millisecond) // let subscriptions propagate
|
||||
|
||||
input := &ec2.RequestSpotInstancesInput{
|
||||
LaunchSpecification: &ec2.RequestSpotLaunchSpecification{
|
||||
ImageId: aws.String("ami-0abcdef1234567890"),
|
||||
InstanceType: aws.String(instanceType),
|
||||
},
|
||||
InstanceCount: aws.Int64(1),
|
||||
}
|
||||
|
||||
_, err = RequestSpotInstances(context.Background(), input, nc, nil, "test-account", "ap-southeast-2a", nil, quota, 1)
|
||||
require.NoError(t, err)
|
||||
|
||||
quotaIdx := order.indexOf("quota-check")
|
||||
capacityIdx := order.indexOf("capacity-dispatch")
|
||||
launchIdx := order.indexOf("vm-launch")
|
||||
|
||||
require.NotEqualf(t, -1, quotaIdx, "EnforceLaunch never touched the quota gate (events: %v)", order.events)
|
||||
require.NotEqualf(t, -1, capacityIdx, "capacity dispatch never ran (events: %v)", order.events)
|
||||
require.NotEqualf(t, -1, launchIdx, "VM launch never ran (events: %v)", order.events)
|
||||
|
||||
assert.Less(t, quotaIdx, capacityIdx, "quota gate must run before capacity dispatch")
|
||||
assert.Less(t, quotaIdx, launchIdx, "quota gate must run before VM launch")
|
||||
}
|
||||
Reference in New Issue
Block a user