-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
remove deprecated Drain
field from structs.Node
#10202
Changes from 10 commits
93d5187
574d26c
ca51bd9
0cd707e
7d78ffb
bc90436
6043594
23c75b0
e0b5320
4e7d84e
c5731eb
a52f32d
405efe6
e9c552b
ed636ad
80066ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -178,6 +178,38 @@ func TestNodes_Info(t *testing.T) { | |
} | ||
} | ||
|
||
func TestNodes_NoSecretID(t *testing.T) { | ||
t.Parallel() | ||
c, s := makeClient(t, nil, func(c *testutil.TestServerConfig) { | ||
c.DevMode = true | ||
}) | ||
defer s.Stop() | ||
nodes := c.Nodes() | ||
|
||
// Get the node ID | ||
var nodeID string | ||
testutil.WaitForResult(func() (bool, error) { | ||
out, _, err := nodes.List(nil) | ||
if err != nil { | ||
return false, err | ||
} | ||
if n := len(out); n != 1 { | ||
return false, fmt.Errorf("expected 1 node, got: %d", n) | ||
} | ||
nodeID = out[0].ID | ||
return true, nil | ||
}, func(err error) { | ||
t.Fatalf("err: %s", err) | ||
}) | ||
|
||
// Query the node, ensure that .SecretID was not returned by the HTTP server | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doing a raw decoding here into a map to make sure that expected fields are present (i.e., I know how to decode) and unexpected fields ("SecretID") are not |
||
resp := make(map[string]interface{}) | ||
_, err := c.query("/v1/node/"+nodeID, &resp, nil) | ||
require.NoError(t, err) | ||
require.Equal(t, nodeID, resp["ID"]) | ||
require.Empty(t, resp["SecretID"]) | ||
} | ||
|
||
func TestNodes_ToggleDrain(t *testing.T) { | ||
t.Parallel() | ||
require := require.New(t) | ||
|
@@ -206,9 +238,7 @@ func TestNodes_ToggleDrain(t *testing.T) { | |
// Check for drain mode | ||
out, _, err := nodes.Info(nodeID, nil) | ||
require.Nil(err) | ||
if out.Drain { | ||
t.Fatalf("drain mode should be off") | ||
} | ||
require.False(out.Drain) | ||
|
||
// Toggle it on | ||
spec := &DrainSpec{ | ||
|
@@ -218,11 +248,36 @@ func TestNodes_ToggleDrain(t *testing.T) { | |
require.Nil(err) | ||
assertWriteMeta(t, &drainOut.WriteMeta) | ||
|
||
// Check again | ||
out, _, err = nodes.Info(nodeID, nil) | ||
require.Nil(err) | ||
if out.SchedulingEligibility != NodeSchedulingIneligible { | ||
t.Fatalf("bad eligibility: %v vs %v", out.SchedulingEligibility, NodeSchedulingIneligible) | ||
// Drain may have completed before we can check, use event stream | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. want to see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a nice pattern for testing... we can get a lot of mileage out of the event stream! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is awesome :D |
||
ctx, cancel := context.WithCancel(context.Background()) | ||
defer cancel() | ||
|
||
streamCh, err := c.EventStream().Stream(ctx, map[Topic][]string{ | ||
TopicNode: {nodeID}, | ||
}, 0, nil) | ||
require.NoError(err) | ||
|
||
// we expect to see the node change to Drain:true and then back to Drain:false+ineligible | ||
var sawDraining, sawDrainComplete uint64 | ||
for sawDrainComplete == 0 { | ||
select { | ||
case events := <-streamCh: | ||
require.NoError(events.Err) | ||
for _, e := range events.Events { | ||
node, err := e.Node() | ||
require.NoError(err) | ||
require.Equal(node.DrainStrategy != nil, node.Drain) | ||
require.True(!node.Drain || node.SchedulingEligibility == NodeSchedulingIneligible) // node.Drain => "ineligible" | ||
if node.Drain && node.SchedulingEligibility == NodeSchedulingIneligible { | ||
cgbaker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
sawDraining = node.ModifyIndex | ||
} else if sawDraining != 0 && node.ModifyIndex > sawDraining && | ||
!node.Drain && node.SchedulingEligibility == NodeSchedulingIneligible { | ||
sawDrainComplete = node.ModifyIndex | ||
} | ||
} | ||
case <-time.After(5 * time.Second): | ||
require.Fail("failed waiting for event stream event") | ||
} | ||
} | ||
|
||
// Toggle off again | ||
|
@@ -233,15 +288,9 @@ func TestNodes_ToggleDrain(t *testing.T) { | |
// Check again | ||
out, _, err = nodes.Info(nodeID, nil) | ||
require.Nil(err) | ||
if out.Drain { | ||
t.Fatalf("drain mode should be off") | ||
} | ||
if out.DrainStrategy != nil { | ||
t.Fatalf("drain strategy should be unset") | ||
} | ||
if out.SchedulingEligibility != NodeSchedulingEligible { | ||
t.Fatalf("should be eligible") | ||
} | ||
require.False(out.Drain) | ||
require.Nil(out.DrainStrategy) | ||
require.Equal(NodeSchedulingEligible, out.SchedulingEligibility) | ||
} | ||
|
||
func TestNodes_ToggleEligibility(t *testing.T) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doing a raw decoding here into a map to make sure that expected fields are present (i.e., I know how to decode) and unexpected fields ("SecretID") are not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind putting that in the comment as it's a very important implementation detail to maintain in this test, yet one that should rarely-if-ever be copied into other tests (which happens a lot).