luxolus/open-nomad
2
0
Fork 0
Code Issues 1 Pull requests Packages Releases Wiki Activity

add network resources equal method, use time ticker

Browse source 
remove impossible test case
This commit is contained in:
Chelsea Holland Komlo 2018-02-27 12:21:06 -05:00
parent e736e31820
commit a72aaaf47f
4 changed files with 209 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
client/client.go
View file

@ -212,7 +212,7 @@ func NewClient(cfg *config.Config, consulCatalog consul.CatalogAPI, consulServic
allocUpdates: make(chan *structs.Allocation, 64),
shutdownCh: make(chan struct{}),
triggerDiscoveryCh: make(chan struct{}),
triggerNodeUpdate: make(chan struct{}, 64),
triggerNodeUpdate: make(chan struct{}, 8),
serversDiscoveredCh: make(chan struct{}),
}
@ -1028,7 +1028,7 @@ func resourcesAreEqual(first, second *structs.Resources) bool {
return false
}
f := second.Networks[i]
if e != f {
if !e.Equals(f) {
return false
}
}
@ -1538,11 +1538,12 @@ func (c *Client) updateNode() {
// it will update the client node copy and re-register the node.
func (c *Client) watchNodeUpdates() {
var hasChanged bool
syncTicker := time.NewTicker(c.retryIntv(nodeUpdateRetryIntv))
timer := time.NewTimer(c.retryIntv(nodeUpdateRetryIntv))
defer timer.Stop()
for {
select {
case <-syncTicker.C:
case <-timer.C:
if !hasChanged {
continue
}
@ -1557,8 +1558,6 @@ func (c *Client) watchNodeUpdates() {
c.retryRegisterNode()
hasChanged = false
syncTicker.Stop()
syncTicker = time.NewTicker(c.retryIntv(nodeUpdateRetryIntv))
case <-c.triggerNodeUpdate:
hasChanged = true
case <-c.shutdownCh:

45
client/client_test.go
View file

@ -125,51 +125,6 @@ func TestClient_Fingerprint(t *testing.T) {
require.NotEqual("", node.Attributes["driver.mock_driver"])
}
func TestClient_TriggerNodeUpdate(t *testing.T) {
driver.CheckForMockDriver(t)
t.Parallel()
// These constants are only defined when nomad_test is enabled, so these fail
// our linter without explicit disabling.
c1 := TestClient(t, func(c *config.Config) {
c.Options = map[string]string{
driver.ShutdownPeriodicAfter: "true", // nolint: varcheck
driver.ShutdownPeriodicDuration: "3", // nolint: varcheck
}
})
defer c1.Shutdown()
mockDriverName := "driver.mock_driver"
go c1.watchNodeUpdates()
c1.updateNode()
// This needs to be directly called as otherwise the client hangs on
// attempt to register with a server. Specifically, retryRegisterNode is
// blocking
// Test that the client's copy of the node is also updated
testutil.WaitForResult(func() (bool, error) {
mockDriverStatusCopy := c1.configCopy.Node.Attributes[mockDriverName]
if mockDriverStatusCopy == "" {
return false, fmt.Errorf("mock driver attribute should be set on the client")
}
return true, nil
}, func(err error) {
t.Fatalf("err: %v", err)
})
// Test that the client's copy of the node is also updated
testutil.WaitForResult(func() (bool, error) {
mockDriverStatusCopy := c1.configCopy.Node.Attributes[mockDriverName]
if mockDriverStatusCopy != "" {
return false, fmt.Errorf("mock driver attribute should not be set on the client")
}
return true, nil
}, func(err error) {
t.Fatalf("err: %v", err)
})
}
func TestClient_Fingerprint_Periodic(t *testing.T) {
driver.CheckForMockDriver(t)
t.Parallel()

44
nomad/structs/structs.go
View file

@ -1432,6 +1432,50 @@ type NetworkResource struct {
DynamicPorts []Port // Host Dynamically assigned ports
}
func (nr *NetworkResource) Equals(other *NetworkResource) bool {
if nr.Device != other.Device {
return false
}
if nr.CIDR != other.CIDR {
return false
}
if nr.IP != other.IP {
return false
}
if nr.MBits != other.MBits {
return false
}
if len(nr.ReservedPorts) != len(other.ReservedPorts) {
return false
}
for i, port := range nr.ReservedPorts {
if len(other.ReservedPorts) <= i {
return false
}
if port != other.ReservedPorts[i] {
return false
}
}
if len(nr.DynamicPorts) != len(other.DynamicPorts) {
return false
}
for i, port := range nr.DynamicPorts {
if len(other.DynamicPorts) <= i {
return false
}
if port != other.DynamicPorts[i] {
return false
}
}
return true
}
func (n *NetworkResource) Canonicalize() {
// Ensure that an empty and nil slices are treated the same to avoid scheduling
// problems since we use reflect DeepEquals.

160
nomad/structs/structs_test.go
View file

@ -13,6 +13,7 @@ import (
"github.com/hashicorp/nomad/helper/uuid"
"github.com/kr/pretty"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestJob_Validate(t *testing.T) {
@ -3104,3 +3105,162 @@ func TestTaskEventPopulate(t *testing.T) {
}
}
}
func TestNetworkResourcesEquals(t *testing.T) {
require := require.New(t)
var networkResourcesTest = []struct {
input []*NetworkResource
expected bool
errorMsg string
}{
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
},
true,
"Equal network resources should return true",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.0",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
},
false,
"Different IP addresses should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 40,
ReservedPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
},
false,
"Different MBits values should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}, {"web", 80}},
},
},
false,
"Different ReservedPorts lengths should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{},
},
},
false,
"Empty and non empty ReservedPorts values should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
ReservedPorts: []Port{{"notweb", 80}},
},
},
false,
"Different valued ReservedPorts values should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
DynamicPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
DynamicPorts: []Port{{"web", 80}, {"web", 80}},
},
},
false,
"Different DynamicPorts lengths should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
DynamicPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
DynamicPorts: []Port{},
},
},
false,
"Empty and non empty DynamicPorts values should return false",
},
{
[]*NetworkResource{
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
DynamicPorts: []Port{{"web", 80}},
},
&NetworkResource{
IP: "10.0.0.1",
MBits: 50,
DynamicPorts: []Port{{"notweb", 80}},
},
},
false,
"Different valued DynamicPorts values should return false",
},
}
for _, testCase := range networkResourcesTest {
first := testCase.input[0]
second := testCase.input[1]
require.Equal(testCase.expected, first.Equals(second), testCase.errorMsg)
}
}