feat: ModifyDBInstance and the shared VM-replace helper (#597)

This commit is contained in:
Julian Sommer
2026-07-29 15:07:06 +10:00
committed by GitHub
parent 7d2106bfb5
commit 23de2ad85d
36 changed files with 3343 additions and 72 deletions
+1 -1
View File
@@ -170,7 +170,7 @@ func newAgent(cfg config, cp controlPlane, probe *engineProbe) *Agent {
a := &Agent{cfg: cfg, id: id, cp: cp, probe: probe, handoffWriter: writeHandoff}
a.engine = newPostgresEngine(cfg, execCommandRunner)
a.hb = newHeartbeater(cp, probe, handlers_rds.HeartbeatInterval)
a.cmd = newCommander(cp, newCommandRegistry(a.engine), cfg.PollWait)
a.cmd = newCommander(cp, newCommandRegistry(a.engine, newGuestStorage(cfg, execCommandRunner)), cfg.PollWait)
return a
}
+4 -1
View File
@@ -20,7 +20,7 @@ type commandRegistry map[string]commandHandler
// An unregistered type is replied to as unsupported, so a control plane ahead
// of the guest gets a clear answer rather than a timeout.
func newCommandRegistry(engine engineOps) commandRegistry {
func newCommandRegistry(engine engineOps, storage storageOps) commandRegistry {
return commandRegistry{
handlers_rds.CommandSetPassword: func(ctx context.Context, cmd handlers_rds.Command) (string, error) {
params := commandParams(cmd)
@@ -40,6 +40,9 @@ func newCommandRegistry(engine engineOps) commandRegistry {
handlers_rds.CommandStopEngine: func(ctx context.Context, cmd handlers_rds.Command) (string, error) {
return "", engine.Stop(ctx)
},
handlers_rds.CommandGrowFilesystem: func(ctx context.Context, cmd handlers_rds.Command) (string, error) {
return storage.GrowFilesystem(ctx)
},
}
}
+2 -2
View File
@@ -57,7 +57,7 @@ func TestCommander_FailedHandlerRepliesWithItsError(t *testing.T) {
// for. The issuer is blocked on the command ID, so it gets an answer rather
// than a timeout.
func TestCommander_UnknownTypeRepliesFailedNotDropped(t *testing.T) {
reply := newCommander(nil, newCommandRegistry(&fakeEngine{}), time.Second).execute(context.Background(),
reply := newCommander(nil, newCommandRegistry(&fakeEngine{}, &fakeStorage{}), time.Second).execute(context.Background(),
handlers_rds.Command{CommandID: "cmd-3", Type: "quiesce"})
if reply.CommandID != "cmd-3" || reply.Status != handlers_rds.CommandStatusFailed {
@@ -107,7 +107,7 @@ func TestCommander_BacksOffAfterAFailedPoll(t *testing.T) {
cp := newFakeControlPlane()
cp.pollErr = errors.New("gateway unreachable")
c := newCommander(cp, newCommandRegistry(&fakeEngine{}), 10*time.Millisecond)
c := newCommander(cp, newCommandRegistry(&fakeEngine{}, &fakeStorage{}), 10*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
defer cancel()
done := make(chan struct{})
+15
View File
@@ -57,6 +57,13 @@ type config struct {
PGUser string
RCService string
EngineService string
// Where the data volume is mounted, and the two kernel surfaces a storage
// grow resolves its device from. Overridable so a test can point them at
// fixtures rather than at the running host's.
DataMount string
MountsFile string
SysBlock string
}
// Reads the cloud-init env file, then lets real env vars override.
@@ -78,6 +85,7 @@ func loadConfig(envFile string) config {
PGUser: get("RDS_PG_USER"),
RCService: get("RDS_RC_SERVICE"),
EngineService: get("RDS_ENGINE_SERVICE"),
DataMount: get("RDS_DATA_MOUNT"),
EnginePort: defaultEnginePort,
PollWait: defaultPollWait,
}
@@ -111,6 +119,13 @@ func loadConfig(envFile string) config {
if cfg.EngineService == "" {
cfg.EngineService = defaultEngineSvcName
}
// rds-datadir reads the same RDS_DATA_MOUNT, so the two agree on where the
// volume landed without either asserting it to the other.
if cfg.DataMount == "" {
cfg.DataMount = defaultDataMount
}
cfg.MountsFile = defaultMountsFile
cfg.SysBlock = defaultSysBlock
// The authoritative port comes from the bootstrap config; this is only what
// the probe uses until that first fetch lands.
if v := get("RDS_ENGINE_PORT"); v != "" {
+4 -4
View File
@@ -43,7 +43,7 @@ func (f *fakeEngine) Stop(context.Context) error {
func TestCommandRegistry_SetPasswordCarriesBothParameters(t *testing.T) {
engine := &fakeEngine{}
reply := newCommander(nil, newCommandRegistry(engine), 0).execute(context.Background(), handlers_rds.Command{
reply := newCommander(nil, newCommandRegistry(engine, &fakeStorage{}), 0).execute(context.Background(), handlers_rds.Command{
CommandID: "cmd-1",
Type: handlers_rds.CommandSetPassword,
Parameters: []handlers_rds.Parameter{
@@ -69,7 +69,7 @@ func TestCommandRegistry_SetPasswordCarriesBothParameters(t *testing.T) {
// restart come back as the message the issuer parses.
func TestCommandRegistry_ApplyParamsReportsPendingRestart(t *testing.T) {
engine := &fakeEngine{pending: []string{"shared_buffers", "max_connections"}}
reply := newCommander(nil, newCommandRegistry(engine), 0).execute(context.Background(), handlers_rds.Command{
reply := newCommander(nil, newCommandRegistry(engine, &fakeStorage{}), 0).execute(context.Background(), handlers_rds.Command{
CommandID: "cmd-2",
Type: handlers_rds.CommandApplyParams,
Parameters: []handlers_rds.Parameter{{Name: "shared_buffers", Value: "256MB"}},
@@ -88,7 +88,7 @@ func TestCommandRegistry_ApplyParamsReportsPendingRestart(t *testing.T) {
func TestCommandRegistry_StopEngineReachesTheEngine(t *testing.T) {
engine := &fakeEngine{}
reply := newCommander(nil, newCommandRegistry(engine), 0).execute(context.Background(),
reply := newCommander(nil, newCommandRegistry(engine, &fakeStorage{}), 0).execute(context.Background(),
handlers_rds.Command{CommandID: "cmd-3", Type: handlers_rds.CommandStopEngine})
if reply.Status != handlers_rds.CommandStatusSucceeded {
@@ -101,7 +101,7 @@ func TestCommandRegistry_StopEngineReachesTheEngine(t *testing.T) {
func TestCommandRegistry_FailureRepliesFailed(t *testing.T) {
engine := &fakeEngine{err: errors.New("the engine did not stop")}
reply := newCommander(nil, newCommandRegistry(engine), 0).execute(context.Background(),
reply := newCommander(nil, newCommandRegistry(engine, &fakeStorage{}), 0).execute(context.Background(),
handlers_rds.Command{CommandID: "cmd-4", Type: handlers_rds.CommandStopEngine})
if reply.Status != handlers_rds.CommandStatusFailed {
+181
View File
@@ -0,0 +1,181 @@
package main
import (
"bufio"
"context"
"fmt"
"log/slog"
"os"
"path/filepath"
"strings"
)
// The in-guest half of a storage grow (D12). The control plane has already
// grown the block device by the time this runs; all that is left is to put the
// filesystem on the capacity that is now there. Both ext4 and XFS grow while
// mounted, so this needs no ordering against the engine start.
type storageOps interface {
// Extends the filesystem at the data mount onto its whole device, and
// reports what it did for the command reply.
GrowFilesystem(ctx context.Context) (string, error)
}
// Tool names rather than paths: the image installs them from
// cloud-utils-growpart, e2fsprogs-extra and xfsprogs, all on PATH.
const (
defaultGrowpart = "growpart"
defaultResize2fs = "resize2fs"
defaultXFSGrowfs = "xfs_growfs"
defaultMountsFile = "/proc/mounts"
defaultSysBlock = "/sys/class/block"
// Where rds-datadir mounts the data volume; the datadir sits one level in.
defaultDataMount = "/var/lib/postgresql"
// growpart says this when the partition already fills the disk, which a
// resumed grow is the normal way to reach.
growpartNoChange = "NOCHANGE"
)
type guestStorage struct {
run commandRunner
dataMount string
mountsFile string
sysBlock string
growpart string
resize2fs string
xfsGrowfs string
}
var _ storageOps = (*guestStorage)(nil)
func newGuestStorage(cfg config, run commandRunner) *guestStorage {
return &guestStorage{
run: run,
dataMount: cfg.DataMount,
mountsFile: cfg.MountsFile,
sysBlock: cfg.SysBlock,
growpart: defaultGrowpart,
resize2fs: defaultResize2fs,
xfsGrowfs: defaultXFSGrowfs,
}
}
// The device and filesystem behind the data mount, resolved from the kernel's
// own mount table rather than from anything the control plane asserted.
type dataMount struct {
device string
fstype string
}
func (g *guestStorage) GrowFilesystem(ctx context.Context) (string, error) {
mount, err := g.resolveDataMount()
if err != nil {
return "", err
}
// rds-datadir formats the whole device and refuses a disk that already
// carries a partition table, so the data volume is normally unpartitioned
// and there is no partition to push out first.
if err := g.growPartition(ctx, mount.device); err != nil {
return "", err
}
switch {
case strings.HasPrefix(mount.fstype, "ext"):
// resize2fs grows a mounted ext filesystem online, and takes the device.
if _, err := g.run(ctx, command{
Name: g.resize2fs,
Args: []string{mount.device},
Env: []string{"PATH=" + defaultGuestPath},
}); err != nil {
return "", fmt.Errorf("grow the %s filesystem on %s: %w", mount.fstype, mount.device, err)
}
case mount.fstype == "xfs":
// xfs_growfs is addressed by mount point, and XFS can only grow mounted.
if _, err := g.run(ctx, command{
Name: g.xfsGrowfs,
Args: []string{g.dataMount},
Env: []string{"PATH=" + defaultGuestPath},
}); err != nil {
return "", fmt.Errorf("grow the xfs filesystem at %s: %w", g.dataMount, err)
}
default:
// Guessing a tool here would either do nothing or corrupt a filesystem
// this image was never meant to be holding.
return "", fmt.Errorf("%s holds an unsupported filesystem %q; refusing to grow it", mount.device, mount.fstype)
}
slog.Info("rds-agent: data filesystem grown", "device", mount.device, "fstype", mount.fstype, "mount", g.dataMount)
return fmt.Sprintf("grew %s (%s) at %s", mount.device, mount.fstype, g.dataMount), nil
}
// Pushes the partition out to the end of its disk when the data mount happens
// to sit on one. A device that is not a partition needs nothing, and a
// partition that already fills its disk is the normal state of a resumed grow.
func (g *guestStorage) growPartition(ctx context.Context, device string) error {
disk, number, ok := g.partitionOf(device)
if !ok {
return nil
}
out, err := g.run(ctx, command{
Name: g.growpart,
Args: []string{disk, number},
Env: []string{"PATH=" + defaultGuestPath},
})
if err == nil {
return nil
}
if strings.Contains(out, growpartNoChange) || strings.Contains(err.Error(), growpartNoChange) {
return nil
}
return fmt.Errorf("grow partition %s of %s: %w", number, disk, err)
}
// Whether device is a partition, and if so its disk and number. Read from
// sysfs, which states it directly rather than inferring it from a trailing
// digit — nvme0n1 is a disk and vdb1 is a partition on the same rule.
func (g *guestStorage) partitionOf(device string) (disk, number string, ok bool) {
name := filepath.Base(device)
raw, err := os.ReadFile(filepath.Join(g.sysBlock, name, "partition"))
if err != nil {
return "", "", false
}
number = strings.TrimSpace(string(raw))
if number == "" {
return "", "", false
}
// A partition's sysfs entry hangs off its disk's, so the parent directory
// names the disk without any parsing of the device name.
target, err := os.Readlink(filepath.Join(g.sysBlock, name))
if err != nil {
return "", "", false
}
return filepath.Join(filepath.Dir(device), filepath.Base(filepath.Dir(target))), number, true
}
// The mount table entry for the data mount. An absent one means the data volume
// is not mounted, which is a broken guest rather than something to grow.
func (g *guestStorage) resolveDataMount() (dataMount, error) {
f, err := os.Open(g.mountsFile)
if err != nil {
return dataMount{}, fmt.Errorf("read %s: %w", g.mountsFile, err)
}
defer f.Close()
// Later entries shadow earlier ones at the same point, so the last match is
// the filesystem actually visible there.
found := dataMount{}
scanner := bufio.NewScanner(f)
for scanner.Scan() {
fields := strings.Fields(scanner.Text())
if len(fields) < 3 || fields[1] != g.dataMount {
continue
}
found = dataMount{device: fields[0], fstype: fields[2]}
}
if err := scanner.Err(); err != nil {
return dataMount{}, fmt.Errorf("read %s: %w", g.mountsFile, err)
}
if found.device == "" {
return dataMount{}, fmt.Errorf("no filesystem is mounted at %s; the data volume is not attached", g.dataMount)
}
return found, nil
}
+226
View File
@@ -0,0 +1,226 @@
package main
import (
"context"
"errors"
"os"
"path/filepath"
"strings"
"testing"
handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds"
)
// Stands in for the guest's storage in the registry tests, where what matters
// is that the command reaches it rather than what it does to a filesystem.
type fakeStorage struct {
grown bool
message string
err error
}
var _ storageOps = (*fakeStorage)(nil)
func (f *fakeStorage) GrowFilesystem(context.Context) (string, error) {
f.grown = true
return f.message, f.err
}
// runLog records what a grow shelled out to, in order, so the test asserts on
// the tools invoked rather than on a filesystem it would have to create.
type runLog struct {
calls []command
// errOn fails the first command whose name matches, which is how growpart's
// tolerated failure and resize2fs's fatal one are told apart.
errOn string
errOut string
err error
}
func (r *runLog) run(_ context.Context, c command) (string, error) {
r.calls = append(r.calls, c)
if r.errOn != "" && c.Name == r.errOn {
return r.errOut, r.err
}
return "", nil
}
func (r *runLog) names() []string {
names := make([]string, 0, len(r.calls))
for _, c := range r.calls {
names = append(names, c.Name)
}
return names
}
// A guest whose data volume is mounted as fstype from device, with a sysfs tree
// that reports the device as a whole disk unless the test says otherwise.
func newTestStorage(t *testing.T, device, fstype string, run commandRunner) *guestStorage {
t.Helper()
dir := t.TempDir()
mounts := filepath.Join(dir, "mounts")
content := "proc /proc proc rw 0 0\n"
if device != "" {
content += device + " " + defaultDataMount + " " + fstype + " rw,relatime 0 0\n"
}
if err := os.WriteFile(mounts, []byte(content), 0o600); err != nil {
t.Fatalf("write the mount table: %v", err)
}
cfg := config{DataMount: defaultDataMount, MountsFile: mounts, SysBlock: filepath.Join(dir, "block")}
if err := os.MkdirAll(cfg.SysBlock, 0o755); err != nil {
t.Fatalf("create the sysfs tree: %v", err)
}
return newGuestStorage(cfg, run)
}
// Makes name look like partition number of disk, the way sysfs states it: a
// "partition" file under the device, hanging off its disk's directory.
func seedPartition(t *testing.T, g *guestStorage, disk, name, number string) {
t.Helper()
// sysfs holds the real entry under the device tree and links to it from
// /sys/class/block, so the partition's disk is its link target's parent.
target := filepath.Join("..", "devices", disk, name)
if err := os.MkdirAll(filepath.Join(g.sysBlock, target), 0o755); err != nil {
t.Fatalf("create the partition tree: %v", err)
}
if err := os.WriteFile(filepath.Join(g.sysBlock, target, "partition"), []byte(number+"\n"), 0o600); err != nil {
t.Fatalf("write the partition number: %v", err)
}
if err := os.Symlink(target, filepath.Join(g.sysBlock, name)); err != nil {
t.Fatalf("link the partition: %v", err)
}
}
// The data volume is formatted whole, so the usual grow is resize2fs against
// the device with no partition to push out first.
func TestGrowFilesystem_ResizesAnExt4DeviceOnline(t *testing.T) {
log := &runLog{}
g := newTestStorage(t, "/dev/vdb", "ext4", log.run)
message, err := g.GrowFilesystem(context.Background())
if err != nil {
t.Fatalf("GrowFilesystem: %v", err)
}
if got := log.names(); len(got) != 1 || got[0] != defaultResize2fs {
t.Fatalf("ran %v, want just %s", got, defaultResize2fs)
}
if args := log.calls[0].Args; len(args) != 1 || args[0] != "/dev/vdb" {
t.Errorf("resize2fs args = %v, want the device", args)
}
if !strings.Contains(message, "/dev/vdb") || !strings.Contains(message, defaultDataMount) {
t.Errorf("reply message = %q, want the device and mount it grew", message)
}
}
// XFS is addressed by mount point and can only grow mounted, unlike ext.
func TestGrowFilesystem_GrowsXFSByMountPoint(t *testing.T) {
log := &runLog{}
g := newTestStorage(t, "/dev/vdb", "xfs", log.run)
if _, err := g.GrowFilesystem(context.Background()); err != nil {
t.Fatalf("GrowFilesystem: %v", err)
}
if got := log.names(); len(got) != 1 || got[0] != defaultXFSGrowfs {
t.Fatalf("ran %v, want just %s", got, defaultXFSGrowfs)
}
if args := log.calls[0].Args; len(args) != 1 || args[0] != defaultDataMount {
t.Errorf("xfs_growfs args = %v, want the mount point", args)
}
}
// A partitioned data device has to have the partition pushed out to the end of
// the disk before the filesystem can be extended onto it.
func TestGrowFilesystem_PushesThePartitionOutFirst(t *testing.T) {
log := &runLog{}
g := newTestStorage(t, "/dev/vdb1", "ext4", log.run)
seedPartition(t, g, "vdb", "vdb1", "1")
if _, err := g.GrowFilesystem(context.Background()); err != nil {
t.Fatalf("GrowFilesystem: %v", err)
}
if got := log.names(); len(got) != 2 || got[0] != defaultGrowpart || got[1] != defaultResize2fs {
t.Fatalf("ran %v, want growpart then resize2fs", got)
}
if args := log.calls[0].Args; len(args) != 2 || args[0] != "/dev/vdb" || args[1] != "1" {
t.Errorf("growpart args = %v, want the disk and the partition number", args)
}
}
// A resumed grow finds the partition already filling its disk, which growpart
// reports as a failure. Treating it as one would fail a grow that has nothing
// left to do at that step.
func TestGrowFilesystem_ToleratesAnAlreadyFullPartition(t *testing.T) {
log := &runLog{errOn: defaultGrowpart, errOut: "NOCHANGE: partition 1 is size 41940992. it cannot be grown", err: errors.New("exit status 1")}
g := newTestStorage(t, "/dev/vdb1", "ext4", log.run)
seedPartition(t, g, "vdb", "vdb1", "1")
if _, err := g.GrowFilesystem(context.Background()); err != nil {
t.Fatalf("GrowFilesystem: %v", err)
}
if got := log.names(); len(got) != 2 || got[1] != defaultResize2fs {
t.Errorf("ran %v, want the resize to follow the tolerated growpart", got)
}
}
// Any other growpart failure is a real one, and extending a filesystem onto
// capacity the partition does not have would be the corrupting move.
func TestGrowFilesystem_FailsOnARealGrowpartError(t *testing.T) {
log := &runLog{errOn: defaultGrowpart, errOut: "unable to read partition table", err: errors.New("exit status 2")}
g := newTestStorage(t, "/dev/vdb1", "ext4", log.run)
seedPartition(t, g, "vdb", "vdb1", "1")
if _, err := g.GrowFilesystem(context.Background()); err == nil {
t.Fatal("GrowFilesystem succeeded on a failed growpart, want an error")
}
if got := log.names(); len(got) != 1 {
t.Errorf("ran %v, want nothing after the failed growpart", got)
}
}
// Guessing a tool for an unrecognised filesystem would either do nothing or
// corrupt one this image was never meant to be holding.
func TestGrowFilesystem_RefusesAnUnsupportedFilesystem(t *testing.T) {
log := &runLog{}
g := newTestStorage(t, "/dev/vdb", "btrfs", log.run)
_, err := g.GrowFilesystem(context.Background())
if err == nil || !strings.Contains(err.Error(), "btrfs") {
t.Fatalf("err = %v, want a refusal naming the filesystem", err)
}
if len(log.calls) != 0 {
t.Errorf("ran %v against an unsupported filesystem, want nothing", log.names())
}
}
// Nothing mounted at the data mount is a broken guest rather than a grow to
// attempt against whatever else happens to be there.
func TestGrowFilesystem_FailsWhenTheDataVolumeIsNotMounted(t *testing.T) {
log := &runLog{}
g := newTestStorage(t, "", "", log.run)
if _, err := g.GrowFilesystem(context.Background()); err == nil {
t.Fatal("GrowFilesystem succeeded with no data volume mounted, want an error")
}
if len(log.calls) != 0 {
t.Errorf("ran %v with nothing mounted, want nothing", log.names())
}
}
// The control plane's grow-filesystem directive has to reach the guest's
// storage, or a grown volume never becomes usable capacity.
func TestCommandRegistry_GrowFilesystemReachesTheGuestStorage(t *testing.T) {
storage := &fakeStorage{message: "grew /dev/vdb (ext4) at /var/lib/postgresql"}
reply := newCommander(nil, newCommandRegistry(&fakeEngine{}, storage), 0).execute(context.Background(),
handlers_rds.Command{CommandID: "cmd-9", Type: handlers_rds.CommandGrowFilesystem})
if !storage.grown {
t.Error("the grow-filesystem command did not reach the guest storage")
}
if reply.Status != handlers_rds.CommandStatusSucceeded || reply.Message != storage.message {
t.Errorf("reply = %+v, want succeeded carrying what the grow reported", reply)
}
}
+1
View File
@@ -1061,6 +1061,7 @@ func (d *Daemon) subscribeAll() error {
natsSub{handlers_rds.SubjectRebootDBInstance, handleNATSRequest(d.rdsService.RebootDBInstance), "spinifex-workers"},
natsSub{handlers_rds.SubjectStartDBInstance, handleNATSRequest(d.rdsService.StartDBInstance), "spinifex-workers"},
natsSub{handlers_rds.SubjectStopDBInstance, handleNATSRequest(d.rdsService.StopDBInstance), "spinifex-workers"},
natsSub{handlers_rds.SubjectModifyDBInstance, handleNATSRequest(d.rdsService.ModifyDBInstance), "spinifex-workers"},
natsSub{handlers_rds.SubjectDeleteDBInstance, handleNATSRequest(d.rdsService.DeleteDBInstance), "spinifex-workers"},
natsSub{handlers_rds.SubjectDescribeEvents, handleNATSRequest(d.rdsService.DescribeEvents), "spinifex-workers"},
natsSub{handlers_rds.SubjectAddTagsToResource, handleNATSRequest(d.rdsService.AddTagsToResource), "spinifex-workers"},
+1
View File
@@ -57,6 +57,7 @@ func (d *Daemon) buildRDSDeps() handlers_rds.Deps {
InstanceState: handlers_rds.NewDescribeInstanceState(d.describeInstancesFanOut),
Instances: handlers_rds.NewNATSInstanceCommander(d.natsConn),
Snapshots: d.snapshotService,
Storage: d.volumeService,
BaseDomain: d.dnsBaseDomain,
HolderID: d.node,
// A DB VM reaches the daemon only over the mgmt bridge, the same
+1 -1
View File
@@ -81,7 +81,7 @@ var actions = map[string]Handler{
// Instance lifecycle.
"CreateDBInstance": typed(CreateDBInstance),
"DescribeDBInstances": typed(DescribeDBInstances),
"ModifyDBInstance": pending(),
"ModifyDBInstance": typed(ModifyDBInstance),
"DeleteDBInstance": typed(DeleteDBInstance),
"RebootDBInstance": typed(RebootDBInstance),
"StartDBInstance": typed(StartDBInstance),
+13 -7
View File
@@ -1,10 +1,13 @@
package gateway_rds
import (
"bytes"
"encoding/json"
"encoding/xml"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/private/protocol/xml/xmlutil"
"github.com/aws/aws-sdk-go/service/rds"
"github.com/mulgadc/spinifex/spinifex/awserrors"
handlers_rds "github.com/mulgadc/spinifex/spinifex/handlers/rds"
@@ -103,7 +106,7 @@ func TestDispatch_UnknownAction(t *testing.T) {
// The customer actions this phase implements forward to the daemon, so they are
// dispatched against a stub responder rather than a nil connection.
var liveActions = []string{
"CreateDBInstance", "DescribeDBInstances",
"CreateDBInstance", "DescribeDBInstances", "ModifyDBInstance",
"RebootDBInstance", "StartDBInstance", "StopDBInstance", "DeleteDBInstance",
"DescribeEvents",
"AddTagsToResource", "RemoveTagsFromResource", "ListTagsForResource",
@@ -118,6 +121,7 @@ func newStubbedNATS(t *testing.T) *nats.Conn {
&rds.CreateDBInstanceOutput{DBInstance: &rds.DBInstance{DBInstanceIdentifier: aws.String("orders-db")}})
respondWith(t, nc, handlers_rds.SubjectDescribeDBInstances,
&rds.DescribeDBInstancesOutput{DBInstances: []*rds.DBInstance{}})
respondWith(t, nc, handlers_rds.SubjectModifyDBInstance, &rds.ModifyDBInstanceOutput{})
respondWith(t, nc, handlers_rds.SubjectRebootDBInstance, &rds.RebootDBInstanceOutput{})
respondWith(t, nc, handlers_rds.SubjectStartDBInstance, &rds.StartDBInstanceOutput{})
respondWith(t, nc, handlers_rds.SubjectStopDBInstance, &rds.StopDBInstanceOutput{})
@@ -178,7 +182,7 @@ func TestDispatch_LiveActionsAreNotPending(t *testing.T) {
}
func TestDispatch_PendingActionIsNotImplemented(t *testing.T) {
_, err := Dispatch(t.Context(), "ModifyDBInstance", map[string]string{"Action": "ModifyDBInstance"}, nil, testCaller)
_, err := Dispatch(t.Context(), "CreateDBSnapshot", map[string]string{"Action": "CreateDBSnapshot"}, nil, testCaller)
require.Error(t, err)
assert.Equal(t, awserrors.ErrorNotImplemented, err.Error())
}
@@ -213,11 +217,13 @@ func TestDispatch_ListTagsForResourceRendersTheNestedTagList(t *testing.T) {
}, newStubbedNATS(t), testCaller)
require.NoError(t, err)
assert.Equal(t,
"<ListTagsForResourceResponse><ListTagsForResourceResult><TagList>"+
"<Tag><Key>env</Key><Value>prod</Value></Tag>"+
"</TagList></ListTagsForResourceResult></ListTagsForResourceResponse>",
string(body))
// Decoded the way a real client decodes it, rather than compared byte-wise:
// BuildXML emits sibling elements in map order, so <Key> and <Value> swap
// places between runs.
var out rds.ListTagsForResourceOutput
require.NoError(t, xmlutil.UnmarshalXML(&out, xml.NewDecoder(bytes.NewReader(body)), "ListTagsForResourceResult"))
assert.Equal(t, []*rds.Tag{{Key: aws.String("env"), Value: aws.String("prod")}}, out.TagList)
}
// RDS serializes a tag list under its own locationNameList, so the params
+4
View File
@@ -30,6 +30,10 @@ func StopDBInstance(ctx context.Context, input *rds.StopDBInstanceInput, nc *nat
return handlers_rds.NewNATSService(nc).StopDBInstance(ctx, input, caller.AccountID)
}
func ModifyDBInstance(ctx context.Context, input *rds.ModifyDBInstanceInput, nc *nats.Conn, caller Caller) (any, error) {
return handlers_rds.NewNATSService(nc).ModifyDBInstance(ctx, input, caller.AccountID)
}
func DeleteDBInstance(ctx context.Context, input *rds.DeleteDBInstanceInput, nc *nats.Conn, caller Caller) (any, error) {
return handlers_rds.NewNATSService(nc).DeleteDBInstance(ctx, input, caller.AccountID)
}
+16 -1
View File
@@ -17,11 +17,14 @@ import (
// long poll over the gateway; this publishes on the bus subject that poll is
// subscribed to and waits for the reply the agent carries back on its next one.
// Command types. rds-5b adds grow-filesystem and rds-8 the snapshot quiesce.
// Command types. rds-8 adds the snapshot quiesce.
const (
CommandSetPassword = "set-password"
CommandApplyParams = "apply-params"
CommandStopEngine = "stop-engine"
// Extends the in-guest filesystem onto a data volume the control plane has
// already grown (D12).
CommandGrowFilesystem = "grow-filesystem"
)
// Parameter names carried on a command. They are AWS-shaped rather than
@@ -38,6 +41,10 @@ const (
setPasswordTimeout = 30 * time.Second
applyParamsTimeout = 60 * time.Second
stopEngineTimeout = 120 * time.Second
// growpart plus an online resize2fs/xfs_growfs. Both scale with the
// filesystem's metadata rather than with the volume, so this is generous
// rather than proportional to the grow.
growFilesystemTimeout = 180 * time.Second
// The channel is a live subscription rather than a durable queue (D8), so a
// command published between two of the agent's polls reaches nobody.
@@ -79,6 +86,14 @@ func (s *Service) stopEngine(ctx context.Context, accountID, dbInstanceIdentifie
return err
}
// Extends the guest's filesystem onto the already-grown data volume. Issued
// once the agent is back rather than during boot: both ext4 and XFS grow while
// mounted, so this needs no ordering against the engine start (D12).
func (s *Service) growFilesystem(ctx context.Context, accountID, dbInstanceIdentifier string) error {
_, err := s.issueCommand(ctx, accountID, dbInstanceIdentifier, CommandGrowFilesystem, growFilesystemTimeout, nil)
return err
}
// Publishes one directive and blocks until the agent replies to that command
// ID, the budget expires, or ctx is cancelled. A reply reporting failure is
// returned as an error carrying the agent's message, so the caller does not
+55
View File
@@ -103,6 +103,17 @@ func (s *Service) projectDBInstance(rec *DBInstanceRecord) *rds.DBInstance {
Status: aws.String("active"),
})
}
if rec.BackupRetentionPeriod > 0 {
out.BackupRetentionPeriod = aws.Int64(rec.BackupRetentionPeriod)
}
if rec.PreferredBackupWindow != "" {
out.PreferredBackupWindow = aws.String(rec.PreferredBackupWindow)
}
if rec.PreferredMaintenanceWindow != "" {
out.PreferredMaintenanceWindow = aws.String(rec.PreferredMaintenanceWindow)
}
out.PendingModifiedValues = projectPendingModifiedValues(rec.PendingModifiedValues)
out.DBParameterGroups = projectParameterGroup(rec)
// Absent until the ENI exists: an Endpoint with no address would have a
// client dial an empty host rather than wait for the instance to come up.
if rec.EndpointAddress != "" {
@@ -113,3 +124,47 @@ func (s *Service) projectDBInstance(rec *DBInstanceRecord) *rds.DBInstance {
}
return out
}
// What a modify asked for and has not yet delivered. Nil when nothing is
// outstanding, so a client polling a deferred change sees the field appear and
// disappear rather than an empty element it has to interpret.
//
// A pending filesystem grow is deliberately absent: the volume is already at the
// new size, so the customer's AllocatedStorage is the new one and reporting it
// as still pending would show Terraform drift on a change that has landed. A
// pending parameter group is absent too — AWS reports that on the parameter
// group's own apply status, not here.
func projectPendingModifiedValues(pending *PendingModifiedValues) *rds.PendingModifiedValues {
if pending.empty() || (pending.AllocatedStorage == nil && pending.DBInstanceClass == "") {
return nil
}
out := &rds.PendingModifiedValues{}
if pending.AllocatedStorage != nil {
out.AllocatedStorage = aws.Int64(*pending.AllocatedStorage)
}
if pending.DBInstanceClass != "" {
out.DBInstanceClass = aws.String(pending.DBInstanceClass)
}
return out
}
// AWS reports a parameter group's state on the membership rather than in
// PendingModifiedValues, and the Terraform provider reads it there: applying
// while a modify is draining, pending-reboot while static settings await the
// restart that adopts them (D16).
func projectParameterGroup(rec *DBInstanceRecord) []*rds.DBParameterGroupStatus {
if rec.DBParameterGroupName == "" {
return nil
}
status := "in-sync"
switch {
case rec.PendingModifiedValues != nil && rec.PendingModifiedValues.DBParameterGroupName != "":
status = "applying"
case len(rec.PendingRebootParameters) > 0:
status = "pending-reboot"
}
return []*rds.DBParameterGroupStatus{{
DBParameterGroupName: aws.String(rec.DBParameterGroupName),
ParameterApplyStatus: aws.String(status),
}}
}
+67
View File
@@ -162,3 +162,70 @@ func TestDesiredDNSChanges_SkipsDeletedButKeepsFailed(t *testing.T) {
require.Len(t, changes, 1)
assert.Equal(t, failed.DNSName, changes[0].Name)
}
// A deferred change is what the customer polls for, and the Terraform provider
// reads PendingModifiedValues to decide whether an apply has landed.
func TestProjectDBInstance_ReportsTheDeferredChange(t *testing.T) {
h := newCreateHarness(t, "")
instance := h.svc.projectDBInstance(&DBInstanceRecord{
DBInstanceIdentifier: testDBInstanceID,
Status: StatusAvailable,
DBInstanceClass: "db.t3.medium",
AllocatedStorage: 20,
PendingModifiedValues: &PendingModifiedValues{
AllocatedStorage: aws.Int64(50),
DBInstanceClass: "db.m5.large",
},
})
require.NotNil(t, instance.PendingModifiedValues)
assert.Equal(t, int64(50), aws.Int64Value(instance.PendingModifiedValues.AllocatedStorage))
assert.Equal(t, "db.m5.large", aws.StringValue(instance.PendingModifiedValues.DBInstanceClass))
// The current values still report what is actually in effect.
assert.Equal(t, int64(20), aws.Int64Value(instance.AllocatedStorage))
assert.Equal(t, "db.t3.medium", aws.StringValue(instance.DBInstanceClass))
}
// The volume is already at the new size once only the in-guest grow is left,
// so reporting it as still pending would show Terraform drift on a change that
// has landed.
func TestProjectDBInstance_OmitsAPendingFilesystemGrow(t *testing.T) {
h := newCreateHarness(t, "")
instance := h.svc.projectDBInstance(&DBInstanceRecord{
DBInstanceIdentifier: testDBInstanceID,
Status: StatusModifying,
AllocatedStorage: 50,
PendingModifiedValues: &PendingModifiedValues{FilesystemGrowPending: true},
})
assert.Nil(t, instance.PendingModifiedValues)
}
// AWS reports a parameter group's state on the membership rather than in
// PendingModifiedValues, and the Terraform provider reads it there.
func TestProjectDBInstance_ReportsTheParameterGroupApplyStatus(t *testing.T) {
h := newCreateHarness(t, "")
rec := &DBInstanceRecord{
DBInstanceIdentifier: testDBInstanceID,
Status: StatusAvailable,
DBParameterGroupName: "default.postgres18",
}
statusOf := func(rec *DBInstanceRecord) string {
groups := h.svc.projectDBInstance(rec).DBParameterGroups
require.Len(t, groups, 1)
assert.Equal(t, "default.postgres18", aws.StringValue(groups[0].DBParameterGroupName))
return aws.StringValue(groups[0].ParameterApplyStatus)
}
assert.Equal(t, "in-sync", statusOf(rec))
// D16: static settings are in the engine's config but not in effect until
// the restart that adopts them.
rec.PendingRebootParameters = []string{"shared_buffers"}
assert.Equal(t, "pending-reboot", statusOf(rec))
rec.PendingModifiedValues = &PendingModifiedValues{DBParameterGroupName: "default.postgres18"}
assert.Equal(t, "applying", statusOf(rec))
}
+103 -38
View File
@@ -53,6 +53,12 @@ type launchVPCProvisioner interface {
CreateNetworkInterface(ctx context.Context, input *ec2.CreateNetworkInterfaceInput, accountID string) (*ec2.CreateNetworkInterfaceOutput, error)
DeleteNetworkInterface(ctx context.Context, input *ec2.DeleteNetworkInterfaceInput, accountID string) (*ec2.DeleteNetworkInterfaceOutput, error)
DetachENI(ctx context.Context, accountID, eniID string) error
// A replace re-attaches the persisted customer ENI, so it has to read back
// the MAC the launcher needs; only the IP is on the DB instance record.
DescribeNetworkInterfaces(ctx context.Context, input *ec2.DescribeNetworkInterfacesInput, accountID string) (*ec2.DescribeNetworkInterfacesOutput, error)
// Re-associates the customer ENI's security groups in place, which is what
// makes a VpcSecurityGroupIds modify need no VM replace and no new IP.
ModifyNetworkInterfaceAttribute(ctx context.Context, input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error)
}
// System-managed VMs get a mgmt-bridge NIC alongside their VPC NICs, which is
@@ -105,6 +111,13 @@ type LaunchInput struct {
// Fails the launch when the created data volume comes back unencrypted,
// rather than reporting an encryption the customer did not get.
RequireEncryptedData bool
// A replace re-uses the DB instance's persisted customer ENI and data
// volume instead of minting new ones (D5/D9): the endpoint address and the
// datadir are the identity that must survive the VM. Both empty is a
// create, which builds them.
ExistingCustomerENI string
ExistingDataVolume string
}
type LaunchOutput struct {
@@ -119,7 +132,9 @@ type LaunchOutput struct {
DataDevice string
MgmtIP string
// The volume's own reported state, not an echo of the request, so
// DescribeDBInstances reports encryption the way EC2 does.
// DescribeDBInstances reports encryption the way EC2 does. Only set when
// this launch created the volume: a replace re-attaches one whose
// encryption the record already carries.
DataVolumeEncrypted bool
// Tears down everything this launch created, for a caller that fails after
// it returned. The launch runs it itself on its own failures, so it is only
@@ -182,14 +197,18 @@ func LaunchDBInstanceVM(ctx context.Context, deps LaunchDeps, in LaunchInput) (o
deleteLaunchENI(ctx, deps.VPC, utils.GlobalAccountID, systemENI.id)
})
customerENI, err := createLaunchENI(ctx, deps.VPC, in.AccountID, in.SubnetID, in.SecurityGroupIDs,
"RDS endpoint ENI for "+in.DBInstanceIdentifier, in.DBInstanceIdentifier)
// A replace adopts the ENI the endpoint already resolves to; only a create
// mints one, and only a create may unwind one — deleting the persisted ENI
// on a failed replace would take the endpoint with it.
customerENI, err := resolveCustomerENI(ctx, deps.VPC, in)
if err != nil {
return nil, err
}
rollback = append(rollback, func(ctx context.Context) {
deleteLaunchENI(ctx, deps.VPC, in.AccountID, customerENI.id)
})
if in.ExistingCustomerENI == "" {
rollback = append(rollback, func(ctx context.Context) {
deleteLaunchENI(ctx, deps.VPC, in.AccountID, customerENI.id)
})
}
sysOut, err := deps.Instance.LaunchSystemInstance(&sysinstance.SystemInstanceInput{
BootMode: sysinstance.BootAMI,
@@ -228,40 +247,45 @@ func LaunchDBInstanceVM(ctx context.Context, deps LaunchDeps, in LaunchInput) (o
}
}
// Kept separate from the boot volume so a replace can discard the boot
// volume and keep the datadir, and owned by the system account.
volume, err := deps.Volume.CreateVolume(ctx, &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(az),
Size: aws.Int64(in.AllocatedStorage),
VolumeType: aws.String("gp3"),
TagSpecifications: []*ec2.TagSpecification{{
ResourceType: aws.String("volume"),
Tags: []*ec2.Tag{
{Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByRDS)},
{Key: aws.String(rdsInstanceTagKey), Value: aws.String(in.DBInstanceIdentifier)},
},
}},
}, utils.GlobalAccountID)
if err != nil {
return nil, fmt.Errorf("rds: create data volume for %s: %w", in.DBInstanceIdentifier, err)
}
if volume == nil || aws.StringValue(volume.VolumeId) == "" {
return nil, fmt.Errorf("rds: create data volume for %s: empty volume id", in.DBInstanceIdentifier)
}
volumeID := aws.StringValue(volume.VolumeId)
volumeEncrypted := aws.BoolValue(volume.Encrypted)
rollback = append(rollback, func(ctx context.Context) {
if _, delErr := deps.Volume.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}, utils.GlobalAccountID); delErr != nil {
slog.WarnContext(ctx, "rds: rollback delete of orphaned data volume failed",
"dbInstance", in.DBInstanceIdentifier, "volumeId", volumeID, "err", delErr)
// A replace re-attaches the datadir it already has; only a create makes one,
// and only a create may unwind one.
volumeID, volumeEncrypted := in.ExistingDataVolume, false
if volumeID == "" {
// Kept separate from the boot volume so a replace can discard the boot
// volume and keep the datadir, and owned by the system account.
volume, volErr := deps.Volume.CreateVolume(ctx, &ec2.CreateVolumeInput{
AvailabilityZone: aws.String(az),
Size: aws.Int64(in.AllocatedStorage),
VolumeType: aws.String("gp3"),
TagSpecifications: []*ec2.TagSpecification{{
ResourceType: aws.String("volume"),
Tags: []*ec2.Tag{
{Key: aws.String(tags.ManagedByKey), Value: aws.String(tags.ManagedByRDS)},
{Key: aws.String(rdsInstanceTagKey), Value: aws.String(in.DBInstanceIdentifier)},
},
}},
}, utils.GlobalAccountID)
if volErr != nil {
return nil, fmt.Errorf("rds: create data volume for %s: %w", in.DBInstanceIdentifier, volErr)
}
})
if volume == nil || aws.StringValue(volume.VolumeId) == "" {
return nil, fmt.Errorf("rds: create data volume for %s: empty volume id", in.DBInstanceIdentifier)
}
volumeID = aws.StringValue(volume.VolumeId)
volumeEncrypted = aws.BoolValue(volume.Encrypted)
rollback = append(rollback, func(ctx context.Context) {
if _, delErr := deps.Volume.DeleteVolume(ctx, &ec2.DeleteVolumeInput{VolumeId: aws.String(volumeID)}, utils.GlobalAccountID); delErr != nil {
slog.WarnContext(ctx, "rds: rollback delete of orphaned data volume failed",
"dbInstance", in.DBInstanceIdentifier, "volumeId", volumeID, "err", delErr)
}
})
// Checked before the attach so an unencrypted volume is unwound rather than
// mounted: the cluster storage key is unset, which no retry here can fix.
if in.RequireEncryptedData && !volumeEncrypted {
return nil, fmt.Errorf("rds: data volume %s for %s was created unencrypted; the cluster storage key is not configured",
volumeID, in.DBInstanceIdentifier)
// Checked before the attach so an unencrypted volume is unwound rather than
// mounted: the cluster storage key is unset, which no retry here can fix.
if in.RequireEncryptedData && !volumeEncrypted {
return nil, fmt.Errorf("rds: data volume %s for %s was created unencrypted; the cluster storage key is not configured",
volumeID, in.DBInstanceIdentifier)
}
}
device, err := deps.Attacher.AttachVolume(ctx, utils.GlobalAccountID, instanceID, volumeID, dataVolumeDevice)
@@ -345,6 +369,47 @@ func createLaunchENI(ctx context.Context, vpcSvc launchVPCProvisioner, accountID
}, nil
}
// The customer NIC this launch attaches: a fresh one for a create, the DB
// instance's persisted one for a replace. The persisted case is read back
// rather than reconstructed, because the launcher needs the MAC and only the
// address is on the record.
func resolveCustomerENI(ctx context.Context, vpcSvc launchVPCProvisioner, in LaunchInput) (*launchENI, error) {
if in.ExistingCustomerENI == "" {
return createLaunchENI(ctx, vpcSvc, in.AccountID, in.SubnetID, in.SecurityGroupIDs,
"RDS endpoint ENI for "+in.DBInstanceIdentifier, in.DBInstanceIdentifier)
}
// The old VM is gone by now but its attachment record can outlive it, and a
// re-attach of an ENI that still reads as attached is rejected.
if err := vpcSvc.DetachENI(ctx, in.AccountID, in.ExistingCustomerENI); err != nil && !awserrors.IsNotFound(err) {
return nil, fmt.Errorf("rds: detach the endpoint ENI %s before re-attaching it: %w", in.ExistingCustomerENI, err)
}
out, err := vpcSvc.DescribeNetworkInterfaces(ctx, &ec2.DescribeNetworkInterfacesInput{
NetworkInterfaceIds: aws.StringSlice([]string{in.ExistingCustomerENI}),
}, in.AccountID)
if err != nil {
return nil, fmt.Errorf("rds: read the endpoint ENI %s: %w", in.ExistingCustomerENI, err)
}
for _, ni := range out.NetworkInterfaces {
if aws.StringValue(ni.NetworkInterfaceId) != in.ExistingCustomerENI {
continue
}
eni := &launchENI{
id: in.ExistingCustomerENI,
ip: aws.StringValue(ni.PrivateIpAddress),
mac: aws.StringValue(ni.MacAddress),
}
if eni.ip == "" || eni.mac == "" {
return nil, fmt.Errorf("rds: endpoint ENI %s has no address or MAC to re-attach", in.ExistingCustomerENI)
}
return eni, nil
}
// Failing here is the only safe answer: minting a replacement would move the
// endpoint to a new address the DNS record and serving cert do not name.
return nil, fmt.Errorf("rds: endpoint ENI %s no longer exists", in.ExistingCustomerENI)
}
// Detach comes first because a terminated VM can leave the attachment record
// behind, which a plain delete rejects as InUse.
func deleteLaunchENI(ctx context.Context, vpcSvc launchVPCProvisioner, accountID, eniID string) {
+54 -2
View File
@@ -22,6 +22,11 @@ const (
testCustomerAccount = "123456789012"
testDBSubnet = "subnet-customer-db"
testEngineAMI = "ami-rds-postgres-18"
// The persisted endpoint a replace re-attaches: its address is the DNS
// target and its MAC is what the replacement VM's NIC has to come up with.
testEndpointIP = "10.20.30.40"
testEndpointMAC = "02:00:00:00:aa:01"
)
// --- Fakes ---
@@ -171,6 +176,18 @@ type fakeENIs struct {
// its independence from the caller's context become observable.
unwind *[]string
deleteCtxErr error
// A replace re-attaches the persisted endpoint ENI rather than minting one,
// so what it detached, read back and re-associated is where that becomes
// observable.
detached []string
described []string
modified []*ec2.ModifyNetworkInterfaceAttributeInput
// describeMissing makes the endpoint ENI read as gone, which is the state a
// replace must refuse to mint a replacement address for.
describeMissing bool
modifyErr error
}
var _ launchVPCProvisioner = (*fakeENIs)(nil)
@@ -199,7 +216,38 @@ func (f *fakeENIs) DeleteNetworkInterface(ctx context.Context, in *ec2.DeleteNet
return &ec2.DeleteNetworkInterfaceOutput{}, nil
}
func (f *fakeENIs) DetachENI(context.Context, string, string) error { return nil }
func (f *fakeENIs) DetachENI(_ context.Context, _, eniID string) error {
f.detached = append(f.detached, eniID)
return nil
}
// Answers for whatever was asked about, since the launch only ever reads back
// an ENI it or a previous launch created. The address is derived from the ID so
// a re-attach observably keeps the endpoint it already had.
func (f *fakeENIs) DescribeNetworkInterfaces(_ context.Context, in *ec2.DescribeNetworkInterfacesInput, _ string) (*ec2.DescribeNetworkInterfacesOutput, error) {
out := &ec2.DescribeNetworkInterfacesOutput{}
for _, id := range aws.StringValueSlice(in.NetworkInterfaceIds) {
f.described = append(f.described, id)
if f.describeMissing {
continue
}
out.NetworkInterfaces = append(out.NetworkInterfaces, &ec2.NetworkInterface{
NetworkInterfaceId: aws.String(id),
PrivateIpAddress: aws.String(testEndpointIP),
MacAddress: aws.String(testEndpointMAC),
SubnetId: aws.String(testDBSubnet),
})
}
return out, nil
}
func (f *fakeENIs) ModifyNetworkInterfaceAttribute(_ context.Context, in *ec2.ModifyNetworkInterfaceAttributeInput, _ string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) {
if f.modifyErr != nil {
return nil, f.modifyErr
}
f.modified = append(f.modified, in)
return &ec2.ModifyNetworkInterfaceAttributeOutput{}, nil
}
// fakeLauncher stands in for the system-instance launcher.
type fakeLauncher struct {
@@ -208,6 +256,10 @@ type fakeLauncher struct {
terminated []string
unwind *[]string
// terminateErr fails the teardown of the superseded VM, which is the step a
// replace cannot proceed past: the volume is still attached to it.
terminateErr error
// onLaunch runs once the VM exists, for tests that need to disturb state
// after the launch has committed resources but before the caller records it.
onLaunch func()
@@ -231,7 +283,7 @@ func (f *fakeLauncher) TerminateSystemInstance(instanceID string) error {
if f.unwind != nil {
*f.unwind = append(*f.unwind, "terminate")
}
return nil
return f.terminateErr
}
// fakeImages answers the engine-AMI lookup from a canned image list.
+31 -10
View File
@@ -109,11 +109,7 @@ func (s *Service) StopDBInstance(ctx context.Context, input *rds.StopDBInstanceI
s.stopEngineOrRecordFallback(ctx, accountID, rec, "stopping")
err = s.deps.Instances.StopInstance(ctx, rec.InstanceID)
if errors.Is(err, ErrInstanceNotOnNode) {
err = s.confirmVMNotRunning(ctx, accountID, rec.InstanceID)
}
if err != nil {
if err := s.stopInstanceVM(ctx, accountID, rec.InstanceID); err != nil {
return nil, s.failTransition(ctx, kv, accountID, rec,
fmt.Sprintf("the DB instance could not be stopped: %v", err))
}
@@ -153,6 +149,17 @@ func (s *Service) StartDBInstance(ctx context.Context, input *rds.StartDBInstanc
return &rds.StartDBInstanceOutput{DBInstance: s.projectDBInstance(rec)}, nil
}
// Stops the VM, treating an unanswered command as a stop only once the fleet
// agrees the VM is not running. Shared with the storage grow, which needs the
// VM genuinely down before ModifyVolume will touch its volume.
func (s *Service) stopInstanceVM(ctx context.Context, accountID, instanceID string) error {
err := s.deps.Instances.StopInstance(ctx, instanceID)
if errors.Is(err, ErrInstanceNotOnNode) {
err = s.confirmVMNotRunning(ctx, accountID, instanceID)
}
return err
}
// The owning node's in-memory command path first, since it is the one that can
// re-attach the volumes it already has mapped. A node restart drops that, and
// the stopped-instance path relaunches from the persisted record instead.
@@ -275,24 +282,38 @@ func (s *Service) failTransition(ctx context.Context, kv jetstream.KeyValue, acc
// A read-modify-write under CAS, replayed on contention so a concurrent agent
// heartbeat cannot make a lifecycle write disappear.
func (s *Service) updateInstance(ctx context.Context, kv jetstream.KeyValue, id string, mutate func(*DBInstanceRecord)) error {
_, err := s.updateInstanceIf(ctx, kv, id, func(rec *DBInstanceRecord) bool {
mutate(rec)
return true
})
return err
}
// updateInstance for a caller that can only decide whether to write once it has
// seen the record it would overwrite — a lease claim, whose whole question is
// whether someone else holds it. Reports whether the write happened; a mutate
// that returns false leaves the record untouched and is not an error.
func (s *Service) updateInstanceIf(ctx context.Context, kv jetstream.KeyValue, id string, mutate func(*DBInstanceRecord) bool) (bool, error) {
key := DBInstanceKey(id)
for range tagWriteAttempts {
rec, rev, err := s.getDBInstance(ctx, kv, id)
if err != nil {
return err
return false, err
}
if !mutate(rec) {
return false, nil
}
mutate(rec)
rec.UpdatedAt = time.Now().UTC()
err = updateJSON(ctx, kv, key, rev, rec)
if err == nil {
return nil
return true, nil
}
if !errors.Is(err, jetstream.ErrKeyExists) {
return err
return false, err
}
}
return fmt.Errorf("rds: update of %s contended after %d attempts", key, tagWriteAttempts)
return false, fmt.Errorf("rds: update of %s contended after %d attempts", key, tagWriteAttempts)
}
func joinStatuses(statuses []Status) string {
+415
View File
@@ -0,0 +1,415 @@
package handlers_rds
import (
"context"
"errors"
"fmt"
"log/slog"
"slices"
"strings"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/aws/aws-sdk-go/service/rds"
"github.com/mulgadc/spinifex/spinifex/awserrors"
"github.com/nats-io/nats.go/jetstream"
)
// AWS's bound on automated-backup retention. Nothing consumes it until rds-9,
// but accepting a value outside the range would store a setting that fails
// later, in a window nobody is watching.
const maxBackupRetentionDays = 35
// The resolved effect of a ModifyDBInstance request: what changes now and what
// is left for a maintenance window. Every field is a *difference* from the
// stored record — Terraform sends the whole body on every apply, so a field
// repeating its current value contributes nothing rather than an outage.
type modifyPlan struct {
// Applied on the spot whatever ApplyImmediately says, because none of them
// interrupts service and AWS applies them as soon as possible too. The
// password especially: D8 forbids persisting the cleartext, so there is
// nothing that could be deferred.
MasterUserPassword string
SecurityGroupIDs []string
DeletionProtection *bool
BackupRetentionPeriod *int64
PreferredBackupWindow string
PreferredMaintenanceWindow string
// Disruptive: each one takes the engine down, so ApplyImmediately=false
// records them as pending instead of applying them.
AllocatedStorage *int64
InstanceClass string
InstanceType string
ParameterGroup string
ApplyImmediately bool
}
// Whether anything in the plan takes the engine down, which is what decides
// between applying now and recording pending values.
func (p *modifyPlan) disruptive() bool {
return p.AllocatedStorage != nil || p.InstanceClass != "" || p.ParameterGroup != ""
}
// Whether anything lands without an outage, which is also what decides whether
// the record write and its event happen at all: a modify that is entirely
// deferred must not report a configuration change that has not happened.
func (p *modifyPlan) immediate() bool {
return p.MasterUserPassword != "" || p.SecurityGroupIDs != nil || p.DeletionProtection != nil ||
p.BackupRetentionPeriod != nil || p.PreferredBackupWindow != "" || p.PreferredMaintenanceWindow != ""
}
func (p *modifyPlan) empty() bool {
return !p.disruptive() && !p.immediate()
}
// Changes a live DB instance. The non-disruptive settings land immediately; a
// storage grow, a class change or a parameter-group change is applied now when
// ApplyImmediately is set and otherwise recorded in PendingModifiedValues for
// the maintenance window (rds-9) to drain through applyPendingModifications.
//
// The endpoint survives every path: the data volume, the customer ENI and its
// address, and the DNS A-record are untouched by a grow and by a class change.
func (s *Service) ModifyDBInstance(ctx context.Context, input *rds.ModifyDBInstanceInput, accountID string) (*rds.ModifyDBInstanceOutput, error) {
if input == nil {
return nil, fmt.Errorf("%s: empty request", awserrors.ErrorInvalidParameterValue)
}
id := aws.StringValue(input.DBInstanceIdentifier)
if id == "" {
return nil, fmt.Errorf("%s: DBInstanceIdentifier is required", awserrors.ErrorInvalidParameterValue)
}
if err := rejectUnimplementedModify(input); err != nil {
return nil, err
}
kv, err := s.bucket(ctx, accountID)
if err != nil {
return nil, err
}
rec, _, err := s.getDBInstance(ctx, kv, id)
if err != nil {
return nil, err
}
// Resolved against the stored record and fully validated before anything
// moves, so a rejected request never leaves a database stopped.
plan, err := s.planModify(ctx, input, accountID, rec)
if err != nil {
return nil, err
}
if plan.empty() {
return &rds.ModifyDBInstanceOutput{DBInstance: s.projectDBInstance(rec)}, nil
}
// A disruptive change needs a live engine to be stopped cleanly and a live
// agent to apply parameters, so it is legal only from available. The
// non-disruptive settings are record and ENI writes, legal from any settled
// state.
if plan.disruptive() && rec.Status != StatusAvailable && rec.Status != StatusFailed {
return nil, fmt.Errorf("%s: DB instance %s is %s; the requested modification requires it to be %s",
awserrors.ErrorDBInstanceInvalidState, id, rec.Status, StatusAvailable)
}
if err := s.applyImmediateModify(ctx, kv, accountID, rec, plan); err != nil {
return nil, err
}
if !plan.disruptive() {
stored, err := s.reloadInstance(ctx, kv, id)
if err != nil {
return nil, err
}
return &rds.ModifyDBInstanceOutput{DBInstance: s.projectDBInstance(stored)}, nil
}
pending := &PendingModifiedValues{
AllocatedStorage: plan.AllocatedStorage,
DBInstanceClass: plan.InstanceClass,
DBParameterGroupName: plan.ParameterGroup,
RequestedAt: time.Now().UTC(),
}
// Recorded before any of it is attempted, so a leader that dies part-way
// leaves the next one enough to finish rather than an instance stuck in
// modifying with no record of what it was becoming.
if err := s.updateInstance(ctx, kv, id, func(stored *DBInstanceRecord) {
stored.PendingModifiedValues = pending
}); err != nil {
return nil, err
}
rec.PendingModifiedValues = pending
if !plan.ApplyImmediately {
s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, id,
"Modification recorded; it will be applied during the next maintenance window, and the database will be unavailable while it is.",
EventCategoryConfigurationChange, EventCategoryNotification)
return &rds.ModifyDBInstanceOutput{DBInstance: s.projectDBInstance(rec)}, nil
}
moved, kv, err := s.beginTransition(ctx, accountID, id, StatusModifying, StatusAvailable, StatusFailed)
if err != nil {
return nil, err
}
// Under the lease, or the reconciler's sweep of modifying instances re-enters
// this same change while it is still running.
if _, err := s.withModifyLease(ctx, kv, id, func() error {
return s.applyPendingModifications(ctx, kv, accountID, moved)
}); err != nil {
return nil, s.failTransition(ctx, kv, accountID, moved,
fmt.Sprintf("the DB instance could not be modified: %v", err))
}
// Returned as modifying: the replacement or restarted VM has to come back
// and report healthy before the reconciler calls it available.
stored, err := s.reloadInstance(ctx, kv, id)
if err != nil {
return nil, err
}
return &rds.ModifyDBInstanceOutput{DBInstance: s.projectDBInstance(stored)}, nil
}
// The record as stored, for a response that reports what actually landed rather
// than what the handler believed it wrote.
func (s *Service) reloadInstance(ctx context.Context, kv jetstream.KeyValue, id string) (*DBInstanceRecord, error) {
rec, _, err := s.getDBInstance(ctx, kv, id)
return rec, err
}
// Resolves the request against the stored record: drops every field that
// repeats its current value, and rejects everything that cannot be delivered.
func (s *Service) planModify(ctx context.Context, input *rds.ModifyDBInstanceInput, accountID string, rec *DBInstanceRecord) (*modifyPlan, error) {
plan := &modifyPlan{ApplyImmediately: aws.BoolValue(input.ApplyImmediately)}
if password := aws.StringValue(input.MasterUserPassword); password != "" {
if err := ValidateMasterUserPassword(password); err != nil {
return nil, err
}
plan.MasterUserPassword = password
}
if storage := aws.Int64Value(input.AllocatedStorage); storage > 0 && storage != rec.AllocatedStorage {
if err := validateStorageGrow(rec.AllocatedStorage, storage); err != nil {
return nil, err
}
plan.AllocatedStorage = aws.Int64(storage)
}
if class := aws.StringValue(input.DBInstanceClass); class != "" && class != rec.DBInstanceClass {
instanceType, err := InstanceTypeForClass(class)
if err != nil {
return nil, fmt.Errorf("%s: DBInstanceClass %q is not supported; supported classes are %s",
awserrors.ErrorInvalidParameterValue, class, strings.Join(SupportedInstanceClasses(), ", "))
}
plan.InstanceClass, plan.InstanceType = class, instanceType
}
// The implicit default group is the only name that resolves until rds-7
// materialises real ones, so anything else is a group that does not exist.
if group := aws.StringValue(input.DBParameterGroupName); group != "" && group != rec.DBParameterGroupName {
engine, err := LookupEngine(rec.Engine)
if err != nil {
return nil, err
}
if group != engine.DefaultParameterGroupName() {
return nil, fmt.Errorf("%s: DB parameter group %q not found", awserrors.ErrorDBParameterGroupNotFound, group)
}
plan.ParameterGroup = group
}
groups, err := s.planSecurityGroups(ctx, accountID, rec, aws.StringValueSlice(input.VpcSecurityGroupIds))
if err != nil {
return nil, err
}
plan.SecurityGroupIDs = groups
if input.DeletionProtection != nil && aws.BoolValue(input.DeletionProtection) != rec.DeletionProtection {
plan.DeletionProtection = input.DeletionProtection
}
if err := planBackupSettings(input, rec, plan); err != nil {
return nil, err
}
return plan, nil
}
// The security groups to re-associate, or nil when the request names none or
// names the set already attached. Validated against the endpoint ENI's own VPC,
// because an ENI cannot carry a group from another one.
func (s *Service) planSecurityGroups(ctx context.Context, accountID string, rec *DBInstanceRecord, requested []string) ([]string, error) {
if len(requested) == 0 {
return nil, nil
}
if slices.Equal(requested, rec.VpcSecurityGroupIDs) {
return nil, nil
}
if rec.ENIID == "" {
return nil, fmt.Errorf("%s: DB instance %s has no endpoint ENI to re-associate",
awserrors.ErrorDBInstanceInvalidState, rec.DBInstanceIdentifier)
}
// ModifyNetworkInterfaceAttribute validates the groups against the ENI's VPC
// itself, but doing it here keeps the rejection ahead of every other write in
// the modify rather than half-way through it.
if _, err := s.resolveSecurityGroups(ctx, accountID, rec.VpcID, requested); err != nil {
return nil, err
}
return requested, nil
}
// The rds-9 fields. Stored rather than acted on here, but still bounds-checked:
// a value outside AWS's range would fail in a maintenance window nobody is
// watching rather than in the call that set it.
func planBackupSettings(input *rds.ModifyDBInstanceInput, rec *DBInstanceRecord, plan *modifyPlan) error {
if input.BackupRetentionPeriod != nil {
days := aws.Int64Value(input.BackupRetentionPeriod)
if days < 0 || days > maxBackupRetentionDays {
return fmt.Errorf("%s: BackupRetentionPeriod must be between 0 and %d days",
awserrors.ErrorInvalidParameterValue, maxBackupRetentionDays)
}
if days != rec.BackupRetentionPeriod {
plan.BackupRetentionPeriod = aws.Int64(days)
}
}
if window := aws.StringValue(input.PreferredBackupWindow); window != "" && window != rec.PreferredBackupWindow {
plan.PreferredBackupWindow = window
}
if window := aws.StringValue(input.PreferredMaintenanceWindow); window != "" && window != rec.PreferredMaintenanceWindow {
plan.PreferredMaintenanceWindow = window
}
return nil
}
// The half of the plan that never takes the engine down: a live password
// rotation, an ENI security-group re-association, and record settings. Each is
// applied before any disruptive work, so a modify that carries both does not
// lose the cheap half to a failure in the expensive one.
func (s *Service) applyImmediateModify(ctx context.Context, kv jetstream.KeyValue, accountID string, rec *DBInstanceRecord, plan *modifyPlan) error {
if !plan.immediate() {
return nil
}
// Never persisted anywhere: an unreachable agent fails the call rather than
// leaving cleartext queued for a later window (D8).
if plan.MasterUserPassword != "" {
if err := s.setMasterPassword(ctx, accountID, rec.DBInstanceIdentifier, rec.MasterUsername, plan.MasterUserPassword); err != nil {
return fmt.Errorf("%s: the master password could not be applied: %w", awserrors.ErrorDBInstanceInvalidState, err)
}
}
// Done before the record write so a rejected group leaves the record still
// naming the groups the ENI actually carries.
if len(plan.SecurityGroupIDs) > 0 {
if err := s.reassociateSecurityGroups(ctx, accountID, rec, plan.SecurityGroupIDs); err != nil {
return err
}
}
now := time.Now().UTC()
if err := s.updateInstance(ctx, kv, rec.DBInstanceIdentifier, func(stored *DBInstanceRecord) {
if plan.MasterUserPassword != "" {
stored.MasterPasswordUpdatedAt = &now
}
if len(plan.SecurityGroupIDs) > 0 {
stored.VpcSecurityGroupIDs = plan.SecurityGroupIDs
}
if plan.DeletionProtection != nil {
stored.DeletionProtection = *plan.DeletionProtection
}
if plan.BackupRetentionPeriod != nil {
stored.BackupRetentionPeriod = *plan.BackupRetentionPeriod
}
if plan.PreferredBackupWindow != "" {
stored.PreferredBackupWindow = plan.PreferredBackupWindow
}
if plan.PreferredMaintenanceWindow != "" {
stored.PreferredMaintenanceWindow = plan.PreferredMaintenanceWindow
}
}); err != nil {
return err
}
s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier,
"DB instance configuration changed.", EventCategoryConfigurationChange)
return nil
}
// Changing a database's ingress is routine day-two work, and the ENI already
// owns the whole job: the groups are validated against the account and the
// ENI's VPC and the OVN binding is republished. No VM replace, no new address,
// so the endpoint the customer connects to does not move.
func (s *Service) reassociateSecurityGroups(ctx context.Context, accountID string, rec *DBInstanceRecord, groups []string) error {
if s.deps.Launch.VPC == nil {
return errors.New("rds: no VPC path configured")
}
if _, err := s.deps.Launch.VPC.ModifyNetworkInterfaceAttribute(ctx, &ec2.ModifyNetworkInterfaceAttributeInput{
NetworkInterfaceId: aws.String(rec.ENIID),
Groups: aws.StringSlice(groups),
}, accountID); err != nil {
return fmt.Errorf("rds: re-associate the security groups of %s: %w", rec.DBInstanceIdentifier, err)
}
slog.InfoContext(ctx, "rds: endpoint security groups re-associated",
"dbInstance", rec.DBInstanceIdentifier, "eniId", rec.ENIID, "groups", groups)
return nil
}
// D19: a supported action carrying an unimplemented parameter must not silently
// drop it. Every rejection below is a parameter whose omission would create a
// false safety, security or availability guarantee; the inert ones —
// AutoMinorVersionUpgrade, CopyTagsToSnapshot, Performance Insights, Enhanced
// Monitoring — are deliberately absent and accepted as no-ops.
func rejectUnimplementedModify(input *rds.ModifyDBInstanceInput) error {
if aws.BoolValue(input.MultiAZ) {
return unimplemented("MultiAZ", "this platform is single-AZ; a standby would not exist")
}
if aws.BoolValue(input.PubliclyAccessible) {
return unimplemented("PubliclyAccessible",
"the endpoint is a private VPC address; a public one would not be reachable")
}
if aws.StringValue(input.NewDBInstanceIdentifier) != "" {
return unimplemented("NewDBInstanceIdentifier",
"the identifier is the endpoint hostname and the certificate's subject; renaming in place is not implemented")
}
if aws.StringValue(input.EngineVersion) != "" {
return unimplemented("EngineVersion", "engine-version upgrade is not implemented")
}
if aws.StringValue(input.Engine) != "" {
return unimplemented("Engine", "an instance cannot change engine")
}
if aws.Int64Value(input.DBPortNumber) > 0 {
return unimplemented("DBPortNumber",
"the port is fixed at create; changing it would break every client and the serving certificate")
}
if aws.StringValue(input.DBSubnetGroupName) != "" {
return unimplemented("DBSubnetGroupName",
"the endpoint ENI is placed at create and moving it would change the address clients resolve")
}
if aws.Int64Value(input.MaxAllocatedStorage) > 0 {
return unimplemented("MaxAllocatedStorage", "storage autoscaling is not implemented")
}
if aws.Int64Value(input.Iops) > 0 {
return unimplemented("Iops", "provisioned IOPS are not implemented; storage is gp3")
}
if aws.Int64Value(input.StorageThroughput) > 0 {
return unimplemented("StorageThroughput", "provisioned throughput is not implemented; storage is gp3")
}
if storageType := aws.StringValue(input.StorageType); storageType != "" && strings.ToLower(storageType) != storageTypeGP3 {
return unimplemented("StorageType", "only gp3 is offered, so no other type can be moved to")
}
if aws.BoolValue(input.ManageMasterUserPassword) || aws.BoolValue(input.RotateMasterUserPassword) {
return unimplemented("ManageMasterUserPassword",
"there is no Secrets Manager integration; supply MasterUserPassword instead")
}
if aws.BoolValue(input.EnableIAMDatabaseAuthentication) {
return unimplemented("EnableIAMDatabaseAuthentication", "IAM database authentication is not implemented")
}
if len(input.DBSecurityGroups) > 0 {
return unimplemented("DBSecurityGroups",
"EC2-Classic security groups are not offered; use VpcSecurityGroupIds")
}
if aws.StringValue(input.CACertificateIdentifier) != "" {
return unimplemented("CACertificateIdentifier",
"the serving certificate is signed by the cluster CA, which is not selectable")
}
if aws.StringValue(input.Domain) != "" || aws.StringValue(input.DomainFqdn) != "" {
return unimplemented("Domain", "Active Directory domain join is not offered")
}
if aws.StringValue(input.OptionGroupName) != "" {
return unimplemented("OptionGroupName", "option groups are not offered")
}
return nil
}
+116
View File
@@ -0,0 +1,116 @@
package handlers_rds
import (
"context"
"crypto/rand"
"fmt"
"log/slog"
"sync"
"time"
"github.com/nats-io/nats.go/jetstream"
)
// Applying PendingModifiedValues is the one control-plane operation with two
// entry points that can fire at once: ApplyImmediately runs it inline in the API
// call for as long as a VM replace takes, and the reconciler sweeps every
// modifying instance every reconcileInterval looking for exactly that record
// shape. Without a lease the sweep re-enters a change still in progress and runs
// a second replaceInstanceVM against the same data volume and the same endpoint
// ENI — two VMs on one datadir, which is the state the replace exists to avoid.
const (
// Short enough that a worker that dies is taken over within a pass or two,
// and long enough for a renewal to be retried several times inside it.
modifyLeaseTTL = 45 * time.Second
modifyLeaseRefresh = 15 * time.Second
)
// Runs apply while holding the instance's modify lease, renewing it for as long
// as the work runs and releasing it whatever the outcome. Reports whether apply
// ran: a lease already held is not an error, it means another worker is inside
// this change and the caller has nothing to do.
func (s *Service) withModifyLease(ctx context.Context, kv jetstream.KeyValue, id string, apply func() error) (bool, error) {
holder := s.newModifyLeaseHolder()
claimed, err := s.claimModifyLease(ctx, kv, id, holder)
if err != nil || !claimed {
return false, err
}
// The release has to outlive a cancelled ctx, or a shutdown mid-modify
// leaves the lease to expire on its own and stalls the takeover it exists
// to enable.
release := context.WithoutCancel(ctx)
renewing, stopRenewing := context.WithCancel(ctx)
var renewals sync.WaitGroup
renewals.Go(func() { s.renewModifyLease(renewing, kv, id, holder) })
defer func() {
stopRenewing()
renewals.Wait()
if _, err := s.updateInstanceIf(release, kv, id, func(rec *DBInstanceRecord) bool {
if rec.ModifyLease == nil || rec.ModifyLease.Holder != holder {
return false
}
rec.ModifyLease = nil
return true
}); err != nil {
slog.WarnContext(ctx, "rds: releasing the modify lease failed; it will expire instead",
"dbInstance", id, "holder", holder, "err", err)
}
}()
return true, apply()
}
// The node plus a per-claim nonce: the API handler and the reconciler run on the
// same node, so the node alone would let each renew the other's lease.
func (s *Service) newModifyLeaseHolder() string {
var nonce [8]byte
// crypto/rand.Read is documented never to fail.
_, _ = rand.Read(nonce[:])
return fmt.Sprintf("%s/%x", s.deps.HolderID, nonce)
}
// Takes the lease unless a live one belongs to someone else. Re-taking our own
// is allowed, so a caller that already holds it is not deadlocked by itself.
func (s *Service) claimModifyLease(ctx context.Context, kv jetstream.KeyValue, id, holder string) (bool, error) {
return s.updateInstanceIf(ctx, kv, id, func(rec *DBInstanceRecord) bool {
if rec.ModifyLease.live() && rec.ModifyLease.Holder != holder {
return false
}
rec.ModifyLease = &ModifyLease{Holder: holder, ExpiresAt: time.Now().UTC().Add(modifyLeaseTTL)}
return true
})
}
// Pushes the expiry out until the work finishes or ctx is cancelled. A renewal
// that finds the lease taken over stops rather than reclaiming it: another
// worker is already inside the change, and two holders is the state this
// prevents.
func (s *Service) renewModifyLease(ctx context.Context, kv jetstream.KeyValue, id, holder string) {
ticker := time.NewTicker(modifyLeaseRefresh)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
held, err := s.updateInstanceIf(ctx, kv, id, func(rec *DBInstanceRecord) bool {
if rec.ModifyLease == nil || rec.ModifyLease.Holder != holder {
return false
}
rec.ModifyLease.ExpiresAt = time.Now().UTC().Add(modifyLeaseTTL)
return true
})
if err != nil {
slog.WarnContext(ctx, "rds: renewing the modify lease failed; retrying",
"dbInstance", id, "holder", holder, "err", err)
continue
}
if !held {
slog.WarnContext(ctx, "rds: the modify lease was taken over while the change was still running",
"dbInstance", id, "holder", holder)
return
}
}
}
}
+142
View File
@@ -0,0 +1,142 @@
package handlers_rds
import (
"errors"
"sync"
"testing"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// A lease held by a worker that is not this one, which is what a modify still
// inside its own API call looks like from a reconcile pass.
func heldLease(remaining time.Duration) *ModifyLease {
return &ModifyLease{Holder: "node-b/deadbeef", ExpiresAt: time.Now().UTC().Add(remaining)}
}
// The failure the lease exists to stop: ApplyImmediately runs the change inline
// for as long as a grow or a replace takes, and the reconciler sweeps every
// modifying instance looking for exactly that record shape. Without the lease
// the sweep runs a second grow — and for a class change, a second
// replaceInstanceVM against the same data volume and endpoint ENI.
func TestModifyDBInstance_HoldsTheLeaseSoAReconcilePassCannotReEnterTheChange(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
// Fired from inside the resize, which is the widest point of the window: the
// VM is down, the volume is being taken, and the record still names the
// change as outstanding. Once, or a re-entered pass would recurse here.
var sweep sync.Once
var swept error
h.storage.onModify = func() {
sweep.Do(func() { swept = h.rec.reconcileOnce(t.Context()) })
}
in := modifyInput()
in.AllocatedStorage, in.ApplyImmediately = aws.Int64(50), aws.Bool(true)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
require.NoError(t, swept)
assert.Len(t, h.storage.modified, 1, "the reconcile pass must not run a second grow")
assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls,
"the reconcile pass must not open a second stop/start cycle")
}
// A pass that cannot take the lease leaves the instance alone entirely: the
// holder is working on it, and the record already says what it is becoming.
func TestReconciler_LeavesAModifyItsHolderIsStillInside(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
rec.ModifyLease = heldLease(modifyLeaseTTL)
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Empty(t, h.storage.modified)
assert.Empty(t, h.cmdr.calls)
stored := h.record(t)
assert.Equal(t, StatusModifying, stored.Status)
assert.Equal(t, rec.ModifyLease.Holder, stored.ModifyLease.Holder, "the holder's lease is left as it is")
}
// Nobody is renewing an abandoned lease, so it expires and the next pass takes
// the change over — which is what makes a leader that died mid-modify
// recoverable rather than terminal.
func TestReconciler_ResumesAModifyWhoseHolderIsGone(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
rec.ModifyLease = heldLease(-time.Second)
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Equal(t, int64(50), h.storage.sizes[testDataVolume])
assert.Equal(t, int64(50), h.record(t).AllocatedStorage)
}
// The lease is released as soon as the work finishes rather than left to time
// out, or the pass that has to finish the grow inside the guest would wait a
// whole TTL for a change that already landed.
func TestApplyPendingModifications_ReleasesTheLeaseWhateverTheOutcome(t *testing.T) {
t.Run("applied", func(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifyingRecord(
&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()}))
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Nil(t, h.record(t).ModifyLease)
})
t.Run("failed", func(t *testing.T) {
h := newModifyHarness(t)
h.storage.modifyErr = errors.New("the volume store is unavailable")
seedInstance(t, h.svc, modifyingRecord(
&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()}))
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Nil(t, h.record(t).ModifyLease, "a failed attempt must not hold the lease against its own retry")
})
}
// A holder that renews but never finishes would otherwise keep the instance in
// modifying forever, since a pass that cannot take the lease does nothing. Past
// the budget it is failed anyway, which is the state a retry is legal from.
func TestReconciler_FailsAModifyWhoseHolderOverrunsTheBudget(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
rec.ModifyLease = heldLease(modifyLeaseTTL)
started := time.Now().UTC().Add(-2 * transitionTimeout)
rec.TransitionStartedAt = &started
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
stored := h.record(t)
assert.Equal(t, StatusFailed, stored.Status)
assert.Contains(t, stored.FailureReason, "still being modified")
require.NotNil(t, stored.PendingModifiedValues, "the request stays recorded so the retry has something to run")
}
// Re-taking a lease we already hold has to be allowed, or a caller that claims
// twice deadlocks against itself.
func TestClaimModifyLease_RefusesAnotherHolderButNotItsOwn(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
kv := h.kv(t)
claimed, err := h.svc.claimModifyLease(t.Context(), kv, testDBID, "node-a/0001")
require.NoError(t, err)
assert.True(t, claimed)
claimed, err = h.svc.claimModifyLease(t.Context(), kv, testDBID, "node-b/0002")
require.NoError(t, err)
assert.False(t, claimed, "a live lease belongs to its holder alone")
claimed, err = h.svc.claimModifyLease(t.Context(), kv, testDBID, "node-a/0001")
require.NoError(t, err)
assert.True(t, claimed)
}
+552
View File
@@ -0,0 +1,552 @@
package handlers_rds
import (
"errors"
"slices"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/rds"
"github.com/mulgadc/spinifex/spinifex/awserrors"
"github.com/mulgadc/spinifex/spinifex/testutil"
"github.com/nats-io/nats.go"
"github.com/nats-io/nats.go/jetstream"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
const (
testDataVolume = "vol-rdsdata01"
testEndpointENI = "eni-cust01"
testDefaultGroup = "default.postgres18"
)
// modifyHarness is a Service wired for day-two work: the launch fakes a replace
// needs, the customer VPC the security groups are validated against, the volume
// store a grow modifies, and a stub agent answering the command channel.
type modifyHarness struct {
svc *Service
rec *Reconciler
nc *nats.Conn
launch *launchHarness
network *fakeNetwork
cmdr *fakeInstanceCommander
storage *fakeVolumeResizer
vmState *fakeInstanceState
agent *stubAgent
}
func newModifyHarness(t *testing.T) *modifyHarness {
return newModifyHarnessWithAgent(t, false)
}
// agentFails makes the stub agent reject every command, which is how a modify
// against a wedged guest is exercised without waiting out a real budget.
func newModifyHarnessWithAgent(t *testing.T, agentFails bool) *modifyHarness {
t.Helper()
_, nc, _ := testutil.StartTestJetStream(t)
h := &modifyHarness{
nc: nc,
launch: newLaunchHarness(),
network: newFakeNetwork(),
cmdr: &fakeInstanceCommander{},
storage: newFakeVolumeResizer(testDataVolume, 20),
vmState: &fakeInstanceState{state: instanceStateRunning},
}
h.agent = newStubAgent(t, nc, testAccountID, testDBID, agentFails)
h.svc = NewService(nc, testRegion).WithDeps(Deps{
LoadCA: newTestCA(t),
Launch: h.launch.deps(),
Network: h.network,
Instances: h.cmdr,
Storage: h.storage,
InstanceState: h.vmState,
})
h.rec = NewReconciler(h.svc, "node-a")
return h
}
func (h *modifyHarness) record(t *testing.T) DBInstanceRecord {
t.Helper()
kv, err := h.svc.bucket(t.Context(), testAccountID)
require.NoError(t, err)
var rec DBInstanceRecord
found, err := getJSON(t.Context(), kv, DBInstanceKey(testDBID), &rec)
require.NoError(t, err)
require.True(t, found)
return rec
}
func (h *modifyHarness) kv(t *testing.T) jetstream.KeyValue {
t.Helper()
kv, err := h.svc.bucket(t.Context(), testAccountID)
require.NoError(t, err)
return kv
}
// An available instance with everything a modify reads: the placement its
// security groups are checked against, the endpoint ENI they are re-associated
// on, and the data volume a grow resizes.
func modifiableRecord() DBInstanceRecord {
rec := availableRecord()
rec.DBInstanceClass = "db.t3.medium"
rec.AllocatedStorage = 20
rec.StorageType = storageTypeGP3
rec.VpcID = testDefaultVPC
rec.SubnetID = testDBSubnet
rec.VpcSecurityGroupIDs = []string{testDefaultSG}
rec.DBParameterGroupName = testDefaultGroup
rec.ENIID = testEndpointENI
rec.DataVolumeID = testDataVolume
return rec
}
func modifyInput() *rds.ModifyDBInstanceInput {
return &rds.ModifyDBInstanceInput{DBInstanceIdentifier: aws.String(testDBID)}
}
// None of these interrupts service, so AWS applies them as soon as possible and
// so does this — ApplyImmediately is not what gates them.
func TestModifyDBInstance_AppliesTheNonDisruptiveSettingsAtOnce(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.MasterUserPassword = aws.String("N3w-Sup3rSecret!")
in.DeletionProtection = aws.Bool(true)
in.BackupRetentionPeriod = aws.Int64(7)
in.PreferredBackupWindow = aws.String("03:00-04:00")
in.PreferredMaintenanceWindow = aws.String("sun:05:00-sun:06:00")
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
// Still available: nothing here takes the engine down, so the instance never
// leaves the state a client can connect to.
assert.Equal(t, string(StatusAvailable), aws.StringValue(out.DBInstance.DBInstanceStatus))
assert.Empty(t, h.cmdr.calls, "a non-disruptive modify does not touch the VM's power state")
issued := h.agent.received()
require.Len(t, issued, 1, "the password is applied live over the command channel")
assert.Equal(t, CommandSetPassword, issued[0].Type)
rec := h.record(t)
assert.True(t, rec.DeletionProtection)
assert.Equal(t, int64(7), rec.BackupRetentionPeriod)
assert.Equal(t, "03:00-04:00", rec.PreferredBackupWindow)
assert.Equal(t, "sun:05:00-sun:06:00", rec.PreferredMaintenanceWindow)
assert.NotNil(t, rec.MasterPasswordUpdatedAt)
assert.Nil(t, rec.PendingModifiedValues)
}
// D8: only the fact of the rotation is recorded. A password that reached KV
// would be readable by anything that can read the bucket, forever.
func TestModifyDBInstance_NeverPersistsTheRotatedPassword(t *testing.T) {
h := newModifyHarness(t)
rec := modifiableRecord()
rec.Bootstrap.MasterUserPassword = ""
seedInstance(t, h.svc, rec)
in := modifyInput()
in.MasterUserPassword = aws.String("N3w-Sup3rSecret!")
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
_, raw := readRecord(t, h.svc)
assert.NotContains(t, raw, "N3w-Sup3rSecret!", "the rotated password must not be at rest anywhere")
}
// An agent that cannot be reached fails the call: a password change that is
// reported as applied but never reached the engine locks the customer out of
// their own database with no signal.
func TestModifyDBInstance_FailsWhenThePasswordCannotBeApplied(t *testing.T) {
h := newModifyHarnessWithAgent(t, true)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.MasterUserPassword = aws.String("N3w-Sup3rSecret!")
in.DeletionProtection = aws.Bool(true)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
// The rest of the request is not applied either: the record would otherwise
// report a change the caller was told had failed.
rec := h.record(t)
assert.False(t, rec.DeletionProtection)
assert.Nil(t, rec.MasterPasswordUpdatedAt)
}
// Changing a database's ingress is a live ENI re-association: no replace, no
// new address, so the endpoint clients resolve does not move.
func TestModifyDBInstance_ReassociatesTheSecurityGroupsOnTheEndpointENI(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.VpcSecurityGroupIds = aws.StringSlice([]string{"sg-app01"})
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
require.Len(t, h.launch.enis.modified, 1)
assert.Equal(t, testEndpointENI, aws.StringValue(h.launch.enis.modified[0].NetworkInterfaceId))
assert.Equal(t, []string{"sg-app01"}, aws.StringValueSlice(h.launch.enis.modified[0].Groups))
assert.Empty(t, h.launch.enis.created, "the endpoint ENI is re-associated, not replaced")
assert.Equal(t, []string{"sg-app01"}, h.record(t).VpcSecurityGroupIDs)
}
// An ENI cannot carry a group from another VPC, so the launch-time rule holds
// at modify too — and rejecting it here keeps the failure ahead of every write.
func TestModifyDBInstance_RejectsASecurityGroupFromAnotherVPC(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.VpcSecurityGroupIds = aws.StringSlice([]string{"sg-elsewhere"})
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
assert.Empty(t, h.launch.enis.modified)
assert.Equal(t, []string{testDefaultSG}, h.record(t).VpcSecurityGroupIDs)
}
// Re-sending the groups already attached is what Terraform does on every apply.
// It is not a change, so it must not reach the ENI at all.
func TestModifyDBInstance_IgnoresTheSecurityGroupsAlreadyAttached(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.VpcSecurityGroupIds = aws.StringSlice([]string{testDefaultSG})
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.Empty(t, h.launch.enis.modified)
}
// D12: storage is grow-only, and a request to shrink must be refused before the
// instance moves anywhere — a rejected request cannot cost a running database
// its availability.
func TestModifyDBInstance_RejectsAStorageShrinkBeforeAnythingMoves(t *testing.T) {
h := newModifyHarness(t)
rec := modifiableRecord()
rec.AllocatedStorage = 100
seedInstance(t, h.svc, rec)
in := modifyInput()
in.AllocatedStorage = aws.Int64(50)
in.ApplyImmediately = aws.Bool(true)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterCombination)
assert.Empty(t, h.cmdr.calls)
assert.Empty(t, h.storage.modified)
stored := h.record(t)
assert.Equal(t, StatusAvailable, stored.Status)
assert.Equal(t, int64(100), stored.AllocatedStorage)
}
// Terraform sends the whole body on every apply, so a size that repeats the
// current one accompanies unrelated changes constantly. Failing it would fail
// every one of them; it contributes nothing to the plan instead.
func TestModifyDBInstance_TreatsTheCurrentStorageSizeAsNoChange(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.AllocatedStorage = aws.Int64(20)
in.DBInstanceClass = aws.String("db.t3.medium")
in.DeletionProtection = aws.Bool(true)
in.ApplyImmediately = aws.Bool(true)
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.Equal(t, string(StatusAvailable), aws.StringValue(out.DBInstance.DBInstanceStatus))
assert.Empty(t, h.cmdr.calls, "repeating the current size and class is not an outage")
assert.Empty(t, h.storage.modified)
stored := h.record(t)
assert.True(t, stored.DeletionProtection, "the change that was real still landed")
assert.Nil(t, stored.PendingModifiedValues)
}
// A request that changes nothing at all is answered with the instance as it
// stands rather than an outage or an error.
func TestModifyDBInstance_IsANoOpWhenNothingDiffers(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.DBInstanceClass = aws.String("db.t3.medium")
in.ApplyImmediately = aws.Bool(true)
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.Equal(t, string(StatusAvailable), aws.StringValue(out.DBInstance.DBInstanceStatus))
assert.Empty(t, h.launch.launcher.terminated)
assert.Nil(t, h.record(t).PendingModifiedValues)
}
// D15: the db.* classes are a facade over the platform's instance types, so a
// class with nothing behind it is rejected with the set that has.
func TestModifyDBInstance_RejectsAnUnmappedInstanceClass(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.DBInstanceClass = aws.String("db.r5.24xlarge")
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
assert.Contains(t, err.Error(), "db.t3.medium", "the rejection names the classes that do resolve")
assert.Equal(t, StatusAvailable, h.record(t).Status)
}
// Until rds-7 materialises real groups the implicit default is the only name
// that resolves, so any other one names a group that does not exist.
func TestModifyDBInstance_RejectsAnUnknownParameterGroup(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.DBParameterGroupName = aws.String("tuned-for-oltp")
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorDBParameterGroupNotFound)
assert.Nil(t, h.record(t).PendingModifiedValues)
}
// The rds-9 fields are stored rather than acted on, but a value outside AWS's
// range would fail in a maintenance window nobody is watching.
func TestModifyDBInstance_RejectsAnOutOfRangeBackupRetention(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.BackupRetentionPeriod = aws.Int64(400)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
assert.Zero(t, h.record(t).BackupRetentionPeriod)
}
// D19: a supported action carrying a parameter this platform does not implement
// is rejected, never served with the parameter quietly dropped. Each of these
// would otherwise leave the customer believing in a guarantee they do not have.
func TestModifyDBInstance_RejectsTheUnimplementedParameters(t *testing.T) {
cases := map[string]func(*rds.ModifyDBInstanceInput){
"MultiAZ": func(in *rds.ModifyDBInstanceInput) { in.MultiAZ = aws.Bool(true) },
"PubliclyAccessible": func(in *rds.ModifyDBInstanceInput) { in.PubliclyAccessible = aws.Bool(true) },
"NewDBInstanceIdentifier": func(in *rds.ModifyDBInstanceInput) { in.NewDBInstanceIdentifier = aws.String("renamed") },
"EngineVersion": func(in *rds.ModifyDBInstanceInput) { in.EngineVersion = aws.String("19.1") },
"Engine": func(in *rds.ModifyDBInstanceInput) { in.Engine = aws.String("mysql") },
"DBPortNumber": func(in *rds.ModifyDBInstanceInput) { in.DBPortNumber = aws.Int64(6543) },
"DBSubnetGroupName": func(in *rds.ModifyDBInstanceInput) { in.DBSubnetGroupName = aws.String("other-subnets") },
"MaxAllocatedStorage": func(in *rds.ModifyDBInstanceInput) { in.MaxAllocatedStorage = aws.Int64(500) },
"Iops": func(in *rds.ModifyDBInstanceInput) { in.Iops = aws.Int64(3000) },
"StorageThroughput": func(in *rds.ModifyDBInstanceInput) { in.StorageThroughput = aws.Int64(250) },
"StorageType": func(in *rds.ModifyDBInstanceInput) { in.StorageType = aws.String("io2") },
"ManageMasterUserPassword": func(in *rds.ModifyDBInstanceInput) { in.ManageMasterUserPassword = aws.Bool(true) },
"EnableIAMDatabaseAuthentication": func(in *rds.ModifyDBInstanceInput) { in.EnableIAMDatabaseAuthentication = aws.Bool(true) },
"DBSecurityGroups": func(in *rds.ModifyDBInstanceInput) { in.DBSecurityGroups = aws.StringSlice([]string{"classic-sg"}) },
"CACertificateIdentifier": func(in *rds.ModifyDBInstanceInput) { in.CACertificateIdentifier = aws.String("rds-ca-2019") },
"Domain": func(in *rds.ModifyDBInstanceInput) { in.Domain = aws.String("d-9876543210") },
"OptionGroupName": func(in *rds.ModifyDBInstanceInput) { in.OptionGroupName = aws.String("custom-options") },
}
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
for name, mutate := range cases {
t.Run(name, func(t *testing.T) {
in := modifyInput()
mutate(in)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err, "%s must be rejected rather than silently dropped", name)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
assert.Equal(t, StatusAvailable, h.record(t).Status)
})
}
}
// gp3 is the only type offered, so naming it is not a change and must not be
// caught by the rejection that guards the types that are not offered.
func TestModifyDBInstance_AcceptsTheStorageTypeItAlreadyHas(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.StorageType = aws.String("GP3")
in.DeletionProtection = aws.Bool(true)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.True(t, h.record(t).DeletionProtection)
}
// A disruptive change without ApplyImmediately is recorded and left for rds-9's
// maintenance window: the database keeps serving until then.
func TestModifyDBInstance_DefersADisruptiveChangeToTheMaintenanceWindow(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.AllocatedStorage = aws.Int64(50)
in.DBInstanceClass = aws.String("db.m5.large")
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.Equal(t, string(StatusAvailable), aws.StringValue(out.DBInstance.DBInstanceStatus))
assert.Empty(t, h.cmdr.calls)
assert.Empty(t, h.storage.modified)
assert.Empty(t, h.launch.launcher.terminated)
rec := h.record(t)
require.NotNil(t, rec.PendingModifiedValues)
assert.Equal(t, int64(50), aws.Int64Value(rec.PendingModifiedValues.AllocatedStorage))
assert.Equal(t, "db.m5.large", rec.PendingModifiedValues.DBInstanceClass)
// Not yet in effect: reporting the new values as current would tell the
// customer a change that has not happened has.
assert.Equal(t, int64(20), rec.AllocatedStorage)
assert.Equal(t, "db.t3.medium", rec.DBInstanceClass)
// Reported to the customer, since the change they asked for is not the one
// they got: it lands later, with an outage.
require.NotEmpty(t, h.eventMessages(t))
assert.Contains(t, h.eventMessages(t)[0], "maintenance window")
}
// A disruptive change needs a live engine to stop cleanly and a live agent to
// apply parameters, so it is legal only from a state that has both.
func TestModifyDBInstance_RejectsADisruptiveChangeWhileNotAvailable(t *testing.T) {
h := newModifyHarness(t)
rec := modifiableRecord()
rec.Status = StatusStopped
seedInstance(t, h.svc, rec)
in := modifyInput()
in.AllocatedStorage = aws.Int64(50)
in.ApplyImmediately = aws.Bool(true)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorDBInstanceInvalidState)
assert.Equal(t, StatusStopped, h.record(t).Status)
assert.Nil(t, h.record(t).PendingModifiedValues)
}
// A failed instance is exactly the one a customer retries a change on, so the
// same change is accepted from there.
func TestModifyDBInstance_AcceptsARetryFromFailed(t *testing.T) {
h := newModifyHarness(t)
rec := modifiableRecord()
rec.Status = StatusFailed
rec.FailureReason = "the DB instance could not be modified"
seedInstance(t, h.svc, rec)
in := modifyInput()
in.AllocatedStorage = aws.Int64(50)
in.ApplyImmediately = aws.Bool(true)
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.Equal(t, string(StatusModifying), aws.StringValue(out.DBInstance.DBInstanceStatus))
}
// The whole immediate path: the engine is stopped cleanly, the VM goes down,
// the volume grows, the VM comes back, and the instance stays in modifying with
// the in-guest filesystem grow still outstanding.
func TestModifyDBInstance_AppliesAStorageGrowImmediately(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
in := modifyInput()
in.AllocatedStorage = aws.Int64(50)
in.ApplyImmediately = aws.Bool(true)
out, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.NoError(t, err)
assert.Equal(t, string(StatusModifying), aws.StringValue(out.DBInstance.DBInstanceStatus))
assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls)
require.Len(t, h.storage.modified, 1)
assert.Equal(t, int64(50), aws.Int64Value(h.storage.modified[0].Size))
issued := h.agent.received()
require.Len(t, issued, 1, "the engine is checkpointed before its VM stops")
assert.Equal(t, CommandStopEngine, issued[0].Type)
rec := h.record(t)
assert.Equal(t, int64(50), rec.AllocatedStorage)
assert.Equal(t, testInstance, rec.InstanceID, "a grow restarts the VM rather than replacing it")
require.NotNil(t, rec.PendingModifiedValues)
assert.True(t, rec.PendingModifiedValues.FilesystemGrowPending)
assert.Nil(t, rec.PendingModifiedValues.AllocatedStorage, "the volume is at its new size")
}
// A change that cannot be delivered leaves the instance failed with the reason
// and the request still recorded, so the reconciler or the customer can retry
// it rather than having to reconstruct what was asked for.
func TestModifyDBInstance_FailsTheInstanceAndKeepsThePendingValues(t *testing.T) {
h := newModifyHarness(t)
seedInstance(t, h.svc, modifiableRecord())
h.storage.modifyErr = errors.New("the volume store rejected the resize")
in := modifyInput()
in.AllocatedStorage = aws.Int64(50)
in.ApplyImmediately = aws.Bool(true)
_, err := h.svc.ModifyDBInstance(t.Context(), in, testAccountID)
require.Error(t, err)
rec := h.record(t)
assert.Equal(t, StatusFailed, rec.Status)
assert.Contains(t, rec.FailureReason, "could not be modified")
require.NotNil(t, rec.PendingModifiedValues)
assert.Equal(t, int64(50), aws.Int64Value(rec.PendingModifiedValues.AllocatedStorage))
assert.Equal(t, int64(20), rec.AllocatedStorage, "the record reports the size the volume actually has")
}
func TestModifyDBInstance_RequiresAnIdentifier(t *testing.T) {
h := newModifyHarness(t)
_, err := h.svc.ModifyDBInstance(t.Context(), &rds.ModifyDBInstanceInput{}, testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorInvalidParameterValue)
_, err = h.svc.ModifyDBInstance(t.Context(), nil, testAccountID)
require.Error(t, err)
}
func TestModifyDBInstance_RejectsAnUnknownInstance(t *testing.T) {
h := newModifyHarness(t)
_, err := h.svc.ModifyDBInstance(t.Context(), modifyInput(), testAccountID)
require.Error(t, err)
assert.Contains(t, err.Error(), awserrors.ErrorDBInstanceNotFound)
}
// The event ring's messages, newest first, which is where a deferred change is
// reported to the customer.
func (h *modifyHarness) eventMessages(t *testing.T) []string {
t.Helper()
kv, err := h.svc.bucket(t.Context(), testAccountID)
require.NoError(t, err)
var ring eventRing
found, err := getJSON(t.Context(), kv, EventRingKey(EventSourceTypeDBInstance, testDBID), &ring)
require.NoError(t, err)
if !found {
return nil
}
messages := make([]string, 0, len(ring.Events))
for _, event := range slices.Backward(ring.Events) {
messages = append(messages, event.Message)
}
return messages
}
+128
View File
@@ -0,0 +1,128 @@
package handlers_rds
import (
"context"
"fmt"
"log/slog"
"github.com/aws/aws-sdk-go/aws"
"github.com/nats-io/nats.go/jetstream"
)
// The single drain for everything a modify recorded but has not delivered. Both
// the ApplyImmediately path and the reconciler's resume of an interrupted
// modify come through here, so a deferred change is applied by exactly the code
// an immediate one uses — the failure mode this closes is a maintenance-window
// modify that quietly does something different from the one the customer
// watched happen.
//
// rds-9 owns the trigger: its window machinery — parsing, deterministic
// assignment, a persisted last-fired stamp, exactly-once firing across leader
// churn — is the same mechanism a maintenance window needs, so it is built once
// there and calls this.
//
// The caller has already moved the instance into modifying; the record is left
// there, because the engine has to come back and report healthy before the
// reconciler calls it available.
func (s *Service) applyPendingModifications(ctx context.Context, kv jetstream.KeyValue, accountID string, rec *DBInstanceRecord) error {
pending := rec.PendingModifiedValues
if pending.empty() {
return nil
}
// Parameters first, while the engine this modify started against is still
// the one running: the disruptive step below restarts it, which is also what
// puts any statically-scoped setting into effect.
if pending.DBParameterGroupName != "" {
if err := s.applyParameterGroup(ctx, kv, accountID, rec, pending.DBParameterGroupName); err != nil {
return err
}
}
// A class change and a storage grow both take the engine down, so they share
// one outage: the volume is grown in the window between the old VM being
// terminated and the new one launching, which is the only window ModifyVolume
// will accept it in.
grewStorage := false
switch {
case pending.DBInstanceClass != "":
instanceType, err := InstanceTypeForClass(pending.DBInstanceClass)
if err != nil {
return fmt.Errorf("rds: DBInstanceClass %q is not supported", pending.DBInstanceClass)
}
if err := s.replaceInstanceVM(ctx, kv, accountID, rec, replaceInput{
InstanceClass: pending.DBInstanceClass,
InstanceType: instanceType,
GrowStorageToGiB: aws.Int64Value(pending.AllocatedStorage),
Reason: "the instance class changed to " + pending.DBInstanceClass,
}); err != nil {
return err
}
grewStorage = pending.AllocatedStorage != nil
case pending.AllocatedStorage != nil:
if err := s.growInstanceStorage(ctx, accountID, rec, *pending.AllocatedStorage); err != nil {
return err
}
grewStorage = true
}
applied := *pending
return s.updateInstance(ctx, kv, rec.DBInstanceIdentifier, func(stored *DBInstanceRecord) {
if applied.DBInstanceClass != "" {
stored.DBInstanceClass = applied.DBInstanceClass
}
if applied.AllocatedStorage != nil {
stored.AllocatedStorage = *applied.AllocatedStorage
}
if applied.DBParameterGroupName != "" {
stored.DBParameterGroupName = applied.DBParameterGroupName
}
// The volume is at its new size but the guest's filesystem is not yet on
// it, so the last step stays outstanding until the agent is back to run
// it. Everything else is now in effect.
if grewStorage || applied.FilesystemGrowPending {
stored.PendingModifiedValues = &PendingModifiedValues{
FilesystemGrowPending: true,
RequestedAt: applied.RequestedAt,
}
return
}
stored.PendingModifiedValues = nil
})
}
// Installs the resolved parameter set into the engine's config and reloads it,
// recording the settings the engine accepted but will not honour until it
// restarts (D16). The set lives on the data volume, so it survives the VM
// replace a class change performs and needs no second apply afterwards.
func (s *Service) applyParameterGroup(ctx context.Context, kv jetstream.KeyValue, accountID string, rec *DBInstanceRecord, group string) error {
pendingReboot, err := s.applyParameters(ctx, accountID, rec.DBInstanceIdentifier, rec.Bootstrap.ResolvedParameters)
if err != nil {
return fmt.Errorf("apply the parameters of %s to %s: %w", group, rec.DBInstanceIdentifier, err)
}
return s.updateInstance(ctx, kv, rec.DBInstanceIdentifier, func(stored *DBInstanceRecord) {
stored.PendingRebootParameters = pendingReboot
})
}
// The last step of a grow, run once the restarted or replaced agent is back:
// the control plane has already grown the volume, and this extends the guest's
// filesystem onto the capacity that is now there. Both ext4 and XFS grow while
// mounted, so it needs no ordering against the engine start.
func (s *Service) finishFilesystemGrow(ctx context.Context, kv jetstream.KeyValue, accountID string, rec *DBInstanceRecord) error {
if err := s.growFilesystem(ctx, accountID, rec.DBInstanceIdentifier); err != nil {
return fmt.Errorf("extend the filesystem of %s onto its grown volume: %w", rec.DBInstanceIdentifier, err)
}
if err := s.updateInstance(ctx, kv, rec.DBInstanceIdentifier, func(stored *DBInstanceRecord) {
stored.PendingModifiedValues = nil
}); err != nil {
return err
}
rec.PendingModifiedValues = nil
slog.InfoContext(ctx, "rds: filesystem extended onto the grown data volume",
"dbInstance", rec.DBInstanceIdentifier, "allocatedStorage", rec.AllocatedStorage)
s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier,
"DB instance storage grown.", EventCategoryConfigurationChange)
return nil
}
+263
View File
@@ -0,0 +1,263 @@
package handlers_rds
import (
"errors"
"testing"
"time"
"github.com/aws/aws-sdk-go/aws"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// A record mid-modify: in modifying with the change recorded, which is the
// state both the immediate path and a resumed one drain from.
func modifyingRecord(pending *PendingModifiedValues) DBInstanceRecord {
rec := modifiableRecord()
rec.Status = StatusModifying
started := time.Now().UTC().Add(-time.Minute)
rec.TransitionStartedAt = &started
rec.PendingModifiedValues = pending
return rec
}
// A grow with no class change alongside it keeps the VM: it goes down, the
// volume grows, and the same VM comes back up on the same datadir.
func TestApplyPendingModifications_GrowsStorageOnTheSameVM(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
seedInstance(t, h.svc, rec)
require.NoError(t, h.svc.applyPendingModifications(t.Context(), h.kv(t), testAccountID, &rec))
assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls)
assert.Empty(t, h.launch.launcher.terminated, "a grow does not replace the VM")
assert.Equal(t, int64(50), h.storage.sizes[testDataVolume])
stored := h.record(t)
assert.Equal(t, int64(50), stored.AllocatedStorage)
assert.Equal(t, testInstance, stored.InstanceID)
// The volume is at its new size but the guest's filesystem is not yet on it,
// so exactly one step is left outstanding.
require.NotNil(t, stored.PendingModifiedValues)
assert.True(t, stored.PendingModifiedValues.FilesystemGrowPending)
assert.Nil(t, stored.PendingModifiedValues.AllocatedStorage)
}
// D15: a class change is a VM replace, and a grow asked for alongside it rides
// the same outage rather than opening a second one.
func TestApplyPendingModifications_ClassChangeReplacesTheVMAndCarriesTheGrow(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{
DBInstanceClass: "db.m5.large",
AllocatedStorage: aws.Int64(80),
RequestedAt: time.Now().UTC(),
})
seedReplaceable(t, h, rec)
require.NoError(t, h.svc.applyPendingModifications(t.Context(), h.kv(t), testAccountID, &rec))
require.NotNil(t, h.launch.launcher.input)
assert.Equal(t, "m5.large", h.launch.launcher.input.InstanceType)
assert.Equal(t, []string{testInstance}, h.launch.launcher.terminated)
assert.Empty(t, h.cmdr.calls, "a replace terminates the VM rather than power-cycling it")
assert.Equal(t, int64(80), h.storage.sizes[testDataVolume])
stored := h.record(t)
assert.Equal(t, "db.m5.large", stored.DBInstanceClass)
assert.Equal(t, int64(80), stored.AllocatedStorage)
assert.Equal(t, testReplacementInstance, stored.InstanceID)
require.NotNil(t, stored.PendingModifiedValues)
assert.True(t, stored.PendingModifiedValues.FilesystemGrowPending)
}
// D16: the parameters go in while the engine this modify started against is
// still the one running, so the restart that follows is what adopts the
// statically-scoped ones.
func TestApplyPendingModifications_AppliesTheParametersBeforeTheOutage(t *testing.T) {
h := newModifyHarness(t)
h.agent.replyWith("shared_buffers")
rec := modifyingRecord(&PendingModifiedValues{
DBParameterGroupName: testDefaultGroup,
AllocatedStorage: aws.Int64(50),
RequestedAt: time.Now().UTC(),
})
rec.DBParameterGroupName = ""
seedInstance(t, h.svc, rec)
require.NoError(t, h.svc.applyPendingModifications(t.Context(), h.kv(t), testAccountID, &rec))
issued := h.agent.received()
require.Len(t, issued, 2)
assert.Equal(t, CommandApplyParams, issued[0].Type, "the parameters go in before the engine is stopped")
assert.Equal(t, CommandStopEngine, issued[1].Type)
stored := h.record(t)
assert.Equal(t, testDefaultGroup, stored.DBParameterGroupName)
assert.Equal(t, []string{"shared_buffers"}, stored.PendingRebootParameters)
}
// A parameter apply the agent rejects stops the modify before the outage: the
// customer asked for one change, and delivering half of it silently is the
// failure mode this closes.
func TestApplyPendingModifications_KeepsThePendingValuesWhenTheChangeFails(t *testing.T) {
h := newModifyHarness(t)
h.storage.modifyErr = errors.New("the volume store is unavailable")
pending := &PendingModifiedValues{
DBParameterGroupName: testDefaultGroup,
AllocatedStorage: aws.Int64(50),
RequestedAt: time.Now().UTC(),
}
rec := modifyingRecord(pending)
seedInstance(t, h.svc, rec)
require.Error(t, h.svc.applyPendingModifications(t.Context(), h.kv(t), testAccountID, &rec))
// The whole request is still recorded, including the half that landed: a
// resumed drain re-applies the parameters, which is idempotent, rather than
// losing the grow it never reached.
stored := h.record(t)
require.NotNil(t, stored.PendingModifiedValues)
assert.Equal(t, int64(50), aws.Int64Value(stored.PendingModifiedValues.AllocatedStorage))
assert.Equal(t, testDefaultGroup, stored.PendingModifiedValues.DBParameterGroupName)
assert.Equal(t, int64(20), stored.AllocatedStorage, "the record reports the size the volume actually has")
}
// Nothing outstanding is not an error: a record whose values have already
// landed is drained again by any resumed pass.
func TestApplyPendingModifications_IsANoOpWithNothingOutstanding(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(nil)
seedInstance(t, h.svc, rec)
require.NoError(t, h.svc.applyPendingModifications(t.Context(), h.kv(t), testAccountID, &rec))
assert.Empty(t, h.cmdr.calls)
assert.Empty(t, h.storage.modified)
}
// The last step of a grow: the control plane has already grown the volume, and
// this is what turns it into capacity the database can use.
func TestFinishFilesystemGrow_ExtendsTheGuestAndClearsThePending(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{FilesystemGrowPending: true, RequestedAt: time.Now().UTC()})
rec.AllocatedStorage = 50
seedInstance(t, h.svc, rec)
require.NoError(t, h.svc.finishFilesystemGrow(t.Context(), h.kv(t), testAccountID, &rec))
issued := h.agent.received()
require.Len(t, issued, 1)
assert.Equal(t, CommandGrowFilesystem, issued[0].Type)
assert.Nil(t, h.record(t).PendingModifiedValues)
assert.Nil(t, rec.PendingModifiedValues)
messages := h.eventMessages(t)
require.NotEmpty(t, messages)
assert.Contains(t, messages[0], "storage grown")
}
// A guest that cannot extend its filesystem keeps the step outstanding: the
// volume is bigger and the database cannot use it, which is exactly the state
// the reconciler has to keep retrying.
func TestFinishFilesystemGrow_KeepsTheStepWhenTheGuestFails(t *testing.T) {
h := newModifyHarnessWithAgent(t, true)
rec := modifyingRecord(&PendingModifiedValues{FilesystemGrowPending: true, RequestedAt: time.Now().UTC()})
seedInstance(t, h.svc, rec)
require.Error(t, h.svc.finishFilesystemGrow(t.Context(), h.kv(t), testAccountID, &rec))
stored := h.record(t)
require.NotNil(t, stored.PendingModifiedValues)
assert.True(t, stored.PendingModifiedValues.FilesystemGrowPending)
}
// A leader that died mid-modify leaves the change recorded and nothing else, so
// the next pass re-runs it rather than waiting on a VM that was never touched.
func TestReconciler_ResumesAnInterruptedModify(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Equal(t, int64(50), h.storage.sizes[testDataVolume])
stored := h.record(t)
assert.Equal(t, StatusModifying, stored.Status, "the engine still has to come back before it is available")
assert.Equal(t, int64(50), stored.AllocatedStorage)
require.NotNil(t, stored.PendingModifiedValues)
assert.True(t, stored.PendingModifiedValues.FilesystemGrowPending)
}
// The in-guest grow can only run once the agent is back, so the reconciler is
// what issues it — a customer's modify finishes without them watching.
func TestReconciler_FinishesTheFilesystemGrowOnceTheAgentIsBack(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(&PendingModifiedValues{FilesystemGrowPending: true, RequestedAt: time.Now().UTC()})
beat := time.Now().UTC()
rec.Agent = AgentState{InstanceID: testInstance, EngineHealth: EngineHealthHealthy, LastSeen: &beat}
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
issued := h.agent.received()
require.Len(t, issued, 1)
assert.Equal(t, CommandGrowFilesystem, issued[0].Type)
assert.Nil(t, h.record(t).PendingModifiedValues)
// The record moved under the revision that pass read, so the transition to
// available is the next pass's rather than a raced write.
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Equal(t, StatusAvailable, h.record(t).Status)
}
// A modify whose VM never comes back cannot sit in modifying forever: the
// instance is failed with the reason, which is a state a retry is legal from.
func TestReconciler_FailsAModifyThatOverrunsItsBudget(t *testing.T) {
h := newModifyHarness(t)
rec := modifyingRecord(nil)
started := time.Now().UTC().Add(-2 * transitionTimeout)
rec.TransitionStartedAt = &started
rec.Agent = AgentState{}
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
stored := h.record(t)
assert.Equal(t, StatusFailed, stored.Status)
assert.Contains(t, stored.FailureReason, "did not report healthy")
}
// A modify that cannot be applied at all is failed rather than retried
// forever, and the request stays recorded so the retry has something to run.
func TestReconciler_FailsAModifyItCannotApplyWithinTheBudget(t *testing.T) {
h := newModifyHarness(t)
h.storage.modifyErr = errors.New("the volume store is unavailable")
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
started := time.Now().UTC().Add(-2 * transitionTimeout)
rec.TransitionStartedAt = &started
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
stored := h.record(t)
assert.Equal(t, StatusFailed, stored.Status)
assert.Contains(t, stored.FailureReason, "could not be modified")
require.NotNil(t, stored.PendingModifiedValues)
assert.Equal(t, int64(50), aws.Int64Value(stored.PendingModifiedValues.AllocatedStorage))
}
// Inside the budget a failed attempt is retried on the next pass rather than
// failing the instance on the first transient error.
func TestReconciler_RetriesAFailedModifyInsideTheBudget(t *testing.T) {
h := newModifyHarness(t)
h.storage.modifyErr = errors.New("the volume store is briefly unavailable")
rec := modifyingRecord(&PendingModifiedValues{AllocatedStorage: aws.Int64(50), RequestedAt: time.Now().UTC()})
seedInstance(t, h.svc, rec)
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Equal(t, StatusModifying, h.record(t).Status)
h.storage.modifyErr = nil
require.NoError(t, h.rec.reconcileOnce(t.Context()))
assert.Equal(t, int64(50), h.record(t).AllocatedStorage)
}
+85
View File
@@ -213,6 +213,8 @@ func (r *Reconciler) reconcileInstance(ctx context.Context, kv jetstream.KeyValu
return r.reconcileCreating(ctx, kv, rev, accountID, &rec)
case StatusRebooting, StatusStarting:
return r.reconcileRestarting(ctx, kv, rev, accountID, &rec)
case StatusModifying:
return r.reconcileModifying(ctx, kv, rev, accountID, &rec)
case StatusStopping:
return r.reconcileStopping(ctx, kv, rev, accountID, &rec)
case StatusDeleting:
@@ -262,6 +264,78 @@ func (r *Reconciler) reconcileRestarting(ctx context.Context, kv jetstream.KeyVa
return nil
}
// A modify is the one transition with work left on both sides of the VM coming
// back: the disruptive change itself, which a dead leader may have left
// half-applied, and the in-guest filesystem grow, which can only run once the
// agent is up again. Both are driven from here so a customer's modify completes
// without them, not just without them watching.
func (r *Reconciler) reconcileModifying(ctx context.Context, kv jetstream.KeyValue, rev uint64, accountID string, rec *DBInstanceRecord) error {
pending := rec.PendingModifiedValues
overrun := time.Since(transitionStarted(rec)) > transitionTimeout
// Still-unapplied disruptive values mean the modify never got as far as the
// VM, so it is re-run rather than waited on: every step is idempotent, and
// the record is what says which ones are outstanding.
if !pending.empty() && !pending.growingFilesystem() {
// The lease is what separates the two: a change still inside its own API
// call holds it, and one whose worker died does not.
resumed, err := r.svc.withModifyLease(ctx, kv, rec.DBInstanceIdentifier, func() error {
return r.svc.applyPendingModifications(ctx, kv, accountID, rec)
})
switch {
case err != nil && overrun:
// Claiming and releasing the lease moved the record, so this pass's
// revision is stale by now and a CAS on it would lose to our own
// write on every pass — leaving the instance modifying forever.
return r.transitionFresh(ctx, kv, rec.DBInstanceIdentifier, StatusFailed,
fmt.Sprintf("the DB instance could not be modified within %s: %v", transitionTimeout, err))
case err != nil:
slog.WarnContext(ctx, "rds reconciler: resuming a modify failed; retrying next pass",
"dbInstance", rec.DBInstanceIdentifier, "err", err)
case !resumed && overrun:
// Held for longer than the whole budget, so the holder is wedged
// rather than working; failing it is what lets the customer retry.
return r.transition(ctx, kv, rev, rec, StatusFailed,
fmt.Sprintf("the DB instance was still being modified after %s", transitionTimeout))
case !resumed:
slog.DebugContext(ctx, "rds reconciler: a modify is already in flight; leaving it to its holder",
"dbInstance", rec.DBInstanceIdentifier)
}
return nil
}
// The VM keeps its ID across a grow's restart and gets a new one across a
// class change, so only a beat sent after the modify began proves the engine
// is back rather than that it was up before the change started.
healthy, err := r.engineReady(ctx, accountID, rec, transitionStarted(rec))
if err != nil {
return err
}
if !healthy {
if overrun {
return r.transition(ctx, kv, rev, rec, StatusFailed,
fmt.Sprintf("the database engine did not report healthy within %s of the modification", transitionTimeout))
}
return nil
}
if pending.growingFilesystem() {
if err := r.svc.finishFilesystemGrow(ctx, kv, accountID, rec); err != nil {
if overrun {
return r.transition(ctx, kv, rev, rec, StatusFailed,
fmt.Sprintf("the filesystem could not be grown within %s: %v", transitionTimeout, err))
}
slog.WarnContext(ctx, "rds reconciler: extending the filesystem failed; retrying next pass",
"dbInstance", rec.DBInstanceIdentifier, "err", err)
return nil
}
// The record moved under the revision this pass read, so the transition
// is left to the next one rather than raced.
return nil
}
return r.transition(ctx, kv, rev, rec, StatusAvailable, "")
}
// A stop whose caller died leaves the VM possibly still running, so the stop is
// re-issued rather than assumed: it is idempotent, and a VM no node holds is
// confirmed down against the fleet before the record calls it stopped.
@@ -376,6 +450,17 @@ func (r *Reconciler) transition(ctx context.Context, kv jetstream.KeyValue, rev
return nil
}
// transition against a freshly read revision, for a caller that has written the
// record itself since the pass opened and so cannot use the revision it read.
func (r *Reconciler) transitionFresh(ctx context.Context, kv jetstream.KeyValue, id string, to Status, reason string) error {
var rec DBInstanceRecord
rev, found, err := getJSONRevision(ctx, kv, DBInstanceKey(id), &rec)
if err != nil || !found {
return err
}
return r.transition(ctx, kv, rev, &rec, to, reason)
}
// Adapts an EC2 describe fan-out to the reconciler's narrow state lookup.
type describeInstanceState struct {
describe func(input *ec2.DescribeInstancesInput, accountID string) (*ec2.DescribeInstancesOutput, error)
+1 -1
View File
@@ -166,7 +166,7 @@ func TestReconciler_LeavesASlowBootstrapAloneInsideTheWindow(t *testing.T) {
// A settled instance, and a transition owned by a later phase, are both left
// alone: touching one would race its owner.
func TestReconciler_LeavesSettledInstancesAlone(t *testing.T) {
for _, status := range []Status{StatusAvailable, StatusStopped, StatusModifying, StatusBackingUp, StatusFailed} {
for _, status := range []Status{StatusAvailable, StatusStopped, StatusBackingUp, StatusFailed} {
t.Run(string(status), func(t *testing.T) {
h := newReconcileHarness(t)
rec := healthyRecord()
+68 -2
View File
@@ -22,10 +22,23 @@ type DBInstanceRecord struct {
// command channel and never persisted, so this records only that it changed.
MasterPasswordUpdatedAt *time.Time `json:"masterPasswordUpdatedAt,omitempty"`
// Blocks DeleteDBInstance outright (D18). Settable at create; rds-5b adds it
// to ModifyDBInstance.
// Blocks DeleteDBInstance outright (D18). Settable at create and modify.
DeletionProtection bool `json:"deletionProtection,omitempty"`
// Stored by ModifyDBInstance but not yet acted on: rds-9's backup and
// maintenance machinery is what consumes them.
BackupRetentionPeriod int64 `json:"backupRetentionPeriod,omitempty"`
PreferredBackupWindow string `json:"preferredBackupWindow,omitempty"`
PreferredMaintenanceWindow string `json:"preferredMaintenanceWindow,omitempty"`
// What a modify asked for and has not yet delivered. Nil once everything is
// in effect, which is also what tells the reconciler a modify is finished.
PendingModifiedValues *PendingModifiedValues `json:"pendingModifiedValues,omitempty"`
// Held by whichever worker is applying those values, so a second one does
// not enter the same change. Nil when nothing is being applied.
ModifyLease *ModifyLease `json:"modifyLease,omitempty"`
// Where the customer ENI was placed, so a replace lands the new VM's ENI in
// the same subnet and security groups without re-deriving them.
SubnetID string `json:"subnetId,omitempty"`
@@ -85,6 +98,59 @@ type DBInstanceRecord struct {
UpdatedAt time.Time `json:"updatedAt"`
}
// The disruptive half of a modify: every field here takes the engine down, so
// each is recorded before the work starts and cleared as it lands. That makes
// one structure serve both meanings AWS gives PendingModifiedValues — a
// deferred change waiting for its maintenance window, and an in-flight change a
// crashed leader has to be able to finish (D12/D15/D16).
//
// MasterUserPassword is deliberately absent: D8 forbids persisting the
// cleartext, and AWS applies a password change as soon as possible regardless
// of ApplyImmediately, so there is nothing to defer.
type PendingModifiedValues struct {
AllocatedStorage *int64 `json:"allocatedStorage,omitempty"`
DBInstanceClass string `json:"dbInstanceClass,omitempty"`
DBParameterGroupName string `json:"dbParameterGroupName,omitempty"`
// The data volume is already at its new size but the in-guest filesystem is
// not yet on it, so a resumed grow extends the filesystem rather than
// re-running a volume modify that would then be rejected as a shrink.
FilesystemGrowPending bool `json:"filesystemGrowPending,omitempty"`
// When the modify was accepted, so an operator can see how long a deferred
// change has been waiting on its window.
RequestedAt time.Time `json:"requestedAt"`
}
// Reports whether anything is still outstanding, which is what lets the record
// drop the whole structure rather than keep an empty one.
func (p *PendingModifiedValues) empty() bool {
return p == nil || (p.AllocatedStorage == nil && p.DBInstanceClass == "" &&
p.DBParameterGroupName == "" && !p.FilesystemGrowPending)
}
// An instance that has never been modified carries no pending values at all, so
// the nil case has to read as "nothing outstanding" rather than panic.
func (p *PendingModifiedValues) growingFilesystem() bool {
return p != nil && p.FilesystemGrowPending
}
// Claimed for as long as a worker is applying PendingModifiedValues, and
// renewed while it works. A modify still inside its own API call and one a dead
// leader abandoned are the same record otherwise — both are modifying with
// values not yet applied — so this is the only thing that tells them apart.
type ModifyLease struct {
// The node and the claim, so two workers on one node are still distinct.
Holder string `json:"holder"`
ExpiresAt time.Time `json:"expiresAt"`
}
// Whether someone is still renewing it. An expired lease is what makes an
// abandoned modify resumable rather than stuck behind a worker that is gone.
func (l *ModifyLease) live() bool {
return l != nil && time.Now().UTC().Before(l.ExpiresAt)
}
var _ TaggedRecord = (*DBInstanceRecord)(nil)
func (r *DBInstanceRecord) GetTags() map[string]string { return r.Tags }
+176
View File
@@ -0,0 +1,176 @@
package handlers_rds
import (
"context"
"errors"
"fmt"
"log/slog"
"github.com/mulgadc/spinifex/spinifex/utils"
"github.com/nats-io/nats.go/jetstream"
)
// The one primitive a class change, a storage grow that rides the same outage,
// and rds-6's auto-recovery all reduce to: stop the engine, terminate the VM,
// launch a fresh one at the target size, re-attach the same data volume (D9)
// and the same persisted customer ENI (D5), and rewrite the rds-system index so
// the superseded agent can no longer authenticate as this instance (D3).
//
// The endpoint is untouched throughout: the customer ENI keeps its address, so
// the DNS record and the serving cert's SANs stay correct and clients reconnect
// to the same hostname.
// What the replacement VM has to come back as.
type replaceInput struct {
// The db.* class and the EC2 instance type behind it. Empty keeps the
// record's current sizing, which is what auto-recovery wants.
InstanceClass string
InstanceType string
// A storage grow to perform while no VM holds the volume. Zero grows
// nothing; ModifyVolume would refuse an attached volume anyway, so a grow
// requested alongside a class change rides this window rather than opening a
// second one.
GrowStorageToGiB int64
// Recorded on the customer's event ring, so a replace is attributable to the
// change or the failure that caused it.
Reason string
}
// Swaps the VM under a DB instance and leaves the record naming the new one.
// Only the VM's identity is written here — the class, the storage size and the
// pending values belong to the caller that asked for the change, so a failure
// part-way through leaves the request still recorded and retryable.
func (s *Service) replaceInstanceVM(ctx context.Context, kv jetstream.KeyValue, accountID string, rec *DBInstanceRecord, in replaceInput) error {
if rec.ENIID == "" || rec.DataVolumeID == "" {
return fmt.Errorf("rds: DB instance %s has no persisted ENI and data volume to replace onto",
rec.DBInstanceIdentifier)
}
instanceType := in.InstanceType
if instanceType == "" {
resolved, err := InstanceTypeForClass(rec.DBInstanceClass)
if err != nil {
return fmt.Errorf("rds: DB instance %s has an unmapped class %q", rec.DBInstanceIdentifier, rec.DBInstanceClass)
}
instanceType = resolved
}
// Checkpointed first so the replacement boots on a clean datadir rather than
// one it has to replay a WAL over. A wedged agent degrades this rather than
// blocking the replace, exactly as a stop does.
s.stopEngineOrRecordFallback(ctx, accountID, rec, "replacing its VM")
oldInstanceID := rec.InstanceID
if err := s.terminateInstanceVM(ctx, oldInstanceID); err != nil {
return fmt.Errorf("terminate the VM behind %s: %w", rec.DBInstanceIdentifier, err)
}
// The system NIC is disposable and the fresh launch mints its own; leaving
// this one behind would hold an address in the shared RDS system subnet.
s.deleteSystemENI(ctx, rec)
// The only moment the volume is held by nothing, which is the only moment
// ModifyVolume will take it.
if in.GrowStorageToGiB > 0 {
if err := s.growDataVolume(ctx, rec.DataVolumeID, in.GrowStorageToGiB); err != nil {
return err
}
}
launched, err := s.launchReplacementVM(ctx, accountID, rec, instanceType)
if err != nil {
return err
}
// Written before the record so an agent that boots fast enough to call the
// gateway is resolvable; the old entry goes first, or a superseded VM still
// authenticates as this instance.
if err := s.rewriteInstanceIndex(ctx, accountID, rec, oldInstanceID, launched.InstanceID); err != nil {
s.unwindLaunched(ctx, launched)
return err
}
if err := s.updateInstance(ctx, kv, rec.DBInstanceIdentifier, func(stored *DBInstanceRecord) {
stored.InstanceID = launched.InstanceID
stored.SystemENIID = launched.SystemENIID
stored.VMGeneration++
// The new VM has never reported, so the old VM's health must not read as
// this one's — the reconciler would call the replace finished at once.
stored.Agent = AgentState{}
}); err != nil {
s.unwindLaunched(ctx, launched)
return err
}
// Kept in step so the caller's own record write does not resurrect the old
// VM's identity on top of this one.
rec.InstanceID = launched.InstanceID
rec.SystemENIID = launched.SystemENIID
rec.VMGeneration++
rec.Agent = AgentState{}
slog.InfoContext(ctx, "rds: DB VM replaced",
"dbInstance", rec.DBInstanceIdentifier, "from", oldInstanceID, "to", launched.InstanceID,
"instanceType", instanceType, "generation", rec.VMGeneration, "reason", in.Reason)
s.RecordEvent(ctx, accountID, EventSourceTypeDBInstance, rec.DBInstanceIdentifier,
"DB instance is being replaced onto a new VM: "+in.Reason, EventCategoryConfigurationChange)
return nil
}
// A fresh COW clone of the engine AMI, re-attached to the identity the DB
// instance owns. The new agent fetches its bootstrap config in attach mode
// (D8): it gets the port, the resolved parameters and a freshly minted serving
// cert, but no master password, and rds-init skips initdb on the datadir it
// finds already initialised.
func (s *Service) launchReplacementVM(ctx context.Context, accountID string, rec *DBInstanceRecord, instanceType string) (*LaunchOutput, error) {
launched, err := LaunchDBInstanceVM(ctx, s.deps.Launch, LaunchInput{
DBInstanceIdentifier: rec.DBInstanceIdentifier,
AccountID: accountID,
SubnetID: rec.SubnetID,
SecurityGroupIDs: rec.VpcSecurityGroupIDs,
Engine: rec.Engine,
EngineVersion: rec.EngineVersion,
InstanceType: instanceType,
AllocatedStorage: rec.AllocatedStorage,
ExistingCustomerENI: rec.ENIID,
ExistingDataVolume: rec.DataVolumeID,
UserData: buildAgentUserData(agentUserDataInput{
GatewayURL: s.deps.GatewayURL,
GatewayCACert: s.deps.GatewayCACert,
Region: s.region,
DBInstanceIdentifier: rec.DBInstanceIdentifier,
EngineVersion: rec.EngineVersion,
EnginePort: rec.Port,
}),
IamInstanceProfileArn: ensureInstanceProfile(s.deps.IAM, utils.GlobalAccountID),
})
if err != nil {
return nil, fmt.Errorf("launch the replacement VM for %s: %w", rec.DBInstanceIdentifier, err)
}
return launched, nil
}
// The instance index keys off the internal EC2 instance ID, which every replace
// mints anew. The old entry is removed first: while it exists the superseded
// agent's IMDS credentials still resolve to this DB instance (D3).
func (s *Service) rewriteInstanceIndex(ctx context.Context, accountID string, rec *DBInstanceRecord, oldInstanceID, newInstanceID string) error {
if oldInstanceID != "" && oldInstanceID != newInstanceID {
if err := s.DeleteInstanceIndex(ctx, oldInstanceID); err != nil && !errors.Is(err, jetstream.ErrKeyNotFound) {
return fmt.Errorf("rds: drop the superseded instance index entry for %s: %w", rec.DBInstanceIdentifier, err)
}
}
if err := s.PutInstanceIndex(ctx, newInstanceID, InstanceIndexEntry{
AccountID: accountID,
DBInstanceIdentifier: rec.DBInstanceIdentifier,
VMGeneration: rec.VMGeneration + 1,
}); err != nil {
return fmt.Errorf("rds: write the instance index for %s: %w", rec.DBInstanceIdentifier, err)
}
return nil
}
// Best-effort: a leaked system NIC costs an address in a platform-owned subnet,
// which is not worth failing a replace that has already terminated the VM.
func (s *Service) deleteSystemENI(ctx context.Context, rec *DBInstanceRecord) {
if rec.SystemENIID == "" || s.deps.Launch.VPC == nil {
return
}
deleteLaunchENI(ctx, s.deps.Launch.VPC, utils.GlobalAccountID, rec.SystemENIID)
}
+301
View File
@@ -0,0 +1,301 @@
package handlers_rds
import (
"errors"
"testing"
"time"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// The instance ID fakeLauncher hands back, which is what a replace has to leave
// the record and the index naming.
const testReplacementInstance = "i-rds0001"
// Puts the record in KV along with the instance-index entry the current VM's
// agent authenticates through, which is the pair a replace has to move
// together.
func seedReplaceable(t *testing.T, h *modifyHarness, rec DBInstanceRecord) DBInstanceRecord {
t.Helper()
seedInstance(t, h.svc, rec)
require.NoError(t, h.svc.PutInstanceIndex(t.Context(), rec.InstanceID, InstanceIndexEntry{
AccountID: testAccountID,
DBInstanceIdentifier: rec.DBInstanceIdentifier,
VMGeneration: rec.VMGeneration,
}))
return rec
}
// D5/D9: the endpoint ENI and the datadir outlive the VM, so a replace adopts
// both. Minting either would move the address clients resolve or hand the
// customer an empty database.
func TestReplaceInstanceVM_ReusesTheEndpointENIAndDataVolume(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec,
replaceInput{InstanceClass: "db.m5.large", InstanceType: "m5.large", Reason: "the instance class changed"}))
require.NotNil(t, h.launch.launcher.input)
require.Len(t, h.launch.launcher.input.ExtraENIs, 1)
endpoint := h.launch.launcher.input.ExtraENIs[0]
assert.Equal(t, testEndpointENI, endpoint.ENIID)
assert.Equal(t, testEndpointIP, endpoint.ENIIP)
assert.Equal(t, testEndpointMAC, endpoint.ENIMac, "the replacement NIC comes up with the endpoint's own MAC")
assert.Equal(t, "m5.large", h.launch.launcher.input.InstanceType)
// The stale attachment is cleared first: re-attaching an ENI that still
// reads as attached is rejected.
assert.Contains(t, h.launch.enis.detached, testEndpointENI)
assert.Len(t, h.launch.enis.created, 1, "only the disposable system NIC is minted")
assert.NotContains(t, h.launch.enis.deleted, testEndpointENI)
assert.Empty(t, h.launch.volumes.created, "the datadir is re-attached, not recreated")
assert.Equal(t, testDataVolume, h.launch.attacher.volumeID)
assert.Empty(t, h.launch.volumes.deleted)
}
// The engine is checkpointed and the old VM is gone before the new one comes
// up, or two VMs hold the same datadir.
func TestReplaceInstanceVM_StopsTheEngineAndTerminatesTheOldVMFirst(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
var terminatedAtLaunch []string
h.launch.launcher.onLaunch = func() {
terminatedAtLaunch = append([]string(nil), h.launch.launcher.terminated...)
}
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"}))
assert.Equal(t, []string{testInstance}, terminatedAtLaunch)
issued := h.agent.received()
require.Len(t, issued, 1)
assert.Equal(t, CommandStopEngine, issued[0].Type)
// The old system NIC is disposable, and leaving it behind would hold an
// address in the shared RDS system subnet.
assert.Contains(t, h.launch.enis.deleted, "eni-sys01")
}
// D3: while the old index entry exists the superseded agent's IMDS credentials
// still resolve to this DB instance, so it goes before the new one lands.
func TestReplaceInstanceVM_RewritesTheInstanceIndex(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"}))
superseded, err := h.svc.LookupInstanceIndex(t.Context(), testInstance)
require.NoError(t, err)
assert.Nil(t, superseded, "the superseded VM must no longer resolve to this DB instance")
current, err := h.svc.LookupInstanceIndex(t.Context(), testReplacementInstance)
require.NoError(t, err)
require.NotNil(t, current)
assert.Equal(t, testDBID, current.DBInstanceIdentifier)
assert.Equal(t, testAccountID, current.AccountID)
assert.Equal(t, int64(1), current.VMGeneration)
}
// The record names the new VM and forgets the old one's health, while every
// field the endpoint is built from is left exactly as it was.
func TestReplaceInstanceVM_RecordsTheNewVMAndKeepsTheEndpoint(t *testing.T) {
h := newModifyHarness(t)
seed := modifiableRecord()
beat := time.Now().UTC()
seed.Agent = AgentState{InstanceID: testInstance, EngineHealth: EngineHealthHealthy, LastSeen: &beat}
rec := seedReplaceable(t, h, seed)
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"}))
stored := h.record(t)
assert.Equal(t, testReplacementInstance, stored.InstanceID)
assert.Equal(t, int64(1), stored.VMGeneration)
assert.NotEqual(t, "eni-sys01", stored.SystemENIID)
// The old VM's health must not read as the new one's, or the reconciler
// calls the replace finished before the replacement has said anything.
assert.Empty(t, stored.Agent.InstanceID)
assert.Nil(t, stored.Agent.LastSeen)
assert.Equal(t, testEndpointENI, stored.ENIID)
assert.Equal(t, seed.ENIPrivateIP, stored.ENIPrivateIP)
assert.Equal(t, seed.DNSName, stored.DNSName)
assert.Equal(t, testDataVolume, stored.DataVolumeID)
// The caller's copy is kept in step, so its own record write cannot
// resurrect the old VM's identity on top of this one.
assert.Equal(t, testReplacementInstance, rec.InstanceID)
assert.Equal(t, int64(1), rec.VMGeneration)
}
// A grow riding a class change takes the only window in which nothing holds the
// volume: after the old VM is gone and before the new one exists.
func TestReplaceInstanceVM_GrowsTheVolumeWhileNoVMHoldsIt(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
var terminatedAtGrow int
var launchedAtGrow bool
h.storage.onModify = func() {
terminatedAtGrow = len(h.launch.launcher.terminated)
launchedAtGrow = h.launch.launcher.input != nil
}
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{
InstanceClass: "db.m5.large", InstanceType: "m5.large", GrowStorageToGiB: 80, Reason: "the instance class changed",
}))
assert.Equal(t, 1, terminatedAtGrow, "the volume was grown while the old VM still held it")
assert.False(t, launchedAtGrow, "the volume was grown after the replacement had already attached it")
assert.Equal(t, int64(80), h.storage.sizes[testDataVolume])
}
// An unmapped class has no instance type behind it, so the replace is refused
// before the VM it would land on is torn down.
func TestReplaceInstanceVM_RefusesAnUnmappedRecordedClass(t *testing.T) {
h := newModifyHarness(t)
seed := modifiableRecord()
seed.DBInstanceClass = "db.r5.24xlarge"
rec := seedReplaceable(t, h, seed)
err := h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"})
require.Error(t, err)
assert.Empty(t, h.launch.launcher.terminated)
}
// Without the persisted ENI and volume there is nothing to replace *onto*: a
// launch from here would mint a new address and an empty datadir.
func TestReplaceInstanceVM_RefusesWithoutThePersistedIdentity(t *testing.T) {
for name, mutate := range map[string]func(*DBInstanceRecord){
"no endpoint ENI": func(rec *DBInstanceRecord) { rec.ENIID = "" },
"no data volume": func(rec *DBInstanceRecord) { rec.DataVolumeID = "" },
} {
t.Run(name, func(t *testing.T) {
h := newModifyHarness(t)
seed := modifiableRecord()
mutate(&seed)
rec := seedReplaceable(t, h, seed)
err := h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"})
require.Error(t, err)
assert.Empty(t, h.launch.launcher.terminated)
assert.Nil(t, h.launch.launcher.input)
})
}
}
// An endpoint ENI that no longer exists is an error, not an invitation to mint
// a replacement: a new ENI would come up on a different address than the one
// DNS and the serving certificate name.
func TestReplaceInstanceVM_RefusesToMintAReplacementEndpoint(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
h.launch.enis.describeMissing = true
err := h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"})
require.Error(t, err)
assert.Nil(t, h.launch.launcher.input)
assert.Equal(t, testInstance, h.record(t).InstanceID)
}
// Every step that can fail leaves the record still naming the VM and generation
// it had, so the reconciler resumes from a state it can read rather than from
// one that half-describes two VMs.
func TestReplaceInstanceVM_LeavesTheRecordRecoverableOnEveryFailure(t *testing.T) {
cases := map[string]func(*modifyHarness){
"the old VM will not terminate": func(h *modifyHarness) {
h.launch.launcher.terminateErr = errors.New("the node did not answer")
},
"the volume will not grow": func(h *modifyHarness) {
h.storage.modifyErr = errors.New("the volume store is unavailable")
},
"the replacement will not launch": func(h *modifyHarness) {
h.launch.launcher.err = errors.New("no capacity on any node")
},
}
for name, breakIt := range cases {
t.Run(name, func(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
breakIt(h)
err := h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec,
replaceInput{GrowStorageToGiB: 80, Reason: "the instance class changed"})
require.Error(t, err)
stored := h.record(t)
assert.Equal(t, testInstance, stored.InstanceID)
assert.Zero(t, stored.VMGeneration)
assert.Equal(t, testEndpointENI, stored.ENIID)
assert.Equal(t, testDataVolume, stored.DataVolumeID)
// The index still resolves the VM the record names, so an agent that
// is somehow still alive is not locked out of a control plane that
// has not moved on.
entry, err := h.svc.LookupInstanceIndex(t.Context(), testInstance)
require.NoError(t, err)
require.NotNil(t, entry)
assert.Equal(t, testDBID, entry.DBInstanceIdentifier)
})
}
}
// A failed launch tears down what it created rather than leaving the DB
// instance's own ENI and volume attached to a VM nobody will ever record.
func TestReplaceInstanceVM_UnwindsAFailedLaunchWithoutTouchingTheEndpoint(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
h.launch.launcher.err = errors.New("no capacity on any node")
require.Error(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"}))
assert.NotContains(t, h.launch.enis.deleted, testEndpointENI, "the endpoint ENI must survive a failed replace")
assert.Empty(t, h.launch.volumes.deleted, "the datadir must survive a failed replace")
}
// Auto-recovery replaces onto the sizing the record already carries, so an
// empty class in the request is not a class change.
func TestReplaceInstanceVM_KeepsTheRecordedSizingWhenNoneIsRequested(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"}))
require.NotNil(t, h.launch.launcher.input)
assert.Equal(t, "t3.medium", h.launch.launcher.input.InstanceType)
assert.Equal(t, "db.t3.medium", h.record(t).DBInstanceClass)
assert.Empty(t, h.storage.modified, "a replace with no grow requested does not touch the volume")
}
// The customer's own event ring is where a replace becomes attributable to the
// change or the failure that caused it.
func TestReplaceInstanceVM_RecordsWhyTheVMWasReplaced(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec,
replaceInput{Reason: "the instance class changed to db.m5.large"}))
messages := h.eventMessages(t)
require.NotEmpty(t, messages)
assert.Contains(t, messages[0], "db.m5.large")
}
// The replacement agent's first bootstrap must be an attach, not an
// initialize: the datadir is already there, and a second initdb would be
// destructive. The user data is what carries the identity it fetches with.
func TestReplaceInstanceVM_LaunchesWithTheInstancesOwnIdentity(t *testing.T) {
h := newModifyHarness(t)
rec := seedReplaceable(t, h, modifiableRecord())
require.NoError(t, h.svc.replaceInstanceVM(t.Context(), h.kv(t), testAccountID, &rec, replaceInput{Reason: "recovery"}))
require.NotNil(t, h.launch.launcher.input)
assert.Contains(t, h.launch.launcher.input.UserData, testDBID)
assert.Equal(t, testDBSubnet, h.launch.launcher.input.ExtraENIs[0].SubnetID)
assert.Equal(t, testAccountID, h.launch.launcher.input.ExtraENIs[0].AccountID)
assert.NotEqual(t, testAccountID, h.launch.launcher.input.AccountID,
"the VM and its management NIC live in the system account")
}
+2
View File
@@ -37,6 +37,8 @@ type Deps struct {
// Takes the final snapshot at delete, and reports which snapshots still
// reference a data volume before it can be deleted.
Snapshots snapshotProvider
// Grows the data volume behind an AllocatedStorage modify (D12).
Storage volumeResizer
// The northstar base zone. Empty means no vanity hostname, and the endpoint
// is the customer-ENI IP instead (D6).
BaseDomain string
+8
View File
@@ -21,6 +21,9 @@ const createTimeout = 5 * time.Minute
const (
lifecycleTimeout = 4 * time.Minute
deleteTimeout = 6 * time.Minute
// A class change terminates a VM and launches a replacement inline, so a
// modify needs a create's budget rather than a lifecycle op's.
modifyTimeout = 6 * time.Minute
)
// The gateway-side adapter that forwards each agent action as a NATS request to
@@ -68,6 +71,11 @@ func (s *NATSService) StopDBInstance(ctx context.Context, input *rds.StopDBInsta
SubjectStopDBInstance, input, lifecycleTimeout, accountID)
}
func (s *NATSService) ModifyDBInstance(ctx context.Context, input *rds.ModifyDBInstanceInput, accountID string) (*rds.ModifyDBInstanceOutput, error) {
return utils.NATSRequest[rds.ModifyDBInstanceOutput](ctx, s.nc,
SubjectModifyDBInstance, input, modifyTimeout, accountID)
}
func (s *NATSService) DeleteDBInstance(ctx context.Context, input *rds.DeleteDBInstanceInput, accountID string) (*rds.DeleteDBInstanceOutput, error) {
return utils.NATSRequest[rds.DeleteDBInstanceOutput](ctx, s.nc,
SubjectDeleteDBInstance, input, deleteTimeout, accountID)
+3 -2
View File
@@ -40,8 +40,9 @@ var transitions = map[Status][]Status{
StatusStarting: {StatusAvailable, StatusFailed, StatusDeleting},
StatusRecovering: {StatusAvailable, StatusFailed, StatusDeleting},
// A failed instance is retried by the reconciler rather than being stuck:
// recovering is the reconciler's path back, available the heartbeat's.
StatusFailed: {StatusRecovering, StatusAvailable, StatusDeleting},
// recovering is the reconciler's path back, available the heartbeat's, and
// modifying the customer's own retry of the change that failed.
StatusFailed: {StatusRecovering, StatusAvailable, StatusModifying, StatusDeleting},
StatusDeleting: {StatusDeleted, StatusFailed},
StatusDeleted: nil,
}
+4
View File
@@ -59,6 +59,10 @@ func TestCanTransition_Lifecycle(t *testing.T) {
{StatusAvailable, StatusRecovering, true},
{StatusAvailable, StatusFailed, true},
{StatusFailed, StatusRecovering, true},
// A failed instance is exactly the one a customer retries a change on.
{StatusFailed, StatusModifying, true},
{StatusAvailable, StatusModifying, true},
{StatusModifying, StatusAvailable, true},
{StatusRecovering, StatusAvailable, true},
// A re-observation of the same state is not an illegal move.
+116
View File
@@ -0,0 +1,116 @@
package handlers_rds
import (
"context"
"errors"
"fmt"
"log/slog"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/awserrors"
"github.com/mulgadc/spinifex/spinifex/utils"
)
// D12: ModifyVolume refuses a volume a running VM holds, so a storage grow is
// stop the engine, stop the VM, grow the volume, start the VM, and extend the
// filesystem inside the guest. There is no online grow in v1, and a shrink is
// rejected outright — as AWS rejects it.
// The volume-level half of a grow, kept apart from the launch provisioner
// because a grow reads the volume's current size before modifying it and a
// launch never does.
type volumeResizer interface {
DescribeVolumes(ctx context.Context, input *ec2.DescribeVolumesInput, accountID string) (*ec2.DescribeVolumesOutput, error)
ModifyVolume(ctx context.Context, input *ec2.ModifyVolumeInput, accountID string) (*ec2.ModifyVolumeOutput, error)
}
// Everything a resize request can be wrong about, checked before the instance
// moves out of available so a bad request never stops a running database. A
// request that merely repeats the current size is not an error: Terraform sends
// the whole body on every apply, so rejecting it would fail every unrelated
// modify. It is dropped from the plan instead, which changes no state.
func validateStorageGrow(current, requested int64) error {
switch {
case requested < minAllocatedStorageGiB || requested > maxAllocatedStorageGiB:
return fmt.Errorf("%s: AllocatedStorage must be between %d and %d GiB",
awserrors.ErrorInvalidParameterValue, minAllocatedStorageGiB, maxAllocatedStorageGiB)
case requested < current:
return fmt.Errorf("%s: AllocatedStorage cannot be reduced from %d to %d GiB; storage is grow-only",
awserrors.ErrorInvalidParameterCombination, current, requested)
}
return nil
}
// The stop/grow/start cycle for a grow with no class change alongside it. A
// class change opens the same outage and grows the volume inside it instead,
// so the two never run back to back.
func (s *Service) growInstanceStorage(ctx context.Context, accountID string, rec *DBInstanceRecord, targetGiB int64) error {
if s.deps.Instances == nil {
return errors.New("rds: no instance command path configured")
}
// The engine is checkpointed first so the filesystem the grow extends is not
// one a live postmaster is still writing into.
s.stopEngineOrRecordFallback(ctx, accountID, rec, "growing its storage")
if err := s.stopInstanceVM(ctx, accountID, rec.InstanceID); err != nil {
return fmt.Errorf("stop the DB VM before growing its storage: %w", err)
}
if err := s.growDataVolume(ctx, rec.DataVolumeID, targetGiB); err != nil {
return err
}
if err := s.startInstanceVM(ctx, rec.InstanceID); err != nil {
return fmt.Errorf("restart the DB VM after growing its storage: %w", err)
}
return nil
}
// Grows the data volume itself. Idempotent: a resumed grow re-reads the volume
// and returns without a second modify once it is already at the target, which
// ModifyVolume would otherwise reject as a shrink.
func (s *Service) growDataVolume(ctx context.Context, volumeID string, targetGiB int64) error {
if volumeID == "" {
return errors.New("rds: the DB instance has no data volume to grow")
}
if s.deps.Storage == nil {
return errors.New("rds: no volume resize path configured")
}
current, err := s.dataVolumeSize(ctx, volumeID)
if err != nil {
return err
}
if current >= targetGiB {
slog.InfoContext(ctx, "rds: data volume is already at the requested size; skipping the modify",
"volumeId", volumeID, "sizeGiB", current, "targetGiB", targetGiB)
return nil
}
if _, err := s.deps.Storage.ModifyVolume(ctx, &ec2.ModifyVolumeInput{
VolumeId: aws.String(volumeID),
Size: aws.Int64(targetGiB),
}, utils.GlobalAccountID); err != nil {
return fmt.Errorf("grow the data volume %s to %d GiB: %w", volumeID, targetGiB, err)
}
slog.InfoContext(ctx, "rds: data volume grown",
"volumeId", volumeID, "fromGiB", current, "toGiB", targetGiB)
return nil
}
// The volume's own reported size, which is what makes the grow idempotent. A
// volume the store cannot describe is an error rather than a zero, since zero
// would read as "smaller than the target" and trigger a modify.
func (s *Service) dataVolumeSize(ctx context.Context, volumeID string) (int64, error) {
out, err := s.deps.Storage.DescribeVolumes(ctx, &ec2.DescribeVolumesInput{
VolumeIds: aws.StringSlice([]string{volumeID}),
}, utils.GlobalAccountID)
if err != nil {
return 0, fmt.Errorf("read the data volume %s: %w", volumeID, err)
}
for _, volume := range out.Volumes {
if aws.StringValue(volume.VolumeId) == volumeID {
return aws.Int64Value(volume.Size), nil
}
}
return 0, fmt.Errorf("rds: data volume %s no longer exists", volumeID)
}
+183
View File
@@ -0,0 +1,183 @@
package handlers_rds
import (
"context"
"errors"
"fmt"
"testing"
"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/mulgadc/spinifex/spinifex/awserrors"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
// fakeVolumeResizer is the volume store as a grow reads and writes it, and it
// enforces the store's own grow-only rule so an idempotence bug surfaces here
// rather than in production.
type fakeVolumeResizer struct {
sizes map[string]int64
modified []*ec2.ModifyVolumeInput
describeErr error
modifyErr error
// missing makes the volume read as gone, which is the state a grow must not
// interpret as "smaller than the target".
missing bool
// onModify runs inside the resize, so a test can observe what else has and
// has not happened by the time the volume is taken.
onModify func()
}
var _ volumeResizer = (*fakeVolumeResizer)(nil)
func newFakeVolumeResizer(volumeID string, sizeGiB int64) *fakeVolumeResizer {
return &fakeVolumeResizer{sizes: map[string]int64{volumeID: sizeGiB}}
}
func (f *fakeVolumeResizer) DescribeVolumes(_ context.Context, in *ec2.DescribeVolumesInput, _ string) (*ec2.DescribeVolumesOutput, error) {
if f.describeErr != nil {
return nil, f.describeErr
}
out := &ec2.DescribeVolumesOutput{}
if f.missing {
return out, nil
}
for _, id := range aws.StringValueSlice(in.VolumeIds) {
size, ok := f.sizes[id]
if !ok {
continue
}
out.Volumes = append(out.Volumes, &ec2.Volume{VolumeId: aws.String(id), Size: aws.Int64(size)})
}
return out, nil
}
func (f *fakeVolumeResizer) ModifyVolume(_ context.Context, in *ec2.ModifyVolumeInput, _ string) (*ec2.ModifyVolumeOutput, error) {
if f.onModify != nil {
f.onModify()
}
if f.modifyErr != nil {
return nil, f.modifyErr
}
id, size := aws.StringValue(in.VolumeId), aws.Int64Value(in.Size)
if size < f.sizes[id] {
return nil, fmt.Errorf("volume %s cannot be shrunk from %d to %d", id, f.sizes[id], size)
}
f.modified = append(f.modified, in)
f.sizes[id] = size
return &ec2.ModifyVolumeOutput{}, nil
}
// The whole rule set a resize is checked against, ahead of any state change.
func TestValidateStorageGrow(t *testing.T) {
cases := []struct {
name string
current, request int64
wantErrContaining string
}{
{name: "grows", current: 20, request: 50},
{name: "repeats the current size", current: 20, request: 20},
{name: "shrinks", current: 100, request: 50, wantErrContaining: awserrors.ErrorInvalidParameterCombination},
{name: "below the floor", current: 20, request: 5, wantErrContaining: awserrors.ErrorInvalidParameterValue},
{name: "above the ceiling", current: 20, request: maxAllocatedStorageGiB + 1, wantErrContaining: awserrors.ErrorInvalidParameterValue},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
err := validateStorageGrow(tc.current, tc.request)
if tc.wantErrContaining == "" {
assert.NoError(t, err)
return
}
require.Error(t, err)
assert.Contains(t, err.Error(), tc.wantErrContaining)
})
}
}
// The volume store is grow-only, so a resumed grow that re-issued the modify
// would be rejected as a shrink. It reads the volume first instead.
func TestGrowDataVolume_SkipsAVolumeAlreadyAtTheTarget(t *testing.T) {
h := newModifyHarness(t)
h.storage.sizes[testDataVolume] = 50
require.NoError(t, h.svc.growDataVolume(t.Context(), testDataVolume, 50))
assert.Empty(t, h.storage.modified, "a volume already at the target is not modified again")
}
// A volume the store cannot describe must not read as zero: zero is smaller
// than every target, so the grow would go ahead against a volume that is gone.
func TestGrowDataVolume_FailsWhenTheVolumeIsGone(t *testing.T) {
h := newModifyHarness(t)
h.storage.missing = true
err := h.svc.growDataVolume(t.Context(), testDataVolume, 50)
require.Error(t, err)
assert.Contains(t, err.Error(), "no longer exists")
assert.Empty(t, h.storage.modified)
}
func TestGrowDataVolume_FailsWithoutAVolume(t *testing.T) {
h := newModifyHarness(t)
require.Error(t, h.svc.growDataVolume(t.Context(), "", 50))
}
// D12: ModifyVolume refuses a volume a running VM holds, so the order is the
// whole mechanism — engine down, VM down, grow, VM back.
func TestGrowInstanceStorage_StopsTheEngineAndTheVMBeforeGrowing(t *testing.T) {
h := newModifyHarness(t)
rec := modifiableRecord()
var vmStoppedAtGrow bool
h.storage.onModify = func() {
vmStoppedAtGrow = assert.ObjectsAreEqual([]string{"stop:" + testInstance}, h.cmdr.calls)
}
require.NoError(t, h.svc.growInstanceStorage(t.Context(), testAccountID, &rec, 50))
assert.True(t, vmStoppedAtGrow, "the volume was taken while the VM still held it")
assert.Equal(t, []string{"stop:" + testInstance, "start:" + testInstance}, h.cmdr.calls)
assert.Equal(t, int64(50), h.storage.sizes[testDataVolume])
issued := h.agent.received()
require.Len(t, issued, 1)
assert.Equal(t, CommandStopEngine, issued[0].Type)
}
// A wedged agent must not block a grow: the engine stop is a courtesy, and the
// VM stop that follows is what actually releases the volume.
func TestGrowInstanceStorage_ProceedsWhenTheEngineWillNotStop(t *testing.T) {
h := newModifyHarnessWithAgent(t, true)
rec := modifiableRecord()
require.NoError(t, h.svc.growInstanceStorage(t.Context(), testAccountID, &rec, 50))
assert.Equal(t, int64(50), h.storage.sizes[testDataVolume])
}
// A grow that fails leaves the VM down rather than restarting it onto a volume
// whose size nobody knows: the reconciler resumes from the recorded request.
func TestGrowInstanceStorage_DoesNotRestartTheVMWhenTheGrowFails(t *testing.T) {
h := newModifyHarness(t)
h.storage.modifyErr = errors.New("the volume store is unavailable")
rec := modifiableRecord()
err := h.svc.growInstanceStorage(t.Context(), testAccountID, &rec, 50)
require.Error(t, err)
assert.Equal(t, []string{"stop:" + testInstance}, h.cmdr.calls)
}
// Stopping the VM is what makes the volume available, so a stop that did not
// happen must not be followed by a resize that would be rejected — or worse,
// accepted against a volume a live guest is writing to.
func TestGrowInstanceStorage_FailsWhenTheVMWillNotStop(t *testing.T) {
h := newModifyHarness(t)
h.cmdr.err = errors.New("the node did not answer")
rec := modifiableRecord()
err := h.svc.growInstanceStorage(t.Context(), testAccountID, &rec, 50)
require.Error(t, err)
assert.Empty(t, h.storage.modified)
}
+1
View File
@@ -28,6 +28,7 @@ const (
SubjectStartDBInstance = "rds.StartDBInstance"
SubjectStopDBInstance = "rds.StopDBInstance"
SubjectDeleteDBInstance = "rds.DeleteDBInstance"
SubjectModifyDBInstance = "rds.ModifyDBInstance"
SubjectDescribeEvents = "rds.DescribeEvents"