diff --git a/cluster/calcium/create.go b/cluster/calcium/create.go index 3562e83f..d72e7b99 100644 --- a/cluster/calcium/create.go +++ b/cluster/calcium/create.go @@ -1,7 +1,6 @@ package calcium import ( - "cmp" "context" "fmt" "maps" @@ -283,7 +282,9 @@ func (c *Calcium) doGetAndPrepareNode(ctx context.Context, nodename, image strin func (c *Calcium) doDeployOneWorkload(ctx context.Context, node *types.Node, opts *types.DeployOptions, msg *types.CreateWorkloadMessage, createOpts *enginetypes.VirtualizationCreateOptions, decrProcessing bool) (err error) { logger := log.WithFunc("calcium.doDeployOneWorkload").WithField("node", node.Name).WithField("ident", opts.ProcessIdent).WithField("msg", msg) + createOpts.ID = node.Engine.VirtualizationCreateID(createOpts.Name) workload := &types.Workload{ + ID: createOpts.ID, Resources: msg.Resources, EngineParams: msg.EngineParams, Name: createOpts.Name, @@ -300,6 +301,7 @@ func (c *Calcium) doDeployOneWorkload(ctx context.Context, node *types.Node, opt } commit, err := c.journal(ctx, logger, eventWorkloadCreated, &types.Workload{ + ID: workload.ID, Name: workload.Name, Nodename: workload.Nodename, }) @@ -314,8 +316,6 @@ func (c *Calcium) doDeployOneWorkload(ctx context.Context, node *types.Node, opt if err != nil { return err } - workload.ID = created.ID - maps.Copy(workload.Labels, created.Labels) return nil }, @@ -384,11 +384,7 @@ func (c *Calcium) doDeployOneWorkload(ctx context.Context, node *types.Node, opt }, func(ctx context.Context, _ bool) (rollbackErr error) { - logger.Warnf(ctx, "failed to deploy workload %s, rollback", cmp.Or(workload.ID, workload.Name)) - if workload.ID == "" { - return removeWorkloadByName(ctx, node, workload.Name) - } - + logger.Warnf(ctx, "failed to deploy workload %s, rollback", workload.ID) if removeErr := c.store.RemoveWorkload(ctx, workload); removeErr != nil { logger.Errorf(ctx, removeErr, "failed to remove workload %s", workload.ID) rollbackErr = errors.Join(rollbackErr, removeErr) diff --git a/cluster/calcium/create_test.go b/cluster/calcium/create_test.go index 9a75a32d..434a15e9 100644 --- a/cluster/calcium/create_test.go +++ b/cluster/calcium/create_test.go @@ -193,7 +193,6 @@ func TestCreateWorkloadTxn(t *testing.T) { } engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, errors.Wrap(context.DeadlineExceeded, "VirtualizationCreate")).Twice() - engine.On("VirtualizationInspect", mock.Anything, mock.Anything).Return(nil, types.ErrWorkloadNotExists).Twice() engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return(nil, types.ErrMockError) walCommitted.Store(false) @@ -330,6 +329,7 @@ func TestDoDeployWorkloadsOnNodeErrorPerWorkload(t *testing.T) { c := NewTestCluster() ctx := t.Context() engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("wrkid").Maybe() node := &types.Node{NodeMeta: types.NodeMeta{Name: "n1"}, Engine: engine} store := c.store.(*storemocks.Store) @@ -337,7 +337,8 @@ func TestDoDeployWorkloadsOnNodeErrorPerWorkload(t *testing.T) { store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return(nil, types.ErrMockError) mockLocks(t, store) engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, types.ErrMockError) - engine.On("VirtualizationInspect", mock.Anything, mock.Anything).Return(nil, types.ErrWorkloadNotExists) + engine.On("VirtualizationRemove", mock.Anything, "wrkid", true, true).Return(types.ErrWorkloadNotExists) + c.store.(*storemocks.Store).On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil) const deploy = 4 opts := &types.DeployOptions{ @@ -360,7 +361,7 @@ func TestDoDeployWorkloadsOnNodeErrorPerWorkload(t *testing.T) { } } -func TestDoDeployOneWorkloadJournalsTheNameBeforeTheEngineCreate(t *testing.T) { +func TestDoDeployOneWorkloadJournalsTheIDBeforeTheEngineCreate(t *testing.T) { c := NewTestCluster() ctx := t.Context() @@ -375,10 +376,12 @@ func TestDoDeployOneWorkloadJournalsTheNameBeforeTheEngineCreate(t *testing.T) { c.wal = mwal engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("wrkid").Maybe() engine.On("VirtualizationCreate", mock.Anything, mock.Anything). Run(func(mock.Arguments) { engineCalled = true }). Return(nil, types.ErrMockError) - engine.On("VirtualizationInspect", mock.Anything, mock.Anything).Return(nil, types.ErrWorkloadNotExists) + engine.On("VirtualizationRemove", mock.Anything, "wrkid", true, true).Return(types.ErrWorkloadNotExists) + c.store.(*storemocks.Store).On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil) node := &types.Node{NodeMeta: types.NodeMeta{Name: "n1"}, Engine: engine} opts := &types.DeployOptions{Name: "app", Podname: "pod", Entrypoint: &types.Entrypoint{Name: "entry"}} @@ -389,7 +392,8 @@ func TestDoDeployOneWorkloadJournalsTheNameBeforeTheEngineCreate(t *testing.T) { assert.False(t, loggedAfterEngine) assert.Equal(t, createOpts.Name, logged.Name) assert.Equal(t, node.Name, logged.Nodename) - assert.Empty(t, logged.ID) + assert.Equal(t, "wrkid", logged.ID) + assert.Equal(t, logged.ID, createOpts.ID) } func TestDoDeployOneWorkloadRollbackRemovesTheContainerTheEngineKept(t *testing.T) { @@ -398,8 +402,9 @@ func TestDoDeployOneWorkloadRollbackRemovesTheContainerTheEngineKept(t *testing. name := "app_entry_abcdef" engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("wrkid").Maybe() engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, types.ErrMockError).Once() - engine.On("VirtualizationInspect", mock.Anything, name).Return(&enginetypes.VirtualizationInfo{ID: "wrkid"}, nil).Once() + c.store.(*storemocks.Store).On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil).Once() engine.On("VirtualizationRemove", mock.Anything, "wrkid", true, true).Return(nil).Once() node := &types.Node{NodeMeta: types.NodeMeta{Name: "n1"}, Engine: engine} @@ -425,6 +430,7 @@ func TestDoDeployOneWorkloadKeepsJournalWhenRollbackFails(t *testing.T) { store.On("AddWorkload", mock.Anything, mock.Anything, mock.Anything).Return(types.ErrMockError).Once() store.On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil).Once() engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("wrkid").Maybe() engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(&enginetypes.VirtualizationCreated{ID: "wrkid"}, nil).Once() engine.On("VirtualizationRemove", mock.Anything, "wrkid", true, true).Return(types.ErrMockError).Once() node := &types.Node{NodeMeta: types.NodeMeta{Name: "n1"}, Engine: engine} @@ -439,6 +445,39 @@ func TestDoDeployOneWorkloadKeepsJournalWhenRollbackFails(t *testing.T) { engine.AssertExpectations(t) } +func TestDoDeployOneWorkloadReplaysTheReservedIDAfterCreateReplyLoss(t *testing.T) { + c := NewTestCluster() + t.Cleanup(c.Finalizer) + enableTestWAL(t, c) + ctx := t.Context() + c.rmgr.(*resourcemocks.Manager).On("GetNodeResourceInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(resourcetypes.Resources{}, resourcetypes.Resources{}, []string{}, nil) + engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("reserved-id").Once() + engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, context.DeadlineExceeded).Once() + engine.On("VirtualizationRemove", mock.Anything, "reserved-id", true, true).Return(types.ErrMockError).Once() + engine.On("VirtualizationRemove", mock.Anything, "reserved-id", true, true).Return(nil).Once() + node := &types.Node{Name: "n1", Engine: engine} + store := c.store.(*storemocks.Store) + store.On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil).Once() + store.On("GetWorkload", mock.Anything, "reserved-id").Return(nil, types.ErrKeyNotFound).Once() + store.On("NotFound", types.ErrKeyNotFound).Return(true).Once() + store.On("GetNode", mock.Anything, node.Name).Return(node, nil).Once() + opts := &types.DeployOptions{Name: "app", Podname: "pod", Entrypoint: &types.Entrypoint{Name: "entry"}} + createOpts := &enginetypes.VirtualizationCreateOptions{Name: "app_entry_abcdef"} + + require.ErrorIs(t, c.doDeployOneWorkload(ctx, node, opts, &types.CreateWorkloadMessage{}, createOpts, false), context.DeadlineExceeded) + entries, err := c.store.GetPrefix(ctx, "/wal/", 0) + require.NoError(t, err) + require.Len(t, entries, 1) + c.wal.Recover(ctx) + entries, err = c.store.GetPrefix(ctx, "/wal/", 0) + require.NoError(t, err) + require.Empty(t, entries) + c.wal.Recover(ctx) + store.AssertExpectations(t) + engine.AssertExpectations(t) +} + func TestDoMakeWorkloadOptionsEnvIsolation(t *testing.T) { c := NewTestCluster() ctx := t.Context() @@ -473,6 +512,7 @@ func newCreateWorkloadCluster(t *testing.T, createProcessingErr, deleteProcessin c := NewTestCluster() engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("c1").Maybe() node1 := &types.Node{ NodeMeta: types.NodeMeta{ Name: "n1", diff --git a/cluster/calcium/lambda_test.go b/cluster/calcium/lambda_test.go index 221ee7f4..496863d3 100644 --- a/cluster/calcium/lambda_test.go +++ b/cluster/calcium/lambda_test.go @@ -308,6 +308,8 @@ func newLambdaCluster(t *testing.T) (*Calcium, []*types.Node) { engine.On("ImageLocalDigests", mock.Anything, mock.Anything).Return([]string{""}, nil) engine.On("ImageRemoteDigest", mock.Anything, mock.Anything).Return("", nil) + engine.On("VirtualizationCreateID", mock.Anything).Unset() + engine.On("VirtualizationCreateID", mock.Anything).Return("workloadfortonictest").Maybe() engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(&enginetypes.VirtualizationCreated{ID: "workloadfortonictest"}, nil) engine.On("VirtualizationStart", mock.Anything, mock.Anything).Return(nil) engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) diff --git a/cluster/calcium/remove.go b/cluster/calcium/remove.go index 15782657..22950786 100644 --- a/cluster/calcium/remove.go +++ b/cluster/calcium/remove.go @@ -41,12 +41,15 @@ func (c *Calcium) doRemoveOneWorkload(ctx context.Context, workload *types.Workl if err != nil { return err } - defer workloadCommit() - return c.doRemoveWorkload(ctx, workload, force) + settled, err := c.doRemoveWorkload(ctx, workload, force) + if settled { + workloadCommit() + } + return err } -func (c *Calcium) doRemoveWorkload(ctx context.Context, workload *types.Workload, force bool) error { - _, err := utils.Txn( +func (c *Calcium) doRemoveWorkload(ctx context.Context, workload *types.Workload, force bool) (bool, error) { + return utils.Txn( ctx, func(ctx context.Context) error { return c.store.RemoveWorkload(ctx, workload) @@ -56,13 +59,15 @@ func (c *Calcium) doRemoveWorkload(ctx context.Context, workload *types.Workload }, func(ctx context.Context, failedByCond bool) error { if failedByCond { - return nil + stored, err := getWorkloadIfExists(ctx, c, workload.ID) + if err != nil || stored != nil { + return err + } } return c.store.AddWorkload(ctx, workload, nil) }, c.config.GlobalTimeout, ) - return err } func (c *Calcium) doRemoveWorkloadSync(ctx context.Context, IDs []string) error { diff --git a/cluster/calcium/remove_test.go b/cluster/calcium/remove_test.go index e1fe3aeb..f91211ad 100644 --- a/cluster/calcium/remove_test.go +++ b/cluster/calcium/remove_test.go @@ -2,19 +2,26 @@ package calcium import ( "context" + "fmt" "sync" "testing" "testing/synctest" + "time" + "github.com/alicebob/miniredis/v2" "github.com/cockroachdb/errors" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/projecteru2/core/engine/factory" enginemocks "github.com/projecteru2/core/engine/mocks" lockmocks "github.com/projecteru2/core/lock/mocks" resourcemocks "github.com/projecteru2/core/resource/mocks" resourcetypes "github.com/projecteru2/core/resource/types" + "github.com/projecteru2/core/store" storemocks "github.com/projecteru2/core/store/mocks" + redisstore "github.com/projecteru2/core/store/redis" "github.com/projecteru2/core/types" "github.com/projecteru2/core/wal" walmocks "github.com/projecteru2/core/wal/mocks" @@ -278,3 +285,179 @@ func TestRemoveWorkloadLocksTheWorkloadThenItsNode(t *testing.T) { assert.Equal(t, []string{"clock_w1", "cnode_op_p1_n1"}, keys[:2], "the workload lock first, then the node lock around the release, never the pod lock") assert.NotContains(t, keys, "plock_p1") } + +func TestRemoveWorkloadCommitsOnlyAfterTheOutcomeSettles(t *testing.T) { + tests := []struct { + name string + deleted bool + readErr error + restoreErr error + engineErr error + wantCommit bool + }{ + {name: "delete never applied", wantCommit: true}, + {name: "delete applied but reply lost", deleted: true, wantCommit: true}, + {name: "delete outcome still unknown", readErr: types.ErrMockError}, + {name: "delete applied but restoration fails", deleted: true, restoreErr: types.ErrMockError}, + {name: "engine and restoration fail", engineErr: types.ErrMockError, restoreErr: types.ErrMockError}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewTestCluster() + t.Cleanup(c.Finalizer) + engine := &enginemocks.API{} + workload := &types.Workload{ID: "w1", Name: "app_web_123456", Nodename: "n1", Engine: engine} + committed := false + mwal := &walmocks.WAL{} + mwal.On("Log", eventWorkloadCreated, mock.Anything).Return(wal.Commit(func() error { committed = true; return nil }), nil).Once() + mwal.On("Close").Return(nil).Maybe() + c.wal = mwal + store := c.store.(*storemocks.Store) + if tt.engineErr != nil { + store.On("RemoveWorkload", mock.Anything, workload).Return(nil).Once() + engine.On("VirtualizationRemove", mock.Anything, workload.ID, true, true).Return(tt.engineErr).Once() + store.On("AddWorkload", mock.Anything, workload, mock.Anything).Return(tt.restoreErr).Once() + } else { + store.On("RemoveWorkload", mock.Anything, workload).Return(context.DeadlineExceeded).Once() + switch { + case tt.readErr != nil: + store.On("GetWorkload", mock.Anything, workload.ID).Return(nil, tt.readErr).Once() + store.On("NotFound", tt.readErr).Return(false).Once() + case tt.deleted: + store.On("GetWorkload", mock.Anything, workload.ID).Return(nil, types.ErrKeyNotFound).Once() + store.On("NotFound", types.ErrKeyNotFound).Return(true).Once() + store.On("AddWorkload", mock.Anything, workload, mock.Anything).Return(tt.restoreErr).Once() + default: + store.On("GetWorkload", mock.Anything, workload.ID).Return(workload, nil).Once() + } + } + require.Error(t, c.doRemoveOneWorkload(t.Context(), workload, true)) + assert.Equal(t, tt.wantCommit, committed) + store.AssertExpectations(t) + engine.AssertExpectations(t) + mwal.AssertExpectations(t) + }) + } +} + +func TestRemoveWorkloadRestoresMetadataAfterDeleteReplyLoss(t *testing.T) { + c := NewTestCluster() + t.Cleanup(c.Finalizer) + srv := miniredis.RunT(t) + c.config.Redis.Addr = srv.Addr() + c.config.ConnectionTimeout = time.Second + factory.InitEngineCache(t.Context(), c.config, nil) + backend, err := redisstore.New(c.config) + require.NoError(t, err) + ctx := t.Context() + _, err = backend.AddPod(ctx, "p1", "") + require.NoError(t, err) + node, err := backend.AddNode(ctx, &types.AddNodeOptions{Podname: "p1", Nodename: "n1", Endpoint: "mock://n1"}) + require.NoError(t, err) + engine := &enginemocks.API{} + workload := &types.Workload{ID: "w1", Name: "app_web_123456", Podname: "p1", Nodename: node.Name, Engine: engine} + require.NoError(t, backend.AddWorkload(ctx, workload, nil)) + c.store = &deleteReplyLostStore{Store: backend} + c.wal, err = enableWAL(ctx, c.config, c, backend) + require.NoError(t, err) + + require.ErrorIs(t, c.doRemoveOneWorkload(ctx, workload, true), context.DeadlineExceeded) + restored, err := backend.GetWorkload(ctx, workload.ID) + require.NoError(t, err) + require.Equal(t, workload.ID, restored.ID) + entries, err := backend.GetPrefix(ctx, "/wal/", 0) + require.NoError(t, err) + require.Empty(t, entries) + engine.AssertNotCalled(t, "VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + + engine.On("VirtualizationRemove", mock.Anything, workload.ID, true, true).Return(nil).Once() + require.NoError(t, c.doRemoveOneWorkload(ctx, workload, true)) + _, err = backend.GetWorkload(ctx, workload.ID) + require.ErrorIs(t, err, types.ErrWorkloadNotExists) + engine.AssertExpectations(t) +} + +func TestReplaceRecoveryRemovesTheEngineAfterUncertainStoreDeletion(t *testing.T) { + for _, newPresent := range []bool{false, true} { + t.Run(fmt.Sprintf("new_present_%t", newPresent), func(t *testing.T) { + c := NewTestCluster() + t.Cleanup(c.Finalizer) + srv := miniredis.RunT(t) + c.config.Redis.Addr = srv.Addr() + c.config.ConnectionTimeout = time.Second + factory.InitEngineCache(t.Context(), c.config, nil) + backend, err := redisstore.New(c.config) + require.NoError(t, err) + ctx := t.Context() + _, err = backend.AddPod(ctx, "p1", "") + require.NoError(t, err) + node, err := backend.AddNode(ctx, &types.AddNodeOptions{Podname: "p1", Nodename: "n1", Endpoint: "mock://n1"}) + require.NoError(t, err) + engine := &enginemocks.API{} + node.Engine = engine + workload := &types.Workload{ID: "old", Name: "app_web_old", Podname: "p1", Nodename: node.Name, Engine: engine} + require.NoError(t, backend.AddWorkload(ctx, workload, nil)) + if newPresent { + require.NoError(t, backend.AddWorkload(ctx, &types.Workload{ID: "new", Name: "app_web_new", Podname: "p1", Nodename: node.Name}, nil)) + } + faulty := &deleteReplyLostStore{Store: backend, readFailure: true, node: node} + c.store = faulty + c.rmgr.(*resourcemocks.Manager).On("GetNodeResourceInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(resourcetypes.Resources{}, resourcetypes.Resources{}, []string{}, nil) + c.wal, err = enableWAL(ctx, c.config, c, backend) + require.NoError(t, err) + _, err = c.wal.Log(eventWorkloadReplaced, &workloadReplacement{OldID: workload.ID, NewID: "new", Nodename: node.Name}) + require.NoError(t, err) + + settled, err := c.doRemoveWorkload(ctx, workload, true) + require.ErrorIs(t, err, context.DeadlineExceeded) + require.False(t, settled) + _, err = backend.GetWorkload(ctx, workload.ID) + require.ErrorIs(t, err, types.ErrWorkloadNotExists) + engine.AssertNotCalled(t, "VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + entries, err := backend.GetPrefix(ctx, "/wal/", 0) + require.NoError(t, err) + require.Len(t, entries, 1) + + faulty.readFailure = false + engine.On("VirtualizationRemove", mock.Anything, workload.ID, true, true).Return(nil).Once() + c.wal.Recover(ctx) + entries, err = backend.GetPrefix(ctx, "/wal/", 0) + require.NoError(t, err) + require.Empty(t, entries) + c.wal.Recover(ctx) + engine.AssertExpectations(t) + }) + } +} + +type deleteReplyLostStore struct { + store.Store + failed bool + readFailure bool + node *types.Node +} + +func (s *deleteReplyLostStore) RemoveWorkload(ctx context.Context, workload *types.Workload) error { + if err := s.Store.RemoveWorkload(ctx, workload); err != nil { + return err + } + if !s.failed { + s.failed = true + return context.DeadlineExceeded + } + return nil +} + +func (s *deleteReplyLostStore) GetWorkload(ctx context.Context, ID string) (*types.Workload, error) { + if s.failed && s.readFailure { + return nil, context.DeadlineExceeded + } + return s.Store.GetWorkload(ctx, ID) +} + +func (s *deleteReplyLostStore) GetNode(ctx context.Context, name string) (*types.Node, error) { + if s.node != nil && s.node.Name == name { + return s.node, nil + } + return s.Store.GetNode(ctx, name) +} diff --git a/cluster/calcium/replace.go b/cluster/calcium/replace.go index 6855cce5..0576cbdf 100644 --- a/cluster/calcium/replace.go +++ b/cluster/calcium/replace.go @@ -133,11 +133,11 @@ func (c *Calcium) doReplaceWorkload(ctx context.Context, workload *types.Workloa return c.doDeployOneWorkload(ctx, node, &opts.DeployOptions, createMessage, vco, false) }, func(ctx context.Context) (err error) { - commit, err := c.journal(ctx, logger, eventWorkloadReplaced, &workloadReplacement{OldID: workload.ID, NewID: createMessage.WorkloadID}) + commit, err := c.journal(ctx, logger, eventWorkloadReplaced, &workloadReplacement{OldID: workload.ID, NewID: createMessage.WorkloadID, Nodename: workload.Nodename}) if err != nil { return err } - if err = c.doRemoveWorkload(ctx, workload, true); err != nil { + if _, err = c.doRemoveWorkload(ctx, workload, true); err != nil { logger.Error(ctx, err, "the new started but the old failed to stop") return err } diff --git a/cluster/calcium/replace_test.go b/cluster/calcium/replace_test.go index 471c17ce..0589b817 100644 --- a/cluster/calcium/replace_test.go +++ b/cluster/calcium/replace_test.go @@ -83,6 +83,7 @@ func TestReplaceWorkload(t *testing.T) { store.AssertExpectations(t) engine := &enginemocks.API{} + engine.On("VirtualizationCreateID", mock.Anything).Return("new").Maybe() workload.Engine = engine store.On("GetWorkload", mock.Anything, mock.Anything).Return(workload, nil) engine.On("VirtualizationInspect", mock.Anything, mock.Anything).Return(&enginetypes.VirtualizationInfo{Running: false}, nil).Once() @@ -159,6 +160,7 @@ func TestReplaceWorkload(t *testing.T) { engine.On("VirtualizationStop", mock.Anything, mock.Anything, mock.Anything).Return(nil) engine.On("VirtualizationRemove", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) + store.On("RemoveWorkload", mock.Anything, mock.Anything).Return(nil).Once() engine.On("VirtualizationCreate", mock.Anything, mock.Anything).Return(nil, types.ErrMockError).Once() engine.On("VirtualizationStart", mock.Anything, mock.Anything).Return(types.ErrMockError).Once() ch, err = c.ReplaceWorkload(ctx, opts) diff --git a/cluster/calcium/wal.go b/cluster/calcium/wal.go index c98d3ff6..9425207a 100644 --- a/cluster/calcium/wal.go +++ b/cluster/calcium/wal.go @@ -115,17 +115,7 @@ func (h *CreateWorkloadHandler) Handle(ctx context.Context, raw any) error { return h.calcium.doRemoveWorkloadSync(ctx, []string{storedID}) } - node, err := h.calcium.GetNode(ctx, wrk.Nodename) - if err != nil { - if h.calcium.store.NotFound(err) { - logger.Info(ctx, "node is gone, nothing to remove") - return nil - } - logger.Error(ctx, err) - return err - } - - if err = h.removeFromEngine(ctx, node, wrk); err != nil { + if err = removeEngineWorkload(ctx, h.calcium, wrk); err != nil { logger.Error(ctx, err) return err } @@ -154,19 +144,10 @@ func (h *CreateWorkloadHandler) storedWorkloadID(ctx context.Context, wrk *types return workloads[index].ID, nil } -func (h *CreateWorkloadHandler) removeFromEngine(ctx context.Context, node *types.Node, wrk *types.Workload) error { - if wrk.ID == "" { - return removeWorkloadByName(ctx, node, wrk.Name) - } - if err := node.Engine.VirtualizationRemove(ctx, wrk.ID, true, true); err != nil && !errors.Is(err, types.ErrWorkloadNotExists) { - return err - } - return nil -} - type workloadReplacement struct { - OldID string `json:"old_id"` - NewID string `json:"new_id"` + OldID string `json:"old_id"` + NewID string `json:"new_id"` + Nodename string `json:"nodename,omitempty"` } // ReplaceWorkloadHandler removes the workload an interrupted replace left behind. @@ -187,17 +168,22 @@ func (h *ReplaceWorkloadHandler) Handle(ctx context.Context, raw any) error { ctx, cancel := getReplayContext(ctx) defer cancel() - newWorkload, err := getWorkloadIfExists(ctx, h.calcium, replacement.NewID) - if err != nil || newWorkload == nil { + oldWorkload, err := getWorkloadIfExists(ctx, h.calcium, replacement.OldID) + if err != nil { return err } - - oldWorkload, err := getWorkloadIfExists(ctx, h.calcium, replacement.OldID) - if err != nil || oldWorkload == nil { + if oldWorkload == nil { + if replacement.Nodename == "" { + return nil + } + return removeEngineWorkload(ctx, h.calcium, &types.Workload{ID: replacement.OldID, Nodename: replacement.Nodename}) + } + newWorkload, err := getWorkloadIfExists(ctx, h.calcium, replacement.NewID) + if err != nil || newWorkload == nil { return err } - if err = h.calcium.doRemoveWorkload(ctx, oldWorkload, true); err != nil { + if _, err = h.calcium.doRemoveWorkload(ctx, oldWorkload, true); err != nil { logger.Error(ctx, err) return err } @@ -369,6 +355,23 @@ func enableWAL(ctx context.Context, config types.Config, calcium *Calcium, store return hydro, nil } +func removeEngineWorkload(ctx context.Context, c *Calcium, wrk *types.Workload) error { + node, err := c.GetNode(ctx, wrk.Nodename) + if err != nil { + if c.store.NotFound(err) { + return nil + } + return err + } + if wrk.ID == "" { + return removeWorkloadByName(ctx, node, wrk.Name) + } + if err = node.Engine.VirtualizationRemove(ctx, wrk.ID, true, true); err != nil && !errors.Is(err, types.ErrWorkloadNotExists) { + return err + } + return nil +} + func getReplayContext(ctx context.Context) (context.Context, context.CancelFunc) { return context.WithTimeout(ctx, replayTimeout) } diff --git a/cluster/calcium/wal_test.go b/cluster/calcium/wal_test.go index 2a10d463..7859a16e 100644 --- a/cluster/calcium/wal_test.go +++ b/cluster/calcium/wal_test.go @@ -268,6 +268,7 @@ func TestHandleReplaceWorkloadKeepsTheOldOneWhenTheNewOneIsGone(t *testing.T) { require.NoError(t, err) store := c.store.(*storemocks.Store) + store.On("GetWorkload", mock.Anything, "old").Return(&types.Workload{ID: "old"}, nil).Once() store.On("GetWorkload", mock.Anything, "new").Return(nil, types.ErrMockError).Once() store.On("NotFound", types.ErrMockError).Return(true).Once() diff --git a/docs/engines.md b/docs/engines.md index d9666439..75e1737b 100644 --- a/docs/engines.md +++ b/docs/engines.md @@ -249,15 +249,16 @@ two, and is dominated by cocoon's own work; start and resume add a second round rewrite. The VM's cocoon name is the workload id — a 32-hex id core generates, exactly as for a process -workload — so every later verb runs `cocoon vm ` without a lookup, and eru-agent keys +workload, reserved before the create intent is journalled — so every later verb runs +`cocoon vm ` without a lookup, and eru-agent keys the cocoon daemon's events on it. The eru name stays in the meta file and in core's store. | `engine.API` | cocoon | | --- | --- | | `VirtualizationCreate` | `vm create --output json --name [--cpu N] [--memory B] [--storage B] [--data-disk …] [--network ] [--windows \| --user U] ` — no boot; then the meta record is written. A failure after the create removes the VM again. cocoon has no way to apply the deploy's `env`, `dns`, `extra_hosts` or the entrypoint's `commands` and `dir` to a guest, so any of them set draws a warning; core's own `APP_NAME`/`ERU_*` env does not | | `VirtualizationStart` | `vm inspect`, `vm start` and `vm inspect` again in one script; the second inspect reports this boot's `console_path`, and both copies of the meta record are rewritten with it and with the VMM pid. An inspect that reports no console keeps the serial socket path. A Windows guest on its first boot gets its address programmed through `vm exec` in the background, after the start has already returned | -| `VirtualizationStop` | `vm stop`, `--force` for a forced stop, `--timeout` when a grace period is given; a workload with no record on the node is `ErrWorkloadNotExists`, as for the other verbs. cocoon's stop is idempotent, so stopping a created or already-stopped guest succeeds | -| `VirtualizationRemove` | `vm rm [--force]`, then the hibernate snapshot and both copies of the meta record; a running guest is refused unless forced | +| `VirtualizationStop` | `vm stop`, `--force` for a forced stop, `--timeout` when a grace period is given; a workload with no record on the node is `ErrWorkloadNotExists`, as for start and inspect. cocoon's stop is idempotent, so stopping a created or already-stopped guest succeeds | +| `VirtualizationRemove` | `vm rm [--force]`, then the hibernate snapshot and both copies of the meta record, even when creation never wrote a record; a running guest is refused unless forced | | `VirtualizationSuspend` / `Resume` | `vm hibernate --name eru-` / `vm restore --restore-mode copy` followed by `snapshot rm` and `vm inspect`, then the record rewrite as at start. The restore copies, so the delete is best-effort: a snapshot that will not go leaves garbage on the node rather than aborting a resume whose guest is already running | | `VirtualizationInspect` | the stored record then `vm inspect`: running when the state is `running`, the image, the CNI address under the network's name, and the deploy's `user` — cocoon's own JSON has no eru user, and returning an empty one made core overwrite the stored value after every start | | `VirtualizationWait` | `vm status --event --format json` until the guest leaves `running`; a VM has no exit code, so the result is 0 | diff --git a/docs/operations.md b/docs/operations.md index 194a347c..ae383b67 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -15,13 +15,20 @@ zero-padded, so replaying the keys in order replays the entries in order. | Event | Written before | Replay does | | --- | --- | --- | | `allocate-workload` | resources are allocated on a set of nodes, a workload is removed, or a realloc starts | re-derives each node's usage from its actual workloads (`NodeResource` with `fix`); a repair whose write fails keeps the entry for the next replay | -| `create-workload` | the engine is asked to create a workload, and before one is removed | removes the workload — from the store if it is there, otherwise off the engine, found by name when the entry has no ID yet | -| `replace-workload` | the old workload of a replace is removed | removes the old workload if the new one reached the store, releasing nothing, because the new one inherited its resources | +| `create-workload` | the engine is asked to create a workload, and before one is removed | removes the workload — from the store if it is there, otherwise off the engine by its reserved ID; legacy entries without an ID retain name lookup | +| `replace-workload` | the old workload of a replace is removed | removes the old workload if the new one reached the store; when old metadata is already absent, uses the recorded node to finish engine cleanup independently of the new workload; releases nothing because resources were inherited | | `realloc-workload` | a realloc mutates plugin usage, metadata and engine limits | re-applies the stored engine params under the node-operation lock — the same step a failed realloc runs inline as its repair, committing the entry only when the reapply holds | | `remap-node` | a node's engine params are reapplied to its workloads | recomputes the remap and reapplies it under the node-operation lock | | `create-processing` | an in-flight deploy counter is written | deletes the stale counter, so it stops inflating deploy counts forever | | `create-lambda` | a `RunAndWait` workload starts | waits for it to exit, then removes it | +Create reserves the engine ID before writing its intent, so recovery can remove a process or VM +even when the create reply or the engine's metadata write never arrives. This does not identify +orphans left by older name-only entries whose randomly generated engine IDs were never stored, +or older replacement entries that lost the old metadata without recording its node. +A failed store deletion is read back and, if needed, restored before its removal intent is +committed; failed read-back or restoration leaves that intent for replay. + Each replayed handler gets a 32-second deadline. Entries whose handler is unknown are logged and skipped; entries that fail are logged and left in place for the next start. diff --git a/engine/cocoon/create.go b/engine/cocoon/create.go index d0d2402a..1ea99704 100644 --- a/engine/cocoon/create.go +++ b/engine/cocoon/create.go @@ -49,6 +49,10 @@ type RawArgs struct { OS string `json:"os"` // "windows" boots a Windows guest } +func (e *Engine) VirtualizationCreateID(string) string { + return utils.RandomID() +} + func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.VirtualizationCreateOptions) (*enginetypes.VirtualizationCreated, error) { logger := log.WithFunc("engine.cocoon.VirtualizationCreate") resource := &engine.VirtualizationResource{} @@ -66,7 +70,7 @@ func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.Vir if err != nil { return nil, err } - ID := utils.RandomID() + ID := opts.ID argv, err := createArgv(e.cocoon.Binary, ID, opts, resource, rArgs.OS == osWindows, network) if err != nil { return nil, err @@ -100,7 +104,6 @@ func (e *Engine) record(ctx context.Context, ID string, opts *enginetypes.Virtua return err } -// discard removes a VM whose eru record never landed; core only knows the ones that did. func (e *Engine) discard(ctx context.Context, ID string) { ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), discardTimeout) defer cancel() diff --git a/engine/cocoon/create_test.go b/engine/cocoon/create_test.go index 4cc9f235..952babea 100644 --- a/engine/cocoon/create_test.go +++ b/engine/cocoon/create_test.go @@ -17,11 +17,13 @@ import ( coretypes "github.com/projecteru2/core/types" ) -func TestVirtualizationCreateRendersTheVMAndRecordsIt(t *testing.T) { +func TestVirtualizationCreateRendersTheReservedIDAndRecordsIt(t *testing.T) { runner := &sshrunnertest.Fake{Respond: createdVM} e := testEngine(t, runner) + ID := e.VirtualizationCreateID("app_web_xyz") created, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{ + ID: ID, Name: "app_web_xyz", Image: testImage, User: testUser, @@ -33,6 +35,9 @@ func TestVirtualizationCreateRendersTheVMAndRecordsIt(t *testing.T) { if err != nil { t.Fatalf("create: %v", err) } + if created.ID != ID { + t.Fatalf("got id %q, want reserved id %q", created.ID, ID) + } if len(created.ID) != testIDLen { t.Fatalf("got id %q, want a %d-hex id", created.ID, testIDLen) } @@ -81,7 +86,7 @@ func TestVirtualizationCreateDiscardsAVMWhoseRecordFailed(t *testing.T) { }} e := testEngine(t, runner) - if _, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{Name: "app_web_xyz", Image: testImage}); err == nil { + if _, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{ID: e.VirtualizationCreateID("app_web_xyz"), Name: "app_web_xyz", Image: testImage}); err == nil { t.Fatal("a failed record must fail the create") } lines := runner.Lines() @@ -96,7 +101,7 @@ func TestVirtualizationCreateDiscardsAVMWhoseDeadlineExpired(t *testing.T) { runner := &sshrunnertest.Fake{Respond: createdVMThenCanceled(cancel)} e := testEngine(t, runner) - if _, err := e.VirtualizationCreate(ctx, &enginetypes.VirtualizationCreateOptions{Name: "app_web_xyz", Image: testImage}); err == nil { + if _, err := e.VirtualizationCreate(ctx, &enginetypes.VirtualizationCreateOptions{ID: e.VirtualizationCreateID("app_web_xyz"), Name: "app_web_xyz", Image: testImage}); err == nil { t.Fatal("a record core could not write must fail the create") } lines := runner.Lines() @@ -110,6 +115,7 @@ func TestVirtualizationCreateKeepsTheConflistOfAnInheritedNetwork(t *testing.T) e := testEngine(t, runner) if _, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{ + ID: e.VirtualizationCreateID("app_web_xyz"), Name: "app_web_xyz", Image: testImage, Networks: map[string]string{"eru-cni": "10.22.0.9"}, diff --git a/engine/cocoon/lifecycle.go b/engine/cocoon/lifecycle.go index 6a78a780..04771cbd 100644 --- a/engine/cocoon/lifecycle.go +++ b/engine/cocoon/lifecycle.go @@ -51,7 +51,6 @@ done ` removeScript = `bin=$1; vm=$2; durable=$3; record=$4; snap=$5; force=$6 -test -f "$durable" || exit 64 set -- if [ "$force" = 1 ]; then set -- --force; fi if ! out=$("$bin" vm rm "$@" "$vm" 2>&1) && "$bin" vm inspect "$vm" >/dev/null 2>&1; then @@ -125,8 +124,7 @@ func (e *Engine) VirtualizationStop(ctx context.Context, ID string, gracefulTime } func (e *Engine) VirtualizationRemove(ctx context.Context, ID string, _, force bool) error { - argv := sshrunner.Shell(removeScript, e.cocoon.Binary, ID, durablePath(e.cocoon.Root, ID), workloadmeta.Path(ID), snapshotName(ID), strconv.Itoa(utils.Bool2Int(force))) - _, err := e.runRecorded(ctx, argv, ID) + _, err := e.run(ctx, sshrunner.Shell(removeScript, e.cocoon.Binary, ID, durablePath(e.cocoon.Root, ID), workloadmeta.Path(ID), snapshotName(ID), strconv.Itoa(utils.Bool2Int(force)))...) return err } diff --git a/engine/cocoon/lifecycle_test.go b/engine/cocoon/lifecycle_test.go index 7c982850..21169543 100644 --- a/engine/cocoon/lifecycle_test.go +++ b/engine/cocoon/lifecycle_test.go @@ -104,7 +104,6 @@ func TestVirtualizationReportsAMissingWorkload(t *testing.T) { }{ {"start", func(e *Engine) error { return e.VirtualizationStart(t.Context(), "w1") }}, {"stop", func(e *Engine) error { return e.VirtualizationStop(t.Context(), "w1", 0) }}, - {"remove", func(e *Engine) error { return e.VirtualizationRemove(t.Context(), "w1", true, false) }}, {"inspect", func(e *Engine) error { _, err := e.VirtualizationInspect(t.Context(), "w1"); return err }}, } for _, tt := range tests { diff --git a/engine/cocoon/scripts_test.go b/engine/cocoon/scripts_test.go index 0ac8c169..6f1149d9 100644 --- a/engine/cocoon/scripts_test.go +++ b/engine/cocoon/scripts_test.go @@ -320,9 +320,13 @@ func TestRemoveScriptDropsTheVMAndBothRecords(t *testing.T) { }, }, { - name: "a vm the node lost", - force: "1", - wantCode: workloadmeta.NotExistsCode, + name: "a vm created before its eru record was written", + force: "1", + wantCalls: []string{ + "cocoon vm rm --force " + scriptVM, + "cocoon snapshot rm " + scriptSnap, + }, + wantGone: true, }, } for _, tt := range tests { diff --git a/engine/containerd/create.go b/engine/containerd/create.go index 4ba1992d..550ae26f 100644 --- a/engine/containerd/create.go +++ b/engine/containerd/create.go @@ -75,6 +75,10 @@ type containerUpdater interface { Update(ctx context.Context, opts ...client.UpdateContainerOpts) error } +func (e *Engine) VirtualizationCreateID(name string) string { + return name +} + func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.VirtualizationCreateOptions) (*enginetypes.VirtualizationCreated, error) { logger := log.WithFunc("engine.containerd.VirtualizationCreate") resource := &engine.VirtualizationResource{} @@ -93,7 +97,7 @@ func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.Vir } // The container id is the workload name: containerd carries no name eru-agent could read - ID := opts.Name + ID := opts.ID if err := identifiers.Validate(ID); err != nil { return nil, errors.Wrapf(coretypes.ErrInvalidWorkloadName, "containerd cannot name %q", ID) } diff --git a/engine/engine.go b/engine/engine.go index 87c1fe06..802a5903 100644 --- a/engine/engine.go +++ b/engine/engine.go @@ -39,6 +39,7 @@ type API interface { BuildRefs(ctx context.Context, opts *enginetypes.BuildRefOptions) []string BuildContent(ctx context.Context, scm coresource.Source, opts *enginetypes.BuildContentOptions) (string, io.Reader, error) + VirtualizationCreateID(name string) string VirtualizationCreate(ctx context.Context, opts *enginetypes.VirtualizationCreateOptions) (*enginetypes.VirtualizationCreated, error) VirtualizationCopyChunkTo(ctx context.Context, ID, target string, size int64, content io.Reader, uid, gid int, mode int64) error VirtualizationStart(ctx context.Context, ID string) error diff --git a/engine/fake/fake.go b/engine/fake/fake.go index df04f212..1683626c 100644 --- a/engine/fake/fake.go +++ b/engine/fake/fake.go @@ -108,6 +108,10 @@ func (f *EngineWithErr) BuildContent(context.Context, coresource.Source, *engine return "", nil, f.DefaultErr } +func (f *EngineWithErr) VirtualizationCreateID(name string) string { + return name +} + func (f *EngineWithErr) VirtualizationCreate(context.Context, *enginetypes.VirtualizationCreateOptions) (*enginetypes.VirtualizationCreated, error) { return nil, f.DefaultErr } diff --git a/engine/fakeengine/fake.go b/engine/fakeengine/fake.go index 20247093..d453ab8a 100644 --- a/engine/fakeengine/fake.go +++ b/engine/fakeengine/fake.go @@ -110,6 +110,10 @@ func (e *Engine) BuildContent(context.Context, coresource.Source, *enginetypes.B return "BuildContent", stream("this is content"), nil } +func (e *Engine) VirtualizationCreateID(string) string { + return utils.RandomString(64) +} + func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.VirtualizationCreateOptions) (*enginetypes.VirtualizationCreated, error) { logger := log.WithFunc("engine.fakeengine.VirtualizationCreate") resourceOpts := &engine.VirtualizationResource{} @@ -117,7 +121,7 @@ func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.Vir logger.Error(ctx, err, "decode engine params") } logger.Debugf(ctx, "resources %+v", resourceOpts) - return &enginetypes.VirtualizationCreated{ID: utils.RandomString(64), Name: "mock-test-cvm" + utils.RandomString(6)}, nil + return &enginetypes.VirtualizationCreated{ID: opts.ID, Name: "mock-test-cvm" + utils.RandomString(6)}, nil } func (e *Engine) VirtualizationCopyChunkTo(context.Context, string, string, int64, io.Reader, int, int, int64) error { diff --git a/engine/mocks/API.go b/engine/mocks/API.go index cb3ce17b..490790a5 100644 --- a/engine/mocks/API.go +++ b/engine/mocks/API.go @@ -2011,6 +2011,57 @@ func (_c *API_VirtualizationCreate_Call) RunAndReturn(run func(ctx context.Conte return _c } +// VirtualizationCreateID provides a mock function for the type API +func (_mock *API) VirtualizationCreateID(name string) string { + ret := _mock.Called(name) + + if len(ret) == 0 { + panic("no return value specified for VirtualizationCreateID") + } + + var r0 string + if returnFunc, ok := ret.Get(0).(func(string) string); ok { + r0 = returnFunc(name) + } else { + r0 = ret.Get(0).(string) + } + return r0 +} + +// API_VirtualizationCreateID_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'VirtualizationCreateID' +type API_VirtualizationCreateID_Call struct { + *mock.Call +} + +// VirtualizationCreateID is a helper method to define mock.On call +// - name string +func (_e *API_Expecter) VirtualizationCreateID(name any) *API_VirtualizationCreateID_Call { + return &API_VirtualizationCreateID_Call{Call: _e.mock.On("VirtualizationCreateID", name)} +} + +func (_c *API_VirtualizationCreateID_Call) Run(run func(name string)) *API_VirtualizationCreateID_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 string + if args[0] != nil { + arg0 = args[0].(string) + } + run( + arg0, + ) + }) + return _c +} + +func (_c *API_VirtualizationCreateID_Call) Return(s string) *API_VirtualizationCreateID_Call { + _c.Call.Return(s) + return _c +} + +func (_c *API_VirtualizationCreateID_Call) RunAndReturn(run func(name string) string) *API_VirtualizationCreateID_Call { + _c.Call.Return(run) + return _c +} + // VirtualizationInspect provides a mock function for the type API func (_mock *API) VirtualizationInspect(ctx context.Context, ID string) (*types.VirtualizationInfo, error) { ret := _mock.Called(ctx, ID) diff --git a/engine/process/create.go b/engine/process/create.go index 8d8cc7d8..0e530243 100644 --- a/engine/process/create.go +++ b/engine/process/create.go @@ -54,6 +54,10 @@ type RawArgs struct { TasksMax int `json:"tasks_max"` // cgroup v2 pids.max } +func (e *Engine) VirtualizationCreateID(string) string { + return utils.RandomID() +} + func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.VirtualizationCreateOptions) (*enginetypes.VirtualizationCreated, error) { logger := log.WithFunc("engine.process.VirtualizationCreate") resource := &engine.VirtualizationResource{} @@ -73,7 +77,7 @@ func (e *Engine) VirtualizationCreate(ctx context.Context, opts *enginetypes.Vir return nil, errors.Wrapf(coretypes.ErrInvalidEngineArgs, "pod %q cannot name a systemd slice", podname) } - ID := utils.RandomID() + ID := opts.ID dir := workloadDir(e.root, ID) u := &unit{ ID: ID, diff --git a/engine/process/lifecycle_test.go b/engine/process/lifecycle_test.go index 77361154..e385892a 100644 --- a/engine/process/lifecycle_test.go +++ b/engine/process/lifecycle_test.go @@ -54,8 +54,10 @@ exit 0 func TestVirtualizationCreateRecordsTheUnitAndTheMetaFile(t *testing.T) { runner := &sshrunnertest.Fake{} e := testEngine(t, runner) + ID := e.VirtualizationCreateID("app_web_xyz") created, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{ + ID: ID, Name: "app_web_xyz", Image: "hub.io/ns/app:v1", Env: []string{"ERU_POD=prod"}, @@ -64,6 +66,9 @@ func TestVirtualizationCreateRecordsTheUnitAndTheMetaFile(t *testing.T) { if err != nil { t.Fatalf("create: %v", err) } + if created.ID != ID { + t.Fatalf("got id %q, want reserved id %q", created.ID, ID) + } if len(runner.Lines()) != 1 { t.Fatalf("got %d commands, want 1", len(runner.Lines())) } @@ -92,6 +97,7 @@ func TestVirtualizationCreateSkipsTheOverlayForARawWorkload(t *testing.T) { e := testEngine(t, runner) created, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{ + ID: e.VirtualizationCreateID("app_web_xyz"), Name: "app_web_xyz", Image: "hub.io/ns/app:v1", Env: []string{"ERU_POD=prod"}, @@ -118,6 +124,7 @@ func TestVirtualizationCreateRejectsAnUnusablePodname(t *testing.T) { e := testEngine(t, &sshrunnertest.Fake{}) _, err := e.VirtualizationCreate(t.Context(), &enginetypes.VirtualizationCreateOptions{ + ID: e.VirtualizationCreateID("app_web_xyz"), Name: "app_web_xyz", Image: "hub.io/ns/app:v1", Env: []string{"ERU_POD=bad/pod"}, diff --git a/engine/types/virtualization.go b/engine/types/virtualization.go index 2f154356..bdb8e348 100644 --- a/engine/types/virtualization.go +++ b/engine/types/virtualization.go @@ -10,6 +10,7 @@ const CPUPeriodBase = 100000 // VirtualizationCreateOptions describes a workload to create. type VirtualizationCreateOptions struct { EngineParams resourcetypes.Resources + ID string Name string User string Image string