From 881613e3f50adcd55d9ac6759a1b4af3f9117757 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 12:37:35 -0700 Subject: [PATCH 1/7] First part of parsing settings client-side --- pkg/cloudcfg/cloudcfg.go | 2 +- pkg/cloudcfg/cloudcfg_test.go | 1 + pkg/cloudcfg/parse.go | 41 ++++++++++++++++ pkg/cloudcfg/parse_test.go | 88 +++++++++++++++++++++++++++++++++++ pkg/util/fake_handler.go | 11 +++-- 5 files changed, 139 insertions(+), 4 deletions(-) create mode 100644 pkg/cloudcfg/parse.go create mode 100644 pkg/cloudcfg/parse_test.go diff --git a/pkg/cloudcfg/cloudcfg.go b/pkg/cloudcfg/cloudcfg.go index 0245fe5f7ef..55e35c466be 100644 --- a/pkg/cloudcfg/cloudcfg.go +++ b/pkg/cloudcfg/cloudcfg.go @@ -250,7 +250,7 @@ func DeleteController(name string, client client.ClientInterface) error { return err } if controller.DesiredState.Replicas != 0 { - return fmt.Errorf("controller has non-zero replicas (%d)", controller.DesiredState.Replicas) + return fmt.Errorf("controller has non-zero replicas (%d), please stop it first", controller.DesiredState.Replicas) } return client.DeleteReplicationController(name) } diff --git a/pkg/cloudcfg/cloudcfg_test.go b/pkg/cloudcfg/cloudcfg_test.go index 04dd8dd800a..f4bffb2857c 100644 --- a/pkg/cloudcfg/cloudcfg_test.go +++ b/pkg/cloudcfg/cloudcfg_test.go @@ -150,6 +150,7 @@ func TestDoRequest(t *testing.T) { fakeHandler := util.FakeHandler{ StatusCode: 200, ResponseBody: expectedBody, + T: t, } testServer := httptest.NewTLSServer(&fakeHandler) request, _ := http.NewRequest("GET", testServer.URL+"/foo/bar", nil) diff --git a/pkg/cloudcfg/parse.go b/pkg/cloudcfg/parse.go new file mode 100644 index 00000000000..2656bf49d73 --- /dev/null +++ b/pkg/cloudcfg/parse.go @@ -0,0 +1,41 @@ +package cloudcfg + +import ( + "encoding/json" + "fmt" + "reflect" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "gopkg.in/v1/yaml" +) + +var storageToType = map[string]reflect.Type{ + "pods": reflect.TypeOf(api.Pod{}), + "services": reflect.TypeOf(api.Service{}), + "replicationControllers": reflect.TypeOf(api.ReplicationController{}), +} + +// Takes input 'data' as either json or yaml, checks that it parses as the +// appropriate object type, and returns json for sending to the API or an +// error. +func ToWireFormat(data []byte, storage string) ([]byte, error) { + prototypeType, found := storageToType[storage] + if !found { + return nil, fmt.Errorf("unknown storage type: %v", storage) + } + + // Try parsing as json and yaml + parsed_json := reflect.New(prototypeType).Interface() + json_err := json.Unmarshal(data, parsed_json) + parsed_yaml := reflect.New(prototypeType).Interface() + yaml_err := yaml.Unmarshal(data, parsed_yaml) + + if json_err != nil && yaml_err != nil { + return nil, fmt.Errorf("Could not parse input as json (error: %v) or yaml (error: %v", json_err, yaml_err) + } + + if json_err != nil { + return json.Marshal(parsed_json) + } + return json.Marshal(parsed_yaml) +} diff --git a/pkg/cloudcfg/parse_test.go b/pkg/cloudcfg/parse_test.go new file mode 100644 index 00000000000..26bf6445f35 --- /dev/null +++ b/pkg/cloudcfg/parse_test.go @@ -0,0 +1,88 @@ +package cloudcfg + +import ( + "encoding/json" + "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "gopkg.in/v1/yaml" +) + +func TestParseBadStorage(t *testing.T) { + _, err := ToWireFormat([]byte("{}"), "badstorage") + if err == nil { + t.Errorf("Expected error, received none") + } +} + +func DoParseTest(t *testing.T, storage string, obj interface{}) { + json_data, _ := json.Marshal(obj) + yaml_data, _ := yaml.Marshal(obj) + + json_got, json_err := ToWireFormat(json_data, storage) + yaml_got, yaml_err := ToWireFormat(yaml_data, storage) + + if json_err != nil { + t.Errorf("json err: %#v", json_err) + } + if yaml_err != nil { + t.Errorf("yaml err: %#v", yaml_err) + } + t.Logf("Intermediate yaml:\n%v\n", string(yaml_data)) + if string(json_got) != string(json_data) { + t.Errorf("json output didn't match:\nGot:\n%v\n\nWanted:\n%v\n", + string(json_got), string(json_data)) + } + if string(yaml_got) != string(json_data) { + t.Errorf("yaml parsed output didn't match:\nGot:\n%v\n\nWanted:\n%v\n", + string(yaml_got), string(json_data)) + } +} + +func TestParsePod(t *testing.T) { + DoParseTest(t, "pods", api.Pod{ + JSONBase: api.JSONBase{ID: "test pod"}, + DesiredState: api.PodState{ + Manifest: api.ContainerManifest{ + Id: "My manifest", + Containers: []api.Container{ + {Name: "my container"}, + }, + Volumes: []api.Volume{ + {Name: "volume"}, + }, + }, + }, + }) +} + +func TestParseService(t *testing.T) { + DoParseTest(t, "services", api.Service{ + JSONBase: api.JSONBase{ID: "my service"}, + Port: 8080, + Labels: map[string]string{ + "area": "staging", + }, + }) +} + +func TestParseController(t *testing.T) { + DoParseTest(t, "replicationControllers", api.ReplicationController{ + DesiredState: api.ReplicationControllerState{ + Replicas: 9001, + PodTemplate: api.PodTemplate{ + DesiredState: api.PodState{ + Manifest: api.ContainerManifest{ + Id: "My manifest", + Containers: []api.Container{ + {Name: "my container"}, + }, + Volumes: []api.Volume{ + {Name: "volume"}, + }, + }, + }, + }, + }, + }) +} diff --git a/pkg/util/fake_handler.go b/pkg/util/fake_handler.go index fa6a698e2f7..f6b4adb7253 100644 --- a/pkg/util/fake_handler.go +++ b/pkg/util/fake_handler.go @@ -17,7 +17,6 @@ package util import ( "io/ioutil" - "log" "net/http" ) @@ -26,12 +25,18 @@ import ( type TestInterface interface { Errorf(format string, args ...interface{}) } +type LogInterface interface { + Logf(format string, args ...interface{}) +} // FakeHandler is to assist in testing HTTP requests. type FakeHandler struct { RequestReceived *http.Request StatusCode int ResponseBody string + // For logging - you can use a *testing.T + // This will keep log messages associated with the test. + T LogInterface } func (f *FakeHandler) ServeHTTP(response http.ResponseWriter, request *http.Request) { @@ -40,8 +45,8 @@ func (f *FakeHandler) ServeHTTP(response http.ResponseWriter, request *http.Requ response.Write([]byte(f.ResponseBody)) bodyReceived, err := ioutil.ReadAll(request.Body) - if err != nil { - log.Printf("Received read error: %#v", err) + if err != nil && f.T != nil { + f.T.Logf("Received read error: %#v", err) } f.ResponseBody = string(bodyReceived) } From 853a4e26a89e2e5180789b25d119a32f2fecd15f Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 13:41:48 -0700 Subject: [PATCH 2/7] Call parsing code from cloudcfg --- cmd/cloudcfg/cloudcfg.go | 24 +++++++++++++++++++++--- pkg/cloudcfg/cloudcfg.go | 6 +++--- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/cmd/cloudcfg/cloudcfg.go b/cmd/cloudcfg/cloudcfg.go index 628f2f83214..e56e6e046cb 100644 --- a/cmd/cloudcfg/cloudcfg.go +++ b/cmd/cloudcfg/cloudcfg.go @@ -18,6 +18,7 @@ package main import ( "flag" "fmt" + "io/ioutil" "log" "net/http" "net/url" @@ -50,6 +51,22 @@ func usage() { log.Fatal("Usage: cloudcfg -h [-c config/file.json] [-p :,..., : ") } +// Reads & parses config file. On error, calls log.Fatal(). +func readConfig(storage string) []byte { + if len(*config) == 0 { + log.Fatal("Need config file (-c)") + } + data, err := ioutil.ReadFile(*config) + if err != nil { + log.Fatalf("Unable to read %v: %#v\n", *config, err) + } + data, err = cloudcfg.ToWireFormat(data, storage) + if err != nil { + log.Fatalf("Error parsing %v as an object for %v: %#v\n", *config, storage, err) + } + return data +} + // CloudCfg command line tool. func main() { flag.Parse() // Scan the arguments list @@ -71,7 +88,8 @@ func main() { if parsedUrl.Scheme != "" && parsedUrl.Scheme != "https" { secure = false } - url := *httpServer + path.Join("/api/v1beta1", flag.Arg(1)) + storage := flag.Arg(1) + url := *httpServer + path.Join("/api/v1beta1", storage) var request *http.Request var printer cloudcfg.ResourcePrinter @@ -100,9 +118,9 @@ func main() { case "delete": request, err = http.NewRequest("DELETE", url, nil) case "create": - request, err = cloudcfg.RequestWithBody(*config, url, "POST") + request, err = cloudcfg.RequestWithBodyData(readConfig(storage), url, "POST") case "update": - request, err = cloudcfg.RequestWithBody(*config, url, "PUT") + request, err = cloudcfg.RequestWithBodyData(readConfig(storage), url, "PUT") case "rollingupdate": client := &kube_client.Client{ Host: *httpServer, diff --git a/pkg/cloudcfg/cloudcfg.go b/pkg/cloudcfg/cloudcfg.go index 55e35c466be..2f589408eb2 100644 --- a/pkg/cloudcfg/cloudcfg.go +++ b/pkg/cloudcfg/cloudcfg.go @@ -102,12 +102,12 @@ func RequestWithBody(configFile, url, method string) (*http.Request, error) { if err != nil { return nil, err } - return requestWithBodyData(data, url, method) + return RequestWithBodyData(data, url, method) } -// requestWithBodyData is a helper method that creates an HTTP request with the specified url, method +// RequestWithBodyData is a helper method that creates an HTTP request with the specified url, method // and body data -func requestWithBodyData(data []byte, url, method string) (*http.Request, error) { +func RequestWithBodyData(data []byte, url, method string) (*http.Request, error) { request, err := http.NewRequest(method, url, bytes.NewBuffer(data)) request.ContentLength = int64(len(data)) return request, err From 74fd0c1a58a4200cf2af5b6a4187281f55ac55a1 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 14:13:02 -0700 Subject: [PATCH 3/7] Usability improvements --- cmd/cloudcfg/cloudcfg.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/cloudcfg/cloudcfg.go b/cmd/cloudcfg/cloudcfg.go index e56e6e046cb..5a952529f0a 100644 --- a/cmd/cloudcfg/cloudcfg.go +++ b/cmd/cloudcfg/cloudcfg.go @@ -25,6 +25,7 @@ import ( "os" "path" "strconv" + "strings" "time" kube_client "github.com/GoogleCloudPlatform/kubernetes/pkg/client" @@ -64,6 +65,7 @@ func readConfig(storage string) []byte { if err != nil { log.Fatalf("Error parsing %v as an object for %v: %#v\n", *config, storage, err) } + log.Printf("Parsed config file successfully; sending:\n%v\n", string(data)) return data } @@ -88,7 +90,7 @@ func main() { if parsedUrl.Scheme != "" && parsedUrl.Scheme != "https" { secure = false } - storage := flag.Arg(1) + storage := strings.Trim(flag.Arg(1), "/") url := *httpServer + path.Join("/api/v1beta1", storage) var request *http.Request @@ -167,7 +169,7 @@ func main() { } err = printer.Print(body, os.Stdout) if err != nil { - log.Fatalf("Failed to print: %#v", err) + log.Fatalf("Failed to print: %#v\nRaw received text:\n%v\n", err, string(body)) } fmt.Print("\n") } From 601f6bb4ade986761b238fd04cee4a46b24673b3 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 15:10:28 -0700 Subject: [PATCH 4/7] Add inline attribute --- pkg/api/types.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/api/types.go b/pkg/api/types.go index 38179c4c7ec..eeeb36d5716 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -91,13 +91,13 @@ type PodState struct { } type PodList struct { - JSONBase - Items []Pod `json:"items" yaml:"items,omitempty"` + JSONBase `json:",inline" yaml:",inline"` + Items []Pod `json:"items" yaml:"items,omitempty"` } // Pod is a collection of containers, used as either input (create, update) or as output (list, get) type Pod struct { - JSONBase + JSONBase `json:",inline" yaml:",inline"` Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"` DesiredState PodState `json:"desiredState,omitempty" yaml:"desiredState,omitempty"` CurrentState PodState `json:"currentState,omitempty" yaml:"currentState,omitempty"` @@ -111,13 +111,13 @@ type ReplicationControllerState struct { } type ReplicationControllerList struct { - JSONBase - Items []ReplicationController `json:"items,omitempty" yaml:"items,omitempty"` + JSONBase `json:",inline" yaml:",inline"` + Items []ReplicationController `json:"items,omitempty" yaml:"items,omitempty"` } // ReplicationController represents the configuration of a replication controller type ReplicationController struct { - JSONBase + JSONBase `json:",inline" yaml:",inline"` DesiredState ReplicationControllerState `json:"desiredState,omitempty" yaml:"desiredState,omitempty"` Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"` } @@ -130,16 +130,16 @@ type PodTemplate struct { // ServiceList holds a list of services type ServiceList struct { - JSONBase - Items []Service `json:"items" yaml:"items"` + JSONBase `json:",inline" yaml:",inline"` + Items []Service `json:"items" yaml:"items"` } // Defines a service abstraction by a name (for example, mysql) consisting of local port // (for example 3306) that the proxy listens on, and the labels that define the service. type Service struct { - JSONBase - Port int `json:"port,omitempty" yaml:"port,omitempty"` - Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"` + JSONBase `json:",inline" yaml:",inline"` + Port int `json:"port,omitempty" yaml:"port,omitempty"` + Labels map[string]string `json:"labels,omitempty" yaml:"labels,omitempty"` } // Defines the endpoints that implement the actual service, for example: From 2abfd95d6bfb5df93aa523b13f770a814f73ecfe Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 16:46:07 -0700 Subject: [PATCH 5/7] Tests pass now. --- pkg/cloudcfg/parse.go | 36 +++++++++++++++++++++++++----------- pkg/cloudcfg/parse_test.go | 2 +- 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/pkg/cloudcfg/parse.go b/pkg/cloudcfg/parse.go index 2656bf49d73..d5b40deca44 100644 --- a/pkg/cloudcfg/parse.go +++ b/pkg/cloudcfg/parse.go @@ -24,18 +24,32 @@ func ToWireFormat(data []byte, storage string) ([]byte, error) { return nil, fmt.Errorf("unknown storage type: %v", storage) } - // Try parsing as json and yaml - parsed_json := reflect.New(prototypeType).Interface() - json_err := json.Unmarshal(data, parsed_json) - parsed_yaml := reflect.New(prototypeType).Interface() - yaml_err := yaml.Unmarshal(data, parsed_yaml) + // Try parsing json + json_out, json_err := tryJSON(data, reflect.New(prototypeType).Interface()) + if json_err == nil { + return json_out, json_err + } - if json_err != nil && yaml_err != nil { + // Try parsing yaml + yaml_out, yaml_err := tryYAML(data, reflect.New(prototypeType).Interface()) + if yaml_err != nil { return nil, fmt.Errorf("Could not parse input as json (error: %v) or yaml (error: %v", json_err, yaml_err) } - - if json_err != nil { - return json.Marshal(parsed_json) - } - return json.Marshal(parsed_yaml) + return yaml_out, yaml_err +} + +func tryJSON(data []byte, obj interface{}) ([]byte, error) { + err := json.Unmarshal(data, obj) + if err != nil { + return nil, err + } + return json.Marshal(obj) +} + +func tryYAML(data []byte, obj interface{}) ([]byte, error) { + err := yaml.Unmarshal(data, obj) + if err != nil { + return nil, err + } + return json.Marshal(obj) } diff --git a/pkg/cloudcfg/parse_test.go b/pkg/cloudcfg/parse_test.go index 26bf6445f35..1884466de6e 100644 --- a/pkg/cloudcfg/parse_test.go +++ b/pkg/cloudcfg/parse_test.go @@ -18,6 +18,7 @@ func TestParseBadStorage(t *testing.T) { func DoParseTest(t *testing.T, storage string, obj interface{}) { json_data, _ := json.Marshal(obj) yaml_data, _ := yaml.Marshal(obj) + t.Logf("Intermediate yaml:\n%v\n", string(yaml_data)) json_got, json_err := ToWireFormat(json_data, storage) yaml_got, yaml_err := ToWireFormat(yaml_data, storage) @@ -28,7 +29,6 @@ func DoParseTest(t *testing.T, storage string, obj interface{}) { if yaml_err != nil { t.Errorf("yaml err: %#v", yaml_err) } - t.Logf("Intermediate yaml:\n%v\n", string(yaml_data)) if string(json_got) != string(json_data) { t.Errorf("json output didn't match:\nGot:\n%v\n\nWanted:\n%v\n", string(json_got), string(json_data)) From 867daadbdb0bca3867fe24e198d377624b066456 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 17:11:02 -0700 Subject: [PATCH 6/7] Yaml parses json, too; no need to try both. --- pkg/cloudcfg/parse.go | 24 +----------------------- 1 file changed, 1 insertion(+), 23 deletions(-) diff --git a/pkg/cloudcfg/parse.go b/pkg/cloudcfg/parse.go index d5b40deca44..4e1f4d0e173 100644 --- a/pkg/cloudcfg/parse.go +++ b/pkg/cloudcfg/parse.go @@ -24,29 +24,7 @@ func ToWireFormat(data []byte, storage string) ([]byte, error) { return nil, fmt.Errorf("unknown storage type: %v", storage) } - // Try parsing json - json_out, json_err := tryJSON(data, reflect.New(prototypeType).Interface()) - if json_err == nil { - return json_out, json_err - } - - // Try parsing yaml - yaml_out, yaml_err := tryYAML(data, reflect.New(prototypeType).Interface()) - if yaml_err != nil { - return nil, fmt.Errorf("Could not parse input as json (error: %v) or yaml (error: %v", json_err, yaml_err) - } - return yaml_out, yaml_err -} - -func tryJSON(data []byte, obj interface{}) ([]byte, error) { - err := json.Unmarshal(data, obj) - if err != nil { - return nil, err - } - return json.Marshal(obj) -} - -func tryYAML(data []byte, obj interface{}) ([]byte, error) { + obj := reflect.New(prototypeType).Interface() err := yaml.Unmarshal(data, obj) if err != nil { return nil, err From 078451053b1c94af32409161be1cfb9d525e0be7 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Thu, 12 Jun 2014 17:25:46 -0700 Subject: [PATCH 7/7] gofmt -s && change gofmt reminder to specify -s in prepare-commit-msg --- hooks/prepare-commit-msg | 2 +- pkg/kubelet/kubelet_test.go | 14 +++++++------- pkg/registry/fake_etcd_client.go | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/hooks/prepare-commit-msg b/hooks/prepare-commit-msg index 0ce78c6ebe5..6ade9aab2d4 100755 --- a/hooks/prepare-commit-msg +++ b/hooks/prepare-commit-msg @@ -10,7 +10,7 @@ for file in $(git diff --cached --name-only | grep "\.go"); do done if [[ $errors == "1" ]]; then - echo "# To fix these errors, run gofmt -w ." >> $1 + echo "# To fix these errors, run gofmt -s -w ." >> $1 echo "# If you want to commit in spite of these format errors," >> $1 echo "# then delete this line. Otherwise, your commit will be" >> $1 echo "# aborted." >> $1 diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 659204f557e..e74036a7865 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -594,11 +594,11 @@ func TestMakeCommandLine(t *testing.T) { func TestMakeEnvVariables(t *testing.T) { container := api.Container{ Env: []api.EnvVar{ - api.EnvVar{ + { Name: "foo", Value: "bar", }, - api.EnvVar{ + { Name: "baz", Value: "blah", }, @@ -619,12 +619,12 @@ func TestMakeEnvVariables(t *testing.T) { func TestMakeVolumesAndBinds(t *testing.T) { container := api.Container{ VolumeMounts: []api.VolumeMount{ - api.VolumeMount{ + { MountPath: "/mnt/path", Name: "disk", ReadOnly: false, }, - api.VolumeMount{ + { MountPath: "/mnt/path2", Name: "disk2", ReadOnly: true, @@ -653,11 +653,11 @@ func TestMakeVolumesAndBinds(t *testing.T) { func TestMakePortsAndBindings(t *testing.T) { container := api.Container{ Ports: []api.Port{ - api.Port{ + { ContainerPort: 80, HostPort: 8080, }, - api.Port{ + { ContainerPort: 443, HostPort: 443, }, @@ -840,7 +840,7 @@ func TestWatchEtcd(t *testing.T) { reader := startReading(changeChannel) manifest := []api.ContainerManifest{ - api.ContainerManifest{ + { Id: "foo", }, } diff --git a/pkg/registry/fake_etcd_client.go b/pkg/registry/fake_etcd_client.go index c03a6443c49..674d8d8db89 100644 --- a/pkg/registry/fake_etcd_client.go +++ b/pkg/registry/fake_etcd_client.go @@ -32,7 +32,7 @@ type FakeEtcdClient struct { deletedKeys []string Err error t *testing.T - Ix int + Ix int } func MakeFakeEtcdClient(t *testing.T) *FakeEtcdClient {