fix(ddil): CI green-up — instance-type discovery, gotestsum, EC2 envelope

Three fixes to take the DDIL E2E suite from "scenarios flipped" to actually
green on the tofu cluster. mulga-siv-56.

1. Witness auto-discovers the smallest registered instance type
   (DescribeInstanceTypes, sort by memory then vCPUs). The hard-coded
   t2.micro default returned InvalidInstanceType on every Scenario A/B/C
   launch because the cluster doesn't register that type. DDIL_WITNESS_-
   INSTANCE_TYPE stays as an operator override.

2. e2e-ddil.yml uses gotestsum so the Actions UI shows per-test PASS/FAIL/
   SKIP lines instead of a JSON wall. results.json artifact preserved for
   future flake-rate dashboarding.

3. GenerateEC2ErrorResponse emits the real EC2 query-API envelope
   <Response><Errors><Error>... instead of the IAM-shaped <ErrorResponse>.
   aws-sdk-go v1's ec2query handler rejects the latter with
   SerializationError and drops the code, leaving Go callers without an
   awserr.Code() to inspect. New SDK round-trip test in gateway_test.go
   guards the regression.
This commit is contained in:
joshsiv-mulga
2026-05-11 12:54:04 +10:00
parent 0e09c9dda6
commit aa03cabf37
4 changed files with 146 additions and 22 deletions
+12 -3
View File
@@ -53,6 +53,13 @@ jobs:
sparse-checkout: scripts/tofu-cluster
path: mulga
- name: Install gotestsum
run: |
# Idempotent: re-running `go install` on a cached binary is a no-op.
# Pinned tag keeps Actions output format stable across runs.
go install gotest.tools/gotestsum@v1.12.0
echo "${HOME}/go/bin" >> "${GITHUB_PATH}"
- name: Build Install Artifacts
working-directory: mulga/scripts/tofu-cluster
run: |
@@ -115,6 +122,8 @@ jobs:
mkdir -p "${RUNNER_TEMP}/ddil-results"
cd "${GITHUB_WORKSPACE}/spinifex"
# gotestsum: human-readable PASS/FAIL/SKIP lines in Actions stdout
# while still emitting the machine-readable jsonfile artifact.
DDIL_NODES="${NODE_IPS}" \
DDIL_SSH_USER="${SSH_USER}" \
DDIL_SSH_KEY="${SSH_KEY}" \
@@ -122,9 +131,9 @@ jobs:
AWS_REGION="${REGION}" \
AWS_ACCESS_KEY_ID="${AWS_ACCESS_KEY_ID}" \
AWS_SECRET_ACCESS_KEY="${AWS_SECRET_ACCESS_KEY}" \
go test -tags=e2e -timeout 45m -json \
./tests/e2e/ddil/scenarios/... \
| tee "${RUNNER_TEMP}/ddil-results/results.json"
gotestsum --jsonfile "${RUNNER_TEMP}/ddil-results/results.json" \
--format testname \
-- -tags=e2e -timeout 45m ./tests/e2e/ddil/scenarios/...
- name: Upload DDIL Results
if: always()
+17 -10
View File
@@ -61,15 +61,22 @@ var supportedServices = map[string]bool{
"spinifex": true,
}
const xmlnsEC2 = "http://ec2.amazonaws.com/doc/2016-11-15/"
type ErrorResponse struct {
XMLName xml.Name `xml:"http://ec2.amazonaws.com/doc/2016-11-15/ ErrorResponse"`
Errors Errors `xml:"Errors"`
RequestID string `xml:"RequestID"`
// EC2ErrorResponse is the EC2 query-API error envelope.
//
// Real AWS EC2 emits `<Response><Errors><Error>...</Error></Errors><RequestID>...</Response>`
// for error replies. aws-sdk-go v1's ec2query response handler is strict — it
// rejects the IAM-style `<ErrorResponse>` envelope with SerializationError and
// drops the underlying error code, leaving callers without an awserr.Code() to
// inspect. The cluster-unavailable bailout in writeClusterUnavailable already
// uses this shape inline; this type is the structural equivalent for the
// generic error path.
type EC2ErrorResponse struct {
XMLName xml.Name `xml:"Response"`
Errors EC2Errors `xml:"Errors"`
RequestID string `xml:"RequestID"`
}
type Errors struct {
type EC2Errors struct {
Error ErrorDetail `xml:"Error"`
}
@@ -423,8 +430,8 @@ func ParseAWSQueryArgs(query string) (map[string]string, error) {
}
func GenerateEC2ErrorResponse(code, message, requestID string) (output []byte) {
errorXml := ErrorResponse{
Errors: Errors{
errorXml := EC2ErrorResponse{
Errors: EC2Errors{
Error: ErrorDetail{
Code: code,
Message: errors.New(message),
@@ -437,7 +444,7 @@ func GenerateEC2ErrorResponse(code, message, requestID string) (output []byte) {
if err != nil {
slog.Error("Failed to build XML", "error", err)
return []byte(xml.Header + `<ErrorResponse xmlns="` + xmlnsEC2 + `"><Errors><Error><Code>InternalError</Code><Message>Internal error</Message></Error></Errors><RequestID>` + requestID + `</RequestID></ErrorResponse>`)
return []byte(xml.Header + `<Response><Errors><Error><Code>InternalError</Code><Message>Internal error</Message></Error></Errors><RequestID>` + requestID + `</RequestID></Response>`)
}
// Add XML header
+59 -6
View File
@@ -12,6 +12,11 @@ import (
"strings"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
awscreds "github.com/aws/aws-sdk-go/aws/credentials"
awssession "github.com/aws/aws-sdk-go/aws/session"
awsec2 "github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/predastore/ratelimit"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam"
@@ -72,9 +77,11 @@ func TestGenerateEC2ErrorResponse_Structure(t *testing.T) {
// Verify request ID
assert.Contains(t, xmlStr, "<RequestID>"+tc.requestID+"</RequestID>")
// Verify root element
assert.Contains(t, xmlStr, `<ErrorResponse xmlns="`+xmlnsEC2+`">`)
assert.Contains(t, xmlStr, "</ErrorResponse>")
// Verify root element — EC2 query API uses <Response>, not
// <ErrorResponse>; aws-sdk-go v1 rejects the latter with
// SerializationError.
assert.Contains(t, xmlStr, "<Response>")
assert.Contains(t, xmlStr, "</Response>")
// Verify Errors wrapper
assert.Contains(t, xmlStr, "<Errors>")
@@ -203,7 +210,7 @@ func TestErrorHandler_UnknownError(t *testing.T) {
xmlStr := string(body)
// Unknown errors should be remapped to InternalError
assert.Contains(t, xmlStr, "<Code>InternalError</Code>")
assert.Contains(t, xmlStr, `<ErrorResponse xmlns="`+xmlnsEC2+`">`)
assert.Contains(t, xmlStr, "<Response>")
assert.Contains(t, xmlStr, "<Errors>")
}
@@ -222,7 +229,7 @@ func TestErrorHandler_EC2Service(t *testing.T) {
body, _ := io.ReadAll(resp.Body)
xmlStr := string(body)
assert.Contains(t, xmlStr, `<ErrorResponse xmlns="`+xmlnsEC2+`">`)
assert.Contains(t, xmlStr, "<Response>")
assert.Contains(t, xmlStr, "<Errors>")
assert.Contains(t, xmlStr, "<Code>InvalidParameterValue</Code>")
}
@@ -1156,7 +1163,7 @@ func TestWriteThrottleError_EC2(t *testing.T) {
assert.Equal(t, 503, resp.StatusCode)
assert.Equal(t, "application/xml", resp.Header.Get("Content-Type"))
assert.Contains(t, string(body), "<Code>RequestLimitExceeded</Code>")
assert.Contains(t, string(body), `<ErrorResponse xmlns="`+xmlnsEC2+`">`)
assert.Contains(t, string(body), "<Response>")
}
func TestWriteThrottleError_IAM(t *testing.T) {
@@ -1338,3 +1345,49 @@ func TestRequest_ClusterUnavailableClosedConn(t *testing.T) {
assert.Equal(t, http.StatusServiceUnavailable, resp.StatusCode)
}
// TestGenerateEC2ErrorResponse_SDKRoundTrip serves the EC2 error envelope from
// an httptest server, points an aws-sdk-go v1 EC2 client at it, and asserts
// the SDK surfaces the code via awserr.Error.Code() — not SerializationError.
//
// This is the regression guard for the bug behind mulga-siv-56 §3: aws-sdk-go
// v1's ec2query response handler rejects the IAM `<ErrorResponse>` envelope
// and discards the embedded code, leaving Go callers without a parseable
// AWS error.
func TestGenerateEC2ErrorResponse_SDKRoundTrip(t *testing.T) {
const wantCode = "InvalidInstanceType"
const wantMessage = "The instance type 't2.micro' is not supported in this region."
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
body := GenerateEC2ErrorResponse(wantCode, wantMessage, "req-sdk-roundtrip")
w.Header().Set("Content-Type", "application/xml")
w.WriteHeader(http.StatusBadRequest)
_, _ = w.Write(body)
}))
defer srv.Close()
sess, err := awssession.NewSession(&aws.Config{
Region: aws.String("us-east-1"),
Endpoint: aws.String(srv.URL),
Credentials: awscreds.NewStaticCredentials("AKIA-TEST", "secret", ""),
DisableSSL: aws.Bool(true),
// Suppress the default retry loop — error responses are not retryable
// here and waiting them out wastes test time.
MaxRetries: aws.Int(0),
})
require.NoError(t, err)
client := awsec2.New(sess)
_, err = client.RunInstances(&awsec2.RunInstancesInput{
ImageId: aws.String("ami-test"),
InstanceType: aws.String("t2.micro"),
MinCount: aws.Int64(1),
MaxCount: aws.Int64(1),
})
require.Error(t, err)
var awsErr awserr.Error
require.True(t, errors.As(err, &awsErr), "expected awserr.Error, got %T: %v", err, err)
assert.Equal(t, wantCode, awsErr.Code())
assert.NotEqual(t, "SerializationError", awsErr.Code(), "SDK could not parse the envelope")
}
+58 -3
View File
@@ -13,6 +13,7 @@ import (
"net/http"
"os"
"regexp"
"sort"
"strconv"
"strings"
"testing"
@@ -55,7 +56,11 @@ type Witness struct {
// - AWS_REGION (required) — passed to the AWS SDK client.
// - DDIL_WITNESS_AMI (optional) — specific AMI ID; if unset, an
// ami-ubuntu-* image is discovered at launch time.
// - DDIL_WITNESS_INSTANCE_TYPE (default t2.micro) — instance type.
// - DDIL_WITNESS_INSTANCE_TYPE (optional) — explicit instance type; if
// unset, the smallest registered type (by memory, then vCPUs) is
// auto-discovered at launch time. Auto-discovery beats hard-coding
// `t2.micro` because tofu clusters seed `*.nano` variants under
// names that change between releases.
// - DDIL_WITNESS_KEY_NAME (default spinifex-key) — EC2 key pair name.
// - DDIL_GUEST_SSH_USER (default ubuntu) — user for guest SSH.
// - DDIL_GUEST_SSH_KEY (default cluster.SSHKeyPath) — private key for
@@ -107,7 +112,7 @@ func NewWitness(cluster *Cluster, transport SSH) (*Witness, error) {
guestSigner: guestSigner,
guestUser: envDefault("DDIL_GUEST_SSH_USER", "ubuntu"),
ami: os.Getenv("DDIL_WITNESS_AMI"),
instanceType: envDefault("DDIL_WITNESS_INSTANCE_TYPE", "t2.micro"),
instanceType: os.Getenv("DDIL_WITNESS_INSTANCE_TYPE"),
keyName: envDefault("DDIL_WITNESS_KEY_NAME", "spinifex-key"),
}, nil
}
@@ -141,6 +146,10 @@ func LaunchWitnessVM(ctx context.Context, w *Witness, host Node) (*WitnessVM, er
if err != nil {
return nil, err
}
instanceType, err := w.resolveInstanceType(ctx)
if err != nil {
return nil, err
}
userData := base64.StdEncoding.EncodeToString(cloudInitWitness)
@@ -148,7 +157,7 @@ func LaunchWitnessVM(ctx context.Context, w *Witness, host Node) (*WitnessVM, er
for attempt := 1; attempt <= maxPlacementAttempts; attempt++ {
out, err := w.ec2.RunInstancesWithContext(ctx, &ec2.RunInstancesInput{
ImageId: aws.String(ami),
InstanceType: aws.String(w.instanceType),
InstanceType: aws.String(instanceType),
MinCount: aws.Int64(1),
MaxCount: aws.Int64(1),
KeyName: aws.String(w.keyName),
@@ -251,6 +260,52 @@ func (w *Witness) resolveAMI(ctx context.Context) (string, error) {
return w.ami, nil
}
// resolveInstanceType returns the witness instance type, caching the result.
// When DDIL_WITNESS_INSTANCE_TYPE is set it is honoured verbatim; otherwise
// DescribeInstanceTypes is consulted and the smallest registered type — by
// memory first, vCPUs as tiebreaker — is selected. The sort key is
// quantitative on purpose: naming conventions (`t2.nano`, `c6i.large`, …)
// shift between cluster releases, but smallest-by-resource always picks the
// cheapest valid launch target.
func (w *Witness) resolveInstanceType(ctx context.Context) (string, error) {
if w.instanceType != "" {
return w.instanceType, nil
}
out, err := w.ec2.DescribeInstanceTypesWithContext(ctx, &ec2.DescribeInstanceTypesInput{})
if err != nil {
return "", fmt.Errorf("ddil harness: DescribeInstanceTypes: %w", err)
}
if len(out.InstanceTypes) == 0 {
return "", errors.New("ddil harness: cluster registered no instance types")
}
sort.Slice(out.InstanceTypes, func(i, j int) bool {
a, b := out.InstanceTypes[i], out.InstanceTypes[j]
var aMem, bMem int64
if a.MemoryInfo != nil {
aMem = aws.Int64Value(a.MemoryInfo.SizeInMiB)
}
if b.MemoryInfo != nil {
bMem = aws.Int64Value(b.MemoryInfo.SizeInMiB)
}
if aMem != bMem {
return aMem < bMem
}
var aCPU, bCPU int64
if a.VCpuInfo != nil {
aCPU = aws.Int64Value(a.VCpuInfo.DefaultVCpus)
}
if b.VCpuInfo != nil {
bCPU = aws.Int64Value(b.VCpuInfo.DefaultVCpus)
}
return aCPU < bCPU
})
w.instanceType = aws.StringValue(out.InstanceTypes[0].InstanceType)
if w.instanceType == "" {
return "", errors.New("ddil harness: smallest instance type has empty name")
}
return w.instanceType, nil
}
func (w *Witness) waitForRunning(ctx context.Context, id string, timeout time.Duration) error {
deadline := time.Now().Add(timeout)
const interval = 2 * time.Second