Skip to content

Commit

Permalink
you can't remove a node with associated workloads now (#299)
Browse files Browse the repository at this point in the history
  • Loading branch information
tonicmuroq authored Dec 15, 2020
1 parent e385279 commit 976cabc
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 5 deletions.
7 changes: 7 additions & 0 deletions cluster/calcium/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ func (c *Calcium) AddNode(ctx context.Context, opts *types.AddNodeOptions) (*typ
// RemoveNode remove a node
func (c *Calcium) RemoveNode(ctx context.Context, nodename string) error {
return c.withNodeLocked(ctx, nodename, func(node *types.Node) error {
ws, err := c.ListNodeWorkloads(ctx, node.Name, nil)
if err != nil {
return err
}
if len(ws) > 0 {
return types.ErrNodeNotEmpty
}
return c.store.RemoveNode(ctx, node)
})
}
Expand Down
15 changes: 12 additions & 3 deletions cluster/calcium/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ func TestRemoveNode(t *testing.T) {
name := "test"
node := &types.Node{NodeMeta: types.NodeMeta{Name: name}}
store := &storemocks.Store{}
c.store = store

lock := &lockmocks.DistributedLock{}
lock.On("Lock", mock.Anything).Return(nil)
lock.On("Unlock", mock.Anything).Return(nil)
Expand All @@ -48,14 +50,21 @@ func TestRemoveNode(t *testing.T) {
store.On("GetNode",
mock.Anything,
mock.Anything).Return(node, nil)
// fail, ListNodeWorkloads fail
store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return([]*types.Workload{}, types.ErrNoETCD).Once()
assert.Error(t, c.RemoveNode(ctx, name))
// fail, node still has associated workloads
store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return([]*types.Workload{{}}, nil).Once()
assert.Error(t, c.RemoveNode(ctx, name))

// succeed
store.On("ListNodeWorkloads", mock.Anything, mock.Anything, mock.Anything).Return([]*types.Workload{}, nil)
store.On("RemoveNode", mock.Anything, mock.Anything).Return(nil)
pod := &types.Pod{Name: name}
store.On("GetPod", mock.Anything, mock.Anything).Return(pod, nil)
c.store = store

store.On("GetNodesByPod", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*types.Node{node}, nil)
err := c.RemoveNode(ctx, name)
assert.NoError(t, err)
assert.NoError(t, c.RemoveNode(ctx, name))
}

func TestListPodNodes(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ var (

ErrZeroNodes = errors.New("no nodes provide to choose some")

ErrNodeFormat = errors.New("bad endpoint name")
ErrNodeExist = errors.New("node already exists")
ErrNodeFormat = errors.New("bad endpoint name")
ErrNodeExist = errors.New("node already exists")
ErrNodeNotEmpty = errors.New("node not empty, still has workloads associated")

ErrKeyIsDir = errors.New("key is a directory")
ErrKeyIsNotDir = errors.New("key is not a directory")
Expand Down

0 comments on commit 976cabc

Please sign in to comment.