From 5b8b1eb2ebad7b5ba3ffd6852afed0bccee5d88a Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Tue, 15 Aug 2017 14:59:56 -0400 Subject: [PATCH 1/2] allow default option values - kube top node|pod --- pkg/kubectl/cmd/top.go | 4 ++-- pkg/kubectl/cmd/top_node.go | 27 +++++++++++++++++++++------ pkg/kubectl/cmd/top_node_test.go | 6 +++--- pkg/kubectl/cmd/top_pod.go | 6 ++++-- pkg/kubectl/cmd/top_pod_test.go | 2 +- 5 files changed, 31 insertions(+), 14 deletions(-) diff --git a/pkg/kubectl/cmd/top.go b/pkg/kubectl/cmd/top.go index 1473f8e8a54..48489cfd789 100644 --- a/pkg/kubectl/cmd/top.go +++ b/pkg/kubectl/cmd/top.go @@ -47,7 +47,7 @@ func NewCmdTop(f cmdutil.Factory, out, errOut io.Writer) *cobra.Command { } // create subcommands - cmd.AddCommand(NewCmdTopNode(f, out)) - cmd.AddCommand(NewCmdTopPod(f, out)) + cmd.AddCommand(NewCmdTopNode(f, nil, out)) + cmd.AddCommand(NewCmdTopPod(f, nil, out)) return cmd } diff --git a/pkg/kubectl/cmd/top_node.go b/pkg/kubectl/cmd/top_node.go index e432a457622..668a96ffa74 100644 --- a/pkg/kubectl/cmd/top_node.go +++ b/pkg/kubectl/cmd/top_node.go @@ -50,10 +50,23 @@ type HeapsterTopOptions struct { } func (o *HeapsterTopOptions) Bind(flags *pflag.FlagSet) { - flags.StringVar(&o.Namespace, "heapster-namespace", metricsutil.DefaultHeapsterNamespace, "Namespace Heapster service is located in") - flags.StringVar(&o.Service, "heapster-service", metricsutil.DefaultHeapsterService, "Name of Heapster service") - flags.StringVar(&o.Scheme, "heapster-scheme", metricsutil.DefaultHeapsterScheme, "Scheme (http or https) to connect to Heapster as") - flags.StringVar(&o.Port, "heapster-port", metricsutil.DefaultHeapsterPort, "Port name in service to use") + if len(o.Namespace) == 0 { + o.Namespace = metricsutil.DefaultHeapsterNamespace + } + if len(o.Service) == 0 { + o.Service = metricsutil.DefaultHeapsterService + } + if len(o.Scheme) == 0 { + o.Scheme = metricsutil.DefaultHeapsterScheme + } + if len(o.Port) == 0 { + o.Port = metricsutil.DefaultHeapsterPort + } + + flags.StringVar(&o.Namespace, "heapster-namespace", o.Namespace, "Namespace Heapster service is located in") + flags.StringVar(&o.Service, "heapster-service", o.Service, "Name of Heapster service") + flags.StringVar(&o.Scheme, "heapster-scheme", o.Scheme, "Scheme (http or https) to connect to Heapster as") + flags.StringVar(&o.Port, "heapster-port", o.Port, "Port name in service to use") } var ( @@ -70,8 +83,10 @@ var ( kubectl top node NODE_NAME`)) ) -func NewCmdTopNode(f cmdutil.Factory, out io.Writer) *cobra.Command { - options := &TopNodeOptions{} +func NewCmdTopNode(f cmdutil.Factory, options *TopNodeOptions, out io.Writer) *cobra.Command { + if options == nil { + options = &TopNodeOptions{} + } cmd := &cobra.Command{ Use: "node [NAME | -l label]", diff --git a/pkg/kubectl/cmd/top_node_test.go b/pkg/kubectl/cmd/top_node_test.go index 354afde1055..e2e9af185cb 100644 --- a/pkg/kubectl/cmd/top_node_test.go +++ b/pkg/kubectl/cmd/top_node_test.go @@ -67,7 +67,7 @@ func TestTopNodeAllMetrics(t *testing.T) { tf.ClientConfig = defaultClientConfig() buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdTopNode(f, buf) + cmd := NewCmdTopNode(f, nil, buf) cmd.Run(cmd, []string{}) // Check the presence of node names in the output. @@ -116,7 +116,7 @@ func TestTopNodeWithNameMetrics(t *testing.T) { tf.ClientConfig = defaultClientConfig() buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdTopNode(f, buf) + cmd := NewCmdTopNode(f, nil, buf) cmd.Run(cmd, []string{expectedMetrics.Name}) // Check the presence of node names in the output. @@ -176,7 +176,7 @@ func TestTopNodeWithLabelSelectorMetrics(t *testing.T) { tf.ClientConfig = defaultClientConfig() buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdTopNode(f, buf) + cmd := NewCmdTopNode(f, nil, buf) cmd.Flags().Set("selector", label) cmd.Run(cmd, []string{}) diff --git a/pkg/kubectl/cmd/top_pod.go b/pkg/kubectl/cmd/top_pod.go index 689e6e3e2fe..e17b02be764 100644 --- a/pkg/kubectl/cmd/top_pod.go +++ b/pkg/kubectl/cmd/top_pod.go @@ -72,8 +72,10 @@ var ( kubectl top pod -l name=myLabel`)) ) -func NewCmdTopPod(f cmdutil.Factory, out io.Writer) *cobra.Command { - options := &TopPodOptions{} +func NewCmdTopPod(f cmdutil.Factory, options *TopPodOptions, out io.Writer) *cobra.Command { + if options == nil { + options = &TopPodOptions{} + } cmd := &cobra.Command{ Use: "pod [NAME | -l label]", diff --git a/pkg/kubectl/cmd/top_pod_test.go b/pkg/kubectl/cmd/top_pod_test.go index d7a43fc1460..ef892887d23 100644 --- a/pkg/kubectl/cmd/top_pod_test.go +++ b/pkg/kubectl/cmd/top_pod_test.go @@ -139,7 +139,7 @@ func TestTopPod(t *testing.T) { tf.ClientConfig = defaultClientConfig() buf := bytes.NewBuffer([]byte{}) - cmd := NewCmdTopPod(f, buf) + cmd := NewCmdTopPod(f, nil, buf) for name, value := range testCase.flags { cmd.Flags().Set(name, value) } From c969079f49397b01e471bde036f6556b5af4b683 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Thu, 17 Aug 2017 18:08:26 -0400 Subject: [PATCH 2/2] add tests --- pkg/kubectl/cmd/top_node_test.go | 53 ++++++++++++ pkg/kubectl/cmd/top_pod_test.go | 143 +++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+) diff --git a/pkg/kubectl/cmd/top_node_test.go b/pkg/kubectl/cmd/top_node_test.go index e2e9af185cb..ba292a63bb1 100644 --- a/pkg/kubectl/cmd/top_node_test.go +++ b/pkg/kubectl/cmd/top_node_test.go @@ -79,6 +79,59 @@ func TestTopNodeAllMetrics(t *testing.T) { } } +func TestTopNodeAllMetricsCustomDefaults(t *testing.T) { + customBaseHeapsterServiceAddress := "/api/v1/namespaces/custom-namespace/services/https:custom-heapster-service:/proxy" + customBaseMetricsAddress := customBaseHeapsterServiceAddress + "/apis/metrics" + + initTestErrorHandler(t) + metrics, nodes := testNodeMetricsData() + expectedMetricsPath := fmt.Sprintf("%s/%s/nodes", customBaseMetricsAddress, metricsApiVersion) + expectedNodePath := fmt.Sprintf("/%s/%s/nodes", apiPrefix, apiVersion) + + f, tf, codec, ns := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + tf.Client = &fake.RESTClient{ + APIRegistry: api.Registry, + NegotiatedSerializer: ns, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m := req.URL.Path, req.Method; { + case p == expectedMetricsPath && m == "GET": + body, err := marshallBody(metrics) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil + case p == expectedNodePath && m == "GET": + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: objBody(codec, nodes)}, nil + default: + t.Fatalf("unexpected request: %#v\nGot URL: %#v\nExpected path: %#v", req, req.URL, expectedMetricsPath) + return nil, nil + } + }), + } + tf.Namespace = "test" + tf.ClientConfig = defaultClientConfig() + buf := bytes.NewBuffer([]byte{}) + + opts := &TopNodeOptions{ + HeapsterOptions: HeapsterTopOptions{ + Namespace: "custom-namespace", + Scheme: "https", + Service: "custom-heapster-service", + }, + } + cmd := NewCmdTopNode(f, opts, buf) + cmd.Run(cmd, []string{}) + + // Check the presence of node names in the output. + result := buf.String() + for _, m := range metrics.Items { + if !strings.Contains(result, m.Name) { + t.Errorf("missing metrics for %s: \n%s", m.Name, result) + } + } +} + func TestTopNodeWithNameMetrics(t *testing.T) { initTestErrorHandler(t) metrics, nodes := testNodeMetricsData() diff --git a/pkg/kubectl/cmd/top_pod_test.go b/pkg/kubectl/cmd/top_pod_test.go index ef892887d23..3766123fb79 100644 --- a/pkg/kubectl/cmd/top_pod_test.go +++ b/pkg/kubectl/cmd/top_pod_test.go @@ -170,6 +170,149 @@ func TestTopPod(t *testing.T) { } } +func TestTopPodCustomDefaults(t *testing.T) { + customBaseHeapsterServiceAddress := "/api/v1/namespaces/custom-namespace/services/https:custom-heapster-service:/proxy" + customBaseMetricsAddress := customBaseHeapsterServiceAddress + "/apis/metrics" + customTopPathPrefix := customBaseMetricsAddress + "/" + metricsApiVersion + + testNS := "custom-namespace" + testCases := []struct { + name string + namespace string + flags map[string]string + args []string + expectedPath string + expectedQuery string + namespaces []string + containers bool + listsNamespaces bool + }{ + { + name: "all namespaces", + flags: map[string]string{"all-namespaces": "true"}, + expectedPath: customTopPathPrefix + "/pods", + namespaces: []string{testNS, "secondtestns", "thirdtestns"}, + listsNamespaces: true, + }, + { + name: "all in namespace", + expectedPath: customTopPathPrefix + "/namespaces/" + testNS + "/pods", + namespaces: []string{testNS, testNS}, + }, + { + name: "pod with name", + args: []string{"pod1"}, + expectedPath: customTopPathPrefix + "/namespaces/" + testNS + "/pods/pod1", + namespaces: []string{testNS}, + }, + { + name: "pod with label selector", + flags: map[string]string{"selector": "key=value"}, + expectedPath: customTopPathPrefix + "/namespaces/" + testNS + "/pods", + expectedQuery: "labelSelector=" + url.QueryEscape("key=value"), + namespaces: []string{testNS, testNS}, + }, + { + name: "pod with container metrics", + flags: map[string]string{"containers": "true"}, + args: []string{"pod1"}, + expectedPath: customTopPathPrefix + "/namespaces/" + testNS + "/pods/pod1", + namespaces: []string{testNS}, + containers: true, + }, + } + initTestErrorHandler(t) + for _, testCase := range testCases { + t.Logf("Running test case: %s", testCase.name) + metricsList := testPodMetricsData() + var expectedMetrics []metricsapi.PodMetrics + var expectedContainerNames, nonExpectedMetricsNames []string + for n, m := range metricsList { + if n < len(testCase.namespaces) { + m.Namespace = testCase.namespaces[n] + expectedMetrics = append(expectedMetrics, m) + for _, c := range m.Containers { + expectedContainerNames = append(expectedContainerNames, c.Name) + } + } else { + nonExpectedMetricsNames = append(nonExpectedMetricsNames, m.Name) + } + } + + var response interface{} + if len(expectedMetrics) == 1 { + response = expectedMetrics[0] + } else { + response = metricsapi.PodMetricsList{ + ListMeta: metav1.ListMeta{ + ResourceVersion: "2", + }, + Items: expectedMetrics, + } + } + + f, tf, _, ns := cmdtesting.NewAPIFactory() + tf.Printer = &testPrinter{} + tf.Client = &fake.RESTClient{ + APIRegistry: api.Registry, + NegotiatedSerializer: ns, + Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) { + switch p, m, q := req.URL.Path, req.Method, req.URL.RawQuery; { + case p == testCase.expectedPath && m == "GET" && (testCase.expectedQuery == "" || q == testCase.expectedQuery): + body, err := marshallBody(response) + if err != nil { + t.Errorf("%s: unexpected error: %v", testCase.name, err) + } + return &http.Response{StatusCode: 200, Header: defaultHeader(), Body: body}, nil + default: + t.Fatalf("%s: unexpected request: %#v\nGot URL: %#v\nExpected path: %#v\nExpected query: %#v", + testCase.name, req, req.URL, testCase.expectedPath, testCase.expectedQuery) + return nil, nil + } + }), + } + tf.Namespace = testNS + tf.ClientConfig = defaultClientConfig() + buf := bytes.NewBuffer([]byte{}) + + opts := &TopPodOptions{ + HeapsterOptions: HeapsterTopOptions{ + Namespace: "custom-namespace", + Scheme: "https", + Service: "custom-heapster-service", + }, + } + cmd := NewCmdTopPod(f, opts, buf) + for name, value := range testCase.flags { + cmd.Flags().Set(name, value) + } + cmd.Run(cmd, testCase.args) + + // Check the presence of pod names&namespaces/container names in the output. + result := buf.String() + if testCase.containers { + for _, containerName := range expectedContainerNames { + if !strings.Contains(result, containerName) { + t.Errorf("%s: missing metrics for container %s: \n%s", testCase.name, containerName, result) + } + } + } + for _, m := range expectedMetrics { + if !strings.Contains(result, m.Name) { + t.Errorf("%s: missing metrics for %s: \n%s", testCase.name, m.Name, result) + } + if testCase.listsNamespaces && !strings.Contains(result, m.Namespace) { + t.Errorf("%s: missing metrics for %s/%s: \n%s", testCase.name, m.Namespace, m.Name, result) + } + } + for _, name := range nonExpectedMetricsNames { + if strings.Contains(result, name) { + t.Errorf("%s: unexpected metrics for %s: \n%s", testCase.name, name, result) + } + } + } +} + func testPodMetricsData() []metricsapi.PodMetrics { return []metricsapi.PodMetrics{ {