Skip to content

Commit

Permalink
create safe getters and setters for fingerprint response
Browse files Browse the repository at this point in the history
  • Loading branch information
chelseakomlo committed Jan 26, 2018
1 parent c21ac46 commit f5fc20a
Show file tree
Hide file tree
Showing 45 changed files with 388 additions and 521 deletions.
39 changes: 12 additions & 27 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -955,19 +955,14 @@ func (c *Client) fingerprint() error {
appliedFingerprints = append(appliedFingerprints, name)

request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err = f.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err = f.Fingerprint(request, &response)
if err != nil {
return err
}

// add the diff found from each fingerprinter
c.updateNodeFromFingerprint(response)
c.updateNodeFromFingerprint(&response)

p, period := f.Periodic()
if p {
Expand All @@ -992,18 +987,13 @@ func (c *Client) fingerprintPeriodic(name string, f fingerprint.Fingerprint, d t
select {
case <-time.After(d):
request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err := f.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err := f.Fingerprint(request, &response)

if err != nil {
c.logger.Printf("[DEBUG] client: periodic fingerprinting for %v failed: %v", name, err)
} else {
c.updateNodeFromFingerprint(response)
c.updateNodeFromFingerprint(&response)
}

case <-c.shutdownCh:
Expand Down Expand Up @@ -1045,17 +1035,12 @@ func (c *Client) setupDrivers() error {
}

request := &cstructs.FingerprintRequest{Config: c.config, Node: c.config.Node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

if err := d.Fingerprint(request, response); err != nil {
var response cstructs.FingerprintResponse
if err := d.Fingerprint(request, &response); err != nil {
return err
}

c.updateNodeFromFingerprint(response)
c.updateNodeFromFingerprint(&response)

p, period := d.Periodic()
if p {
Expand All @@ -1077,7 +1062,7 @@ func (c *Client) setupDrivers() error {
func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintResponse) {
c.configLock.Lock()
defer c.configLock.Unlock()
for name, val := range response.Attributes {
for name, val := range response.GetAttributes() {
if val == "" {
delete(c.config.Node.Attributes, name)
} else {
Expand All @@ -1087,15 +1072,15 @@ func (c *Client) updateNodeFromFingerprint(response *cstructs.FingerprintRespons

// update node links and resources from the diff created from
// fingerprinting
for name, val := range response.Links {
for name, val := range response.GetLinks() {
if val == "" {
delete(c.config.Node.Links, name)
} else {
c.config.Node.Links[name] = val
}
}

c.config.Node.Resources.Merge(response.Resources)
c.config.Node.Resources.Merge(response.GetResources())
}

// retryIntv calculates a retry interval value given the base
Expand Down
10 changes: 5 additions & 5 deletions client/driver/docker.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,17 +497,17 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
return nil
}

resp.Attributes[dockerDriverAttr] = "1"
resp.Attributes["driver.docker.version"] = env.Get("Version")
resp.AddAttribute(dockerDriverAttr, "1")
resp.AddAttribute("driver.docker.version", env.Get("Version"))

privileged := d.config.ReadBoolDefault(dockerPrivilegedConfigOption, false)
if privileged {
resp.Attributes[dockerPrivilegedConfigOption] = "1"
resp.AddAttribute(dockerPrivilegedConfigOption, "1")
}

// Advertise if this node supports Docker volumes
if d.config.ReadBoolDefault(dockerVolumesConfigOption, dockerVolumesConfigDefault) {
resp.Attributes["driver."+dockerVolumesConfigOption] = "1"
resp.AddAttribute("driver."+dockerVolumesConfigOption, "1")
}

// Detect bridge IP address - #2785
Expand All @@ -525,7 +525,7 @@ func (d *DockerDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstru
}

if n.IPAM.Config[0].Gateway != "" {
resp.Attributes["driver.docker.bridge_ip"] = n.IPAM.Config[0].Gateway
resp.AddAttribute("driver.docker.bridge_ip", n.IPAM.Config[0].Gateway)
} else if d.fingerprintSuccess == nil {
// Docker 17.09.0-ce dropped the Gateway IP from the bridge network
// See https://github.com/moby/moby/issues/32648
Expand Down
33 changes: 13 additions & 20 deletions client/driver/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,26 +173,22 @@ func TestDockerDriver_Fingerprint(t *testing.T) {
}

request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err := d.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err := d.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
}

if testutil.DockerIsConnected(t) && response.Attributes["driver.docker"] == "" {
attributes := response.GetAttributes()
if testutil.DockerIsConnected(t) && attributes["driver.docker"] == "" {
t.Fatalf("Fingerprinter should detect when docker is available")
}

if response.Attributes["driver.docker"] != "1" {
if attributes["driver.docker"] != "1" {
t.Log("Docker daemon not available. The remainder of the docker tests will be skipped.")
}

t.Logf("Found docker version %s", response.Attributes["driver.docker.version"])
t.Logf("Found docker version %s", attributes["driver.docker.version"])
}

// TestDockerDriver_Fingerprint_Bridge asserts that if Docker is running we set
Expand Down Expand Up @@ -223,24 +219,21 @@ func TestDockerDriver_Fingerprint_Bridge(t *testing.T) {
dd := NewDockerDriver(NewDriverContext("", "", conf, conf.Node, testLogger(), nil))

request := &cstructs.FingerprintRequest{Config: conf, Node: conf.Node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err = dd.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err = dd.Fingerprint(request, &response)
if err != nil {
t.Fatalf("error fingerprinting docker: %v", err)
}
if response.Attributes["driver.docker"] == "" {

attributes := response.GetAttributes()
if attributes["driver.docker"] == "" {
t.Fatalf("expected Docker to be enabled but false was returned")
}

if found := response.Attributes["driver.docker.bridge_ip"]; found != expectedAddr {
if found := attributes["driver.docker.bridge_ip"]; found != expectedAddr {
t.Fatalf("expected bridge ip %q but found: %q", expectedAddr, found)
}
t.Logf("docker bridge ip: %q", response.Attributes["driver.docker.bridge_ip"])
t.Logf("docker bridge ip: %q", attributes["driver.docker.bridge_ip"])
}

func TestDockerDriver_StartOpen_Wait(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion client/driver/exec_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func (d *ExecDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
if d.fingerprintSuccess == nil || !*d.fingerprintSuccess {
d.logger.Printf("[DEBUG] driver.exec: exec driver is enabled")
}
resp.Attributes[execDriverAttr] = "1"
resp.AddAttribute(execDriverAttr, "1")
d.fingerprintSuccess = helper.BoolToPtr(true)
return nil
}
11 changes: 3 additions & 8 deletions client/driver/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,12 @@ func TestExecDriver_Fingerprint(t *testing.T) {
}

request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err := d.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err := d.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
}
if response.Attributes["driver.exec"] == "" {
if response.GetAttributes()["driver.exec"] == "" {
t.Fatalf("missing driver")
}
}
Expand Down
11 changes: 4 additions & 7 deletions client/driver/java.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
if d.fingerprintSuccess == nil || *d.fingerprintSuccess {
d.logger.Printf("[DEBUG] driver.java: root privileges and mounted cgroups required on linux, disabling")
}
resp.Attributes[javaDriverAttr] = ""
d.fingerprintSuccess = helper.BoolToPtr(false)
return nil
}
Expand All @@ -131,7 +130,6 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
err := cmd.Run()
if err != nil {
// assume Java wasn't found
resp.Attributes[javaDriverAttr] = ""
d.fingerprintSuccess = helper.BoolToPtr(false)
return nil
}
Expand All @@ -151,7 +149,6 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
if d.fingerprintSuccess == nil || *d.fingerprintSuccess {
d.logger.Println("[WARN] driver.java: error parsing Java version information, aborting")
}
resp.Attributes[javaDriverAttr] = ""
d.fingerprintSuccess = helper.BoolToPtr(false)
return nil
}
Expand All @@ -165,10 +162,10 @@ func (d *JavaDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
versionString := info[0]
versionString = strings.TrimPrefix(versionString, "java version ")
versionString = strings.Trim(versionString, "\"")
resp.Attributes[javaDriverAttr] = "1"
resp.Attributes["driver.java.version"] = versionString
resp.Attributes["driver.java.runtime"] = info[1]
resp.Attributes["driver.java.vm"] = info[2]
resp.AddAttribute(javaDriverAttr, "1")
resp.AddAttribute("driver.java.version", versionString)
resp.AddAttribute("driver.java.runtime", info[1])
resp.AddAttribute("driver.java.vm", info[2])
d.fingerprintSuccess = helper.BoolToPtr(true)

return nil
Expand Down
14 changes: 5 additions & 9 deletions client/driver/java_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,26 +52,22 @@ func TestJavaDriver_Fingerprint(t *testing.T) {
}

request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err := d.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err := d.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
}

if response.Attributes["driver.java"] != "1" && javaLocated() {
attributes := response.GetAttributes()
if attributes["driver.java"] != "1" && javaLocated() {
if v, ok := osJavaDriverSupport[runtime.GOOS]; v && ok {
t.Fatalf("missing java driver")
} else {
t.Skipf("missing java driver, no OS support")
}
}
for _, key := range []string{"driver.java.version", "driver.java.runtime", "driver.java.vm"} {
if response.Attributes[key] == "" {
if attributes[key] == "" {
t.Fatalf("missing driver key (%s)", key)
}
}
Expand Down
6 changes: 3 additions & 3 deletions client/driver/lxc.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,12 @@ func (d *LxcDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs
if version == "" {
return nil
}
resp.Attributes["driver.lxc.version"] = version
resp.Attributes["driver.lxc"] = "1"
resp.AddAttribute("driver.lxc.version", version)
resp.AddAttribute("driver.lxc", "1")

// Advertise if this node supports lxc volumes
if d.config.ReadBoolDefault(lxcVolumesConfigOption, lxcVolumesConfigDefault) {
resp.Attributes["driver."+lxcVolumesConfigOption] = "1"
resp.AddAttribute("driver."+lxcVolumesConfigOption, "1")
}

return nil
Expand Down
20 changes: 5 additions & 15 deletions client/driver/lxc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,8 @@ func TestLxcDriver_Fingerprint(t *testing.T) {
// test with an empty config
{
request := &cstructs.FingerprintRequest{Config: &config.Config{}, Node: node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err := d.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err := d.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
}
Expand All @@ -59,17 +54,12 @@ func TestLxcDriver_Fingerprint(t *testing.T) {
{
conf := &config.Config{Options: map[string]string{lxcConfigOption: "1"}}
request := &cstructs.FingerprintRequest{Config: conf, Node: node}
response := &cstructs.FingerprintResponse{
Attributes: make(map[string]string, 0),
Links: make(map[string]string, 0),
Resources: &structs.Resources{},
}

err := d.Fingerprint(request, response)
var response cstructs.FingerprintResponse
err := d.Fingerprint(request, &response)
if err != nil {
t.Fatalf("err: %v", err)
}
if response.Attributes["driver.lxc"] == "" {
if response.GetAttributes()["driver.lxc"] == "" {
t.Fatalf("missing driver")
}
}
Expand Down
2 changes: 1 addition & 1 deletion client/driver/mock_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ func (m *MockDriver) Validate(map[string]interface{}) error {

// Fingerprint fingerprints a node and returns if MockDriver is enabled
func (m *MockDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstructs.FingerprintResponse) error {
resp.Attributes["driver.mock_driver"] = "1"
resp.AddAttribute("driver.mock_driver", "1")
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions client/driver/qemu.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ func (d *QemuDriver) Fingerprint(req *cstructs.FingerprintRequest, resp *cstruct
}
currentQemuVersion := matches[1]

resp.Attributes[qemuDriverAttr] = "1"
resp.Attributes[qemuDriverVersionAttr] = currentQemuVersion
resp.AddAttribute(qemuDriverAttr, "1")
resp.AddAttribute(qemuDriverVersionAttr, currentQemuVersion)

return nil
}
Expand Down
Loading

0 comments on commit f5fc20a

Please sign in to comment.