From ce4ab2bfdff36c3f81a3a7f2ee41d545c8974320 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Thu, 10 Nov 2016 16:48:07 +0100 Subject: [PATCH 1/3] registry/rpc: call etcdRegistry.MachineState() for every case Call MachineState, CreateMachineState of etcdRegistry, instead of registryMux, as machine states methods will not implemented for rpcRegistry. --- registry/rpc/registrymux.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/registry/rpc/registrymux.go b/registry/rpc/registrymux.go index 156c89e11..eb23700c7 100644 --- a/registry/rpc/registrymux.go +++ b/registry/rpc/registrymux.go @@ -255,7 +255,7 @@ func (r *RegistryMux) CreateUnit(unit *job.Unit) error { } func (r *RegistryMux) CreateMachineState(ms machine.MachineState, ttl time.Duration) (uint64, error) { - return r.getRegistry().CreateMachineState(ms, ttl) + return r.etcdRegistry.CreateMachineState(ms, ttl) } func (r *RegistryMux) DestroyUnit(unit string) error { @@ -291,7 +291,7 @@ func (r *RegistryMux) SetUnitTargetState(name string, state job.JobState) error } func (r *RegistryMux) MachineState(machID string) (machine.MachineState, error) { - return r.getRegistry().MachineState(machID) + return r.etcdRegistry.MachineState(machID) } func (r *RegistryMux) SetMachineState(ms machine.MachineState, ttl time.Duration) (uint64, error) { From 992f542984445cfa490311ec276f63a1a43a1c27 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Fri, 11 Nov 2016 11:21:09 +0100 Subject: [PATCH 2/3] api: get Value embedded as members in struct To avoid API incompatibility in the future, we need to get machineMetadataOp.Value embedded in a separate structure. Unit tests must be also changed accordingly. --- api/machines.go | 10 ++++++---- api/machines_test.go | 8 ++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/api/machines.go b/api/machines.go index 93f8b61fa..09c3e229d 100644 --- a/api/machines.go +++ b/api/machines.go @@ -44,8 +44,10 @@ type machinesResource struct { type machineMetadataOp struct { Operation string `json:"op"` - Path string - Value string + Path string `json:"path"` + Value struct { + Value string `json:"value"` + } } func (mr *machinesResource) ServeHTTP(rw http.ResponseWriter, req *http.Request) { @@ -100,7 +102,7 @@ func (mr *machinesResource) patch(rw http.ResponseWriter, req *http.Request) { return } - if op.Operation != "remove" && len(op.Value) == 0 { + if op.Operation != "remove" && len(op.Value.Value) == 0 { sendError(rw, http.StatusBadRequest, errors.New("invalid value: add and replace require a value")) return } @@ -119,7 +121,7 @@ func (mr *machinesResource) patch(rw http.ResponseWriter, req *http.Request) { return } } else { - err := mr.cAPI.SetMachineMetadata(machID, key, op.Value) + err := mr.cAPI.SetMachineMetadata(machID, key, op.Value.Value) if err != nil { sendError(rw, http.StatusInternalServerError, err) return diff --git a/api/machines_test.go b/api/machines_test.go index c08e5b51f..dfcc575fe 100644 --- a/api/machines_test.go +++ b/api/machines_test.go @@ -146,8 +146,8 @@ func TestExtractMachinePage(t *testing.T) { func TestMachinesPatchAddModify(t *testing.T) { reqBody := ` - [{"op": "add", "path": "/XXX/metadata/foo", "value": "bar"}, - {"op": "replace", "path": "/YYY/metadata/ping", "value": "splat"}] + [{"op": "add", "path": "/XXX/metadata/foo", "value": { "value": "bar" }}, + {"op": "replace", "path": "/YYY/metadata/ping", "value": { "value": "splat" }}] ` resource, rw := fakeMachinesSetup() @@ -218,7 +218,7 @@ func TestMachinesPatchDelete(t *testing.T) { func TestMachinesPatchBadOp(t *testing.T) { reqBody := ` - [{"op": "noop", "path": "/XXX/metadata/foo", "value": "bar"}] + [{"op": "noop", "path": "/XXX/metadata/foo", "value": { "value": "bar" }}] ` resource, rw := fakeMachinesSetup() @@ -235,7 +235,7 @@ func TestMachinesPatchBadOp(t *testing.T) { func TestMachinesPatchBadPath(t *testing.T) { reqBody := ` - [{"op": "add", "path": "/XXX/foo", "value": "bar"}] + [{"op": "add", "path": "/XXX/foo", "value": { "value": "bar" }}] ` resource, rw := fakeMachinesSetup() From 0823b9117f11eec08621292589b600c55f7e4d94 Mon Sep 17 00:00:00 2001 From: Dongsu Park Date: Fri, 11 Nov 2016 17:18:06 +0100 Subject: [PATCH 3/3] functional: add new machine tests for dynamic metadata Now that dynamic metadata is supported, we need to do machine metadata operations also in functional tests. These are nearly equivalent to the tests in api/machines_test.go. * TestMachinesList * TestMachinesListBadNextPageToken * TestMachinesPatchAddModify * TestMachinesPatchDelete * TestMachinesPatchBad --- functional/machines_test.go | 479 ++++++++++++++++++++++++++++++++++++ 1 file changed, 479 insertions(+) create mode 100644 functional/machines_test.go diff --git a/functional/machines_test.go b/functional/machines_test.go new file mode 100644 index 000000000..fd4cc2cc5 --- /dev/null +++ b/functional/machines_test.go @@ -0,0 +1,479 @@ +// Copyright 2016 The fleet Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package functional + +import ( + "encoding/json" + "fmt" + "io/ioutil" + "net/http" + "reflect" + "strings" + "testing" + + "github.com/coreos/fleet/functional/platform" +) + +// schemaMachines structures should match with those of +// schema/v1-json.go. Note that these structures cannot be +// directly imported from schema, because functional tests +// would then fail to compile. +type fxSchemaMachine struct { + Id string `json:"id,omitempty"` + + Metadata map[string]string `json:"metadata,omitempty"` + + PrimaryIP string `json:"primaryIP,omitempty"` +} + +type fxSchemaMachinePage struct { + Machines []*fxSchemaMachine `json:"machines,omitempty"` + + NextPageToken string `json:"nextPageToken,omitempty"` +} + +// machineMetadataOp structure should match with those of +// api/machines.go. Note that these structures cannot be +// directly imported from api, because functional tests +// would then fail to compile. +type ValueType struct { + Value string `json:"value"` +} + +type machineMetadataOp struct { + Operation string `json:"op"` + Path string `json:"path"` + Value ValueType +} + +func TestMachinesList(t *testing.T) { + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + + m, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + + _, err = cluster.WaitForNMachines(m, 1) + if err != nil { + t.Fatal(err) + } + + // Get a normal machine list, should return OK + resp, err := getHTTPResponse("GET", m.Endpoint()+"/fleet/v1/machines", "") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusNoContent { + t.Fatalf("Got HTTP response status %d.", resp.StatusCode) + } + + err = checkContentType(resp) + if err != nil { + t.Fatal(err) + } +} + +func TestMachinesListBadNextPageToken(t *testing.T) { + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + + m, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + + _, err = cluster.WaitForNMachines(m, 1) + if err != nil { + t.Fatal(err) + } + + // Send an invalid GET request, should return failure + resp, err := getHTTPResponse("GET", m.Endpoint()+"/fleet/v1/machines?nextPageToken=EwBMLg==", "") + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected status %d, got %d.", http.StatusBadRequest, resp.StatusCode) + } + + err = checkContentType(resp) + if err != nil { + t.Fatal(err) + } +} + +func TestMachinesPatchAddModify(t *testing.T) { + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + + m0, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + m1, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + + _, err = cluster.WaitForNMachines(m0, 2) + if err != nil { + t.Fatal(err) + } + + sm0 := &fxSchemaMachine{ + Id: m0.ID(), + Metadata: map[string]string{ + "foo": "bar", + "hostname": fmt.Sprintf("smoke%s", m0.ID()), + }, + PrimaryIP: m0.IP(), + } + + sm1 := &fxSchemaMachine{ + Id: m1.ID(), + Metadata: map[string]string{ + "ping": "splat", + "hostname": fmt.Sprintf("smoke%s", m1.ID()), + }, + PrimaryIP: m1.IP(), + } + + msp0 := &machineMetadataOp{ + Operation: "add", + Path: fmt.Sprintf("/%s/metadata/foo", m0.ID()), + Value: ValueType{Value: "bar"}, + } + msp1 := &machineMetadataOp{ + Operation: "replace", + Path: fmt.Sprintf("/%s/metadata/ping", m1.ID()), + Value: ValueType{Value: "splat"}, + } + msreq, err := json.Marshal([]*machineMetadataOp{msp0, msp1}) + if err != nil { + t.Fatalf("unexpected error marshalling: %#v", err) + } + reqBody := string(msreq) + + respPatch, err := getHTTPResponse("PATCH", m0.Endpoint()+"/fleet/v1/machines", reqBody) + if err != nil { + t.Fatal(err) + } + defer respPatch.Body.Close() + + if respPatch.StatusCode != http.StatusNoContent { + t.Fatalf("Expected status %d, got %d.", http.StatusNoContent, respPatch.StatusCode) + } + + err = checkContentType(respPatch) + if err != nil { + t.Fatal(err) + } + + // Send a normal GET to get list to be compared to an expected list + respGet, err := getHTTPResponse("GET", m0.Endpoint()+"/fleet/v1/machines", "") + if err != nil { + t.Fatal(err) + } + defer respGet.Body.Close() + + if respGet.StatusCode != http.StatusOK { + t.Fatalf("Got HTTP response status %d.", respGet.StatusCode) + } + + err = checkContentType(respGet) + if err != nil { + t.Fatal(err) + } + + body, rerr := ioutil.ReadAll(respGet.Body) + if rerr != nil { + t.Fatalf("Failed to read response body: %v", rerr) + } + + // unmarshal, compare + var page fxSchemaMachinePage + if err := json.Unmarshal(body, &page); err != nil { + t.Fatalf("Received unparsable body: %v", err) + } + + got := page.Machines + + for _, gotms := range got { + for _, sm := range []*fxSchemaMachine{sm0, sm1} { + if sm.Id != gotms.Id { + continue + } + if !reflect.DeepEqual(gotms, sm) { + t.Errorf("Unexpected Machines received.") + t.Logf("Got Machines:") + t.Logf("%#v", gotms) + t.Logf("Expected Machines:") + t.Logf("%#v", sm) + } + } + } +} + +func TestMachinesPatchDelete(t *testing.T) { + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + + m0, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + m1, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + + _, err = cluster.WaitForNMachines(m0, 2) + if err != nil { + t.Fatal(err) + } + + sm0 := &fxSchemaMachine{ + Id: m0.ID(), + Metadata: map[string]string{ + "hostname": fmt.Sprintf("smoke%s", m0.ID()), + }, + PrimaryIP: m0.IP(), + } + + sm1 := &fxSchemaMachine{ + Id: m1.ID(), + Metadata: map[string]string{ + "hostname": fmt.Sprintf("smoke%s", m1.ID()), + }, + PrimaryIP: m1.IP(), + } + + msp0 := &machineMetadataOp{ + Operation: "remove", + Path: fmt.Sprintf("/%s/metadata/foo", m0.ID()), + Value: ValueType{Value: ""}, + } + msp1 := &machineMetadataOp{ + Operation: "remove", + Path: fmt.Sprintf("/%s/metadata/ping", m1.ID()), + Value: ValueType{Value: ""}, + } + msreq, err := json.Marshal([]*machineMetadataOp{msp0, msp1}) + if err != nil { + t.Fatalf("unexpected error marshalling: %#v", err) + } + reqBody := string(msreq) + + respPatch, err := getHTTPResponse("PATCH", m0.Endpoint()+"/fleet/v1/machines", reqBody) + if err != nil { + t.Fatal(err) + } + defer respPatch.Body.Close() + + if respPatch.StatusCode != http.StatusNoContent { + t.Fatalf("Expected status %d, got %d.", http.StatusNoContent, respPatch.StatusCode) + } + + err = checkContentType(respPatch) + if err != nil { + t.Fatal(err) + } + + // Send a normal GET to get list to be compared to an expected list + respGet, err := getHTTPResponse("GET", m0.Endpoint()+"/fleet/v1/machines", "") + if err != nil { + t.Fatal(err) + } + defer respGet.Body.Close() + + if respGet.StatusCode != http.StatusOK { + t.Fatalf("Got HTTP response status %d.", respGet.StatusCode) + } + + err = checkContentType(respGet) + if err != nil { + t.Fatal(err) + } + + body, rerr := ioutil.ReadAll(respGet.Body) + if rerr != nil { + t.Fatalf("Failed to read response body: %v", rerr) + } + + // unmarshal, compare + var page fxSchemaMachinePage + if err := json.Unmarshal(body, &page); err != nil { + t.Fatalf("Received unparsable body: %v", err) + } + + got := page.Machines + + for _, gotms := range got { + for _, sm := range []*fxSchemaMachine{sm0, sm1} { + if sm.Id != gotms.Id { + continue + } + if !reflect.DeepEqual(gotms, sm) { + t.Errorf("Unexpected Machines received.") + t.Logf("Got Machines:") + t.Logf("%#v", gotms) + t.Logf("Expected Machines:") + t.Logf("%#v", sm) + } + } + } +} + +func TestMachinesPatchBad(t *testing.T) { + cluster, err := platform.NewNspawnCluster("smoke") + if err != nil { + t.Fatal(err) + } + defer cluster.Destroy(t) + + m0, err := cluster.CreateMember() + if err != nil { + t.Fatal(err) + } + + _, err = cluster.WaitForNMachines(m0, 1) + if err != nil { + t.Fatal(err) + } + + // Test bad operation + msp0 := &machineMetadataOp{ + Operation: "noop", + Path: fmt.Sprintf("/%s/metadata/foo", m0.ID()), + Value: ValueType{Value: "bar"}, + } + msreq, err := json.Marshal([]*machineMetadataOp{msp0}) + if err != nil { + t.Fatalf("unexpected error marshalling: %#v", err) + } + + respPatch0, err := getHTTPResponse("PATCH", m0.Endpoint()+"/fleet/v1/machines", string(msreq)) + if err != nil { + t.Fatal(err) + } + defer respPatch0.Body.Close() + + if respPatch0.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected status %d, got %d.", http.StatusBadRequest, respPatch0.StatusCode) + } + + err = checkContentType(respPatch0) + if err != nil { + t.Fatal(err) + } + + // Test bad path + msp1 := &machineMetadataOp{ + Operation: "add", + Path: fmt.Sprintf("/%s/foo", m0.ID()), + Value: ValueType{Value: "bar"}, + } + msreq, err = json.Marshal([]*machineMetadataOp{msp1}) + if err != nil { + t.Fatalf("unexpected error marshalling: %#v", err) + } + + respPatch1, err := getHTTPResponse("PATCH", m0.Endpoint()+"/fleet/v1/machines", string(msreq)) + if err != nil { + t.Fatal(err) + } + defer respPatch1.Body.Close() + + if respPatch1.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected status %d, got %d.", http.StatusBadRequest, respPatch1.StatusCode) + } + + err = checkContentType(respPatch1) + if err != nil { + t.Fatal(err) + } + + // Test bad value + msp2 := &machineMetadataOp{ + Operation: "add", + Path: fmt.Sprintf("/%s/foo", m0.ID()), + Value: ValueType{Value: ""}, + } + msreq, err = json.Marshal([]*machineMetadataOp{msp2}) + if err != nil { + t.Fatalf("unexpected error marshalling: %#v", err) + } + + respPatch2, err := getHTTPResponse("PATCH", m0.Endpoint()+"/fleet/v1/machines", string(msreq)) + if err != nil { + t.Fatal(err) + } + defer respPatch2.Body.Close() + + if respPatch2.StatusCode != http.StatusBadRequest { + t.Fatalf("Expected status %d, got %d.", http.StatusBadRequest, respPatch2.StatusCode) + } + + err = checkContentType(respPatch2) + if err != nil { + t.Fatal(err) + } +} + +func getHTTPResponse(method string, urlPath string, val string) (*http.Response, error) { + req, err := http.NewRequest(method, urlPath, strings.NewReader(val)) + if err != nil { + return nil, fmt.Errorf("Failed creating http.Request: %v", err) + } + + cl := http.Client{} + resp, err := cl.Do(req) + if err != nil { + return nil, fmt.Errorf("Failed to run client.Do: %v", err) + } + if resp.Body == nil { + return nil, fmt.Errorf("Got HTTP response nil body") + } + + return resp, nil +} + +func checkContentType(resp *http.Response) error { + ct := resp.Header.Get("Content-Type") + if len(ct) == 0 { + return fmt.Errorf("Response has wrong number of Content-Type values: %v", ct) + } else if !strings.Contains(ct, "application/json") { + return fmt.Errorf("Expected application/json, got %v", ct) + } + return nil +}