From 89f6f1d0cc9dd886b4a0e65fd673c96cc2d4f90d Mon Sep 17 00:00:00 2001 From: Kenichi Omichi Date: Wed, 13 Feb 2019 01:46:04 +0000 Subject: [PATCH] Fix golint failures under e2e/framework/metrics --- hack/.golint_failures | 1 - test/e2e/framework/framework.go | 2 +- .../framework/metrics/api_server_metrics.go | 18 ++--- .../metrics/cluster_autoscaler_metrics.go | 6 +- .../metrics/controller_manager_metrics.go | 6 +- test/e2e/framework/metrics/generic_metrics.go | 4 +- test/e2e/framework/metrics/kubelet_metrics.go | 11 ++- test/e2e/framework/metrics/metrics_grabber.go | 68 ++++++++++--------- .../framework/metrics/scheduler_metrics.go | 6 +- test/e2e/framework/metrics_util.go | 19 +++--- .../monitoring/metrics_grabber.go | 4 +- test/e2e/storage/volume_metrics.go | 8 +-- 12 files changed, 86 insertions(+), 67 deletions(-) diff --git a/hack/.golint_failures b/hack/.golint_failures index 682576a48a5..3d18fdf9ff5 100644 --- a/hack/.golint_failures +++ b/hack/.golint_failures @@ -682,7 +682,6 @@ test/e2e/cloud test/e2e/common test/e2e/framework test/e2e/framework/ingress -test/e2e/framework/metrics test/e2e/framework/providers/aws test/e2e/framework/providers/azure test/e2e/framework/providers/gce diff --git a/test/e2e/framework/framework.go b/test/e2e/framework/framework.go index 4acc0bf9855..e4c0c0aa42b 100644 --- a/test/e2e/framework/framework.go +++ b/test/e2e/framework/framework.go @@ -119,7 +119,7 @@ type Framework struct { TestSummaries []TestDataSummary // Place to keep ClusterAutoscaler metrics from before test in order to compute delta. - clusterAutoscalerMetricsBeforeTest metrics.MetricsCollection + clusterAutoscalerMetricsBeforeTest metrics.Collection } type TestDataSummary interface { diff --git a/test/e2e/framework/metrics/api_server_metrics.go b/test/e2e/framework/metrics/api_server_metrics.go index edc1d82712a..2305d07da23 100644 --- a/test/e2e/framework/metrics/api_server_metrics.go +++ b/test/e2e/framework/metrics/api_server_metrics.go @@ -16,26 +16,28 @@ limitations under the License. package metrics -type ApiServerMetrics Metrics +// APIServerMetrics is metrics for API server +type APIServerMetrics Metrics -func (m *ApiServerMetrics) Equal(o ApiServerMetrics) bool { +// Equal returns true if all metrics are the same as the arguments. +func (m *APIServerMetrics) Equal(o APIServerMetrics) bool { return (*Metrics)(m).Equal(Metrics(o)) } -func NewApiServerMetrics() ApiServerMetrics { +func newAPIServerMetrics() APIServerMetrics { result := NewMetrics() - return ApiServerMetrics(result) + return APIServerMetrics(result) } -func parseApiServerMetrics(data string) (ApiServerMetrics, error) { - result := NewApiServerMetrics() +func parseAPIServerMetrics(data string) (APIServerMetrics, error) { + result := newAPIServerMetrics() if err := parseMetrics(data, (*Metrics)(&result)); err != nil { - return ApiServerMetrics{}, err + return APIServerMetrics{}, err } return result, nil } -func (g *MetricsGrabber) getMetricsFromApiServer() (string, error) { +func (g *Grabber) getMetricsFromAPIServer() (string, error) { rawOutput, err := g.client.CoreV1().RESTClient().Get().RequestURI("/metrics").Do().Raw() if err != nil { return "", err diff --git a/test/e2e/framework/metrics/cluster_autoscaler_metrics.go b/test/e2e/framework/metrics/cluster_autoscaler_metrics.go index 9aa7dfebecc..0863890fbdb 100644 --- a/test/e2e/framework/metrics/cluster_autoscaler_metrics.go +++ b/test/e2e/framework/metrics/cluster_autoscaler_metrics.go @@ -16,19 +16,21 @@ limitations under the License. package metrics +// ClusterAutoscalerMetrics is metrics for cluster autoscaller type ClusterAutoscalerMetrics Metrics +// Equal returns true if all metrics are the same as the arguments. func (m *ClusterAutoscalerMetrics) Equal(o ClusterAutoscalerMetrics) bool { return (*Metrics)(m).Equal(Metrics(o)) } -func NewClusterAutoscalerMetrics() ClusterAutoscalerMetrics { +func newClusterAutoscalerMetrics() ClusterAutoscalerMetrics { result := NewMetrics() return ClusterAutoscalerMetrics(result) } func parseClusterAutoscalerMetrics(data string) (ClusterAutoscalerMetrics, error) { - result := NewClusterAutoscalerMetrics() + result := newClusterAutoscalerMetrics() if err := parseMetrics(data, (*Metrics)(&result)); err != nil { return ClusterAutoscalerMetrics{}, err } diff --git a/test/e2e/framework/metrics/controller_manager_metrics.go b/test/e2e/framework/metrics/controller_manager_metrics.go index 60f0e9649b8..511189244af 100644 --- a/test/e2e/framework/metrics/controller_manager_metrics.go +++ b/test/e2e/framework/metrics/controller_manager_metrics.go @@ -16,19 +16,21 @@ limitations under the License. package metrics +// ControllerManagerMetrics is metrics for controller manager type ControllerManagerMetrics Metrics +// Equal returns true if all metrics are the same as the arguments. func (m *ControllerManagerMetrics) Equal(o ControllerManagerMetrics) bool { return (*Metrics)(m).Equal(Metrics(o)) } -func NewControllerManagerMetrics() ControllerManagerMetrics { +func newControllerManagerMetrics() ControllerManagerMetrics { result := NewMetrics() return ControllerManagerMetrics(result) } func parseControllerManagerMetrics(data string) (ControllerManagerMetrics, error) { - result := NewControllerManagerMetrics() + result := newControllerManagerMetrics() if err := parseMetrics(data, (*Metrics)(&result)); err != nil { return ControllerManagerMetrics{}, err } diff --git a/test/e2e/framework/metrics/generic_metrics.go b/test/e2e/framework/metrics/generic_metrics.go index 150ad4a9a00..5afcde56c24 100644 --- a/test/e2e/framework/metrics/generic_metrics.go +++ b/test/e2e/framework/metrics/generic_metrics.go @@ -17,7 +17,6 @@ limitations under the License. package metrics import ( - "fmt" "io" "reflect" "strings" @@ -27,8 +26,10 @@ import ( "k8s.io/klog" ) +// Metrics is generic metrics for other specific metrics type Metrics map[string]model.Samples +// Equal returns true if all metrics are the same as the arguments. func (m *Metrics) Equal(o Metrics) bool { leftKeySet := []string{} rightKeySet := []string{} @@ -49,6 +50,7 @@ func (m *Metrics) Equal(o Metrics) bool { return true } +// NewMetrics returns new metrics which are initialized. func NewMetrics() Metrics { result := make(Metrics) return result diff --git a/test/e2e/framework/metrics/kubelet_metrics.go b/test/e2e/framework/metrics/kubelet_metrics.go index cf2d666e8b0..a10a7b9c8a3 100644 --- a/test/e2e/framework/metrics/kubelet_metrics.go +++ b/test/e2e/framework/metrics/kubelet_metrics.go @@ -23,12 +23,19 @@ import ( "time" ) +const ( + proxyTimeout = 2 * time.Minute +) + +// KubeletMetrics is metrics for kubelet type KubeletMetrics Metrics +// Equal returns true if all metrics are the same as the arguments. func (m *KubeletMetrics) Equal(o KubeletMetrics) bool { return (*Metrics)(m).Equal(Metrics(o)) } +// NewKubeletMetrics returns new metrics which are initialized. func NewKubeletMetrics() KubeletMetrics { result := NewMetrics() return KubeletMetrics(result) @@ -58,7 +65,7 @@ func parseKubeletMetrics(data string) (KubeletMetrics, error) { return result, nil } -func (g *MetricsGrabber) getMetricsFromNode(nodeName string, kubeletPort int) (string, error) { +func (g *Grabber) getMetricsFromNode(nodeName string, kubeletPort int) (string, error) { // There's a problem with timing out during proxy. Wrapping this in a goroutine to prevent deadlock. // Hanging goroutine will be leaked. finished := make(chan struct{}) @@ -74,7 +81,7 @@ func (g *MetricsGrabber) getMetricsFromNode(nodeName string, kubeletPort int) (s finished <- struct{}{} }() select { - case <-time.After(ProxyTimeout): + case <-time.After(proxyTimeout): return "", fmt.Errorf("Timed out when waiting for proxy to gather metrics from %v", nodeName) case <-finished: if err != nil { diff --git a/test/e2e/framework/metrics/metrics_grabber.go b/test/e2e/framework/metrics/metrics_grabber.go index 667ecc29d65..d975eb3a0b3 100644 --- a/test/e2e/framework/metrics/metrics_grabber.go +++ b/test/e2e/framework/metrics/metrics_grabber.go @@ -18,7 +18,6 @@ package metrics import ( "fmt" - "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" @@ -30,22 +29,20 @@ import ( "k8s.io/klog" ) -const ( - ProxyTimeout = 2 * time.Minute -) - -type MetricsCollection struct { - ApiServerMetrics ApiServerMetrics +// Collection is metrics collection of components +type Collection struct { + APIServerMetrics APIServerMetrics ControllerManagerMetrics ControllerManagerMetrics KubeletMetrics map[string]KubeletMetrics SchedulerMetrics SchedulerMetrics ClusterAutoscalerMetrics ClusterAutoscalerMetrics } -type MetricsGrabber struct { +// Grabber provides functions which grab metrics from components +type Grabber struct { client clientset.Interface externalClient clientset.Interface - grabFromApiServer bool + grabFromAPIServer bool grabFromControllerManager bool grabFromKubelets bool grabFromScheduler bool @@ -54,7 +51,8 @@ type MetricsGrabber struct { registeredMaster bool } -func NewMetricsGrabber(c clientset.Interface, ec clientset.Interface, kubelets bool, scheduler bool, controllers bool, apiServer bool, clusterAutoscaler bool) (*MetricsGrabber, error) { +// NewMetricsGrabber returns new metrics which are initialized. +func NewMetricsGrabber(c clientset.Interface, ec clientset.Interface, kubelets bool, scheduler bool, controllers bool, apiServer bool, clusterAutoscaler bool) (*Grabber, error) { registeredMaster := false masterName := "" nodeList, err := c.CoreV1().Nodes().List(metav1.ListOptions{}) @@ -82,10 +80,10 @@ func NewMetricsGrabber(c clientset.Interface, ec clientset.Interface, kubelets b } } - return &MetricsGrabber{ + return &Grabber{ client: c, externalClient: ec, - grabFromApiServer: apiServer, + grabFromAPIServer: apiServer, grabFromControllerManager: controllers, grabFromKubelets: kubelets, grabFromScheduler: scheduler, @@ -96,11 +94,12 @@ func NewMetricsGrabber(c clientset.Interface, ec clientset.Interface, kubelets b } // HasRegisteredMaster returns if metrics grabber was able to find a master node -func (g *MetricsGrabber) HasRegisteredMaster() bool { +func (g *Grabber) HasRegisteredMaster() bool { return g.registeredMaster } -func (g *MetricsGrabber) GrabFromKubelet(nodeName string) (KubeletMetrics, error) { +// GrabFromKubelet returns metrics from kubelet +func (g *Grabber) GrabFromKubelet(nodeName string) (KubeletMetrics, error) { nodes, err := g.client.CoreV1().Nodes().List(metav1.ListOptions{FieldSelector: fields.Set{api.ObjectNameField: nodeName}.AsSelector().String()}) if err != nil { return KubeletMetrics{}, err @@ -112,9 +111,9 @@ func (g *MetricsGrabber) GrabFromKubelet(nodeName string) (KubeletMetrics, error return g.grabFromKubeletInternal(nodeName, int(kubeletPort)) } -func (g *MetricsGrabber) grabFromKubeletInternal(nodeName string, kubeletPort int) (KubeletMetrics, error) { +func (g *Grabber) grabFromKubeletInternal(nodeName string, kubeletPort int) (KubeletMetrics, error) { if kubeletPort <= 0 || kubeletPort > 65535 { - return KubeletMetrics{}, fmt.Errorf("Invalid Kubelet port %v. Skipping Kubelet's metrics gathering.", kubeletPort) + return KubeletMetrics{}, fmt.Errorf("Invalid Kubelet port %v. Skipping Kubelet's metrics gathering", kubeletPort) } output, err := g.getMetricsFromNode(nodeName, int(kubeletPort)) if err != nil { @@ -123,9 +122,10 @@ func (g *MetricsGrabber) grabFromKubeletInternal(nodeName string, kubeletPort in return parseKubeletMetrics(output) } -func (g *MetricsGrabber) GrabFromScheduler() (SchedulerMetrics, error) { +// GrabFromScheduler returns metrics from scheduler +func (g *Grabber) GrabFromScheduler() (SchedulerMetrics, error) { if !g.registeredMaster { - return SchedulerMetrics{}, fmt.Errorf("Master's Kubelet is not registered. Skipping Scheduler's metrics gathering.") + return SchedulerMetrics{}, fmt.Errorf("Master's Kubelet is not registered. Skipping Scheduler's metrics gathering") } output, err := g.getMetricsFromPod(g.client, fmt.Sprintf("%v-%v", "kube-scheduler", g.masterName), metav1.NamespaceSystem, ports.InsecureSchedulerPort) if err != nil { @@ -134,9 +134,10 @@ func (g *MetricsGrabber) GrabFromScheduler() (SchedulerMetrics, error) { return parseSchedulerMetrics(output) } -func (g *MetricsGrabber) GrabFromClusterAutoscaler() (ClusterAutoscalerMetrics, error) { +// GrabFromClusterAutoscaler returns metrics from cluster autoscaler +func (g *Grabber) GrabFromClusterAutoscaler() (ClusterAutoscalerMetrics, error) { if !g.registeredMaster && g.externalClient == nil { - return ClusterAutoscalerMetrics{}, fmt.Errorf("Master's Kubelet is not registered. Skipping ClusterAutoscaler's metrics gathering.") + return ClusterAutoscalerMetrics{}, fmt.Errorf("Master's Kubelet is not registered. Skipping ClusterAutoscaler's metrics gathering") } var client clientset.Interface var namespace string @@ -154,9 +155,10 @@ func (g *MetricsGrabber) GrabFromClusterAutoscaler() (ClusterAutoscalerMetrics, return parseClusterAutoscalerMetrics(output) } -func (g *MetricsGrabber) GrabFromControllerManager() (ControllerManagerMetrics, error) { +// GrabFromControllerManager returns metrics from controller manager +func (g *Grabber) GrabFromControllerManager() (ControllerManagerMetrics, error) { if !g.registeredMaster { - return ControllerManagerMetrics{}, fmt.Errorf("Master's Kubelet is not registered. Skipping ControllerManager's metrics gathering.") + return ControllerManagerMetrics{}, fmt.Errorf("Master's Kubelet is not registered. Skipping ControllerManager's metrics gathering") } output, err := g.getMetricsFromPod(g.client, fmt.Sprintf("%v-%v", "kube-controller-manager", g.masterName), metav1.NamespaceSystem, ports.InsecureKubeControllerManagerPort) if err != nil { @@ -165,23 +167,25 @@ func (g *MetricsGrabber) GrabFromControllerManager() (ControllerManagerMetrics, return parseControllerManagerMetrics(output) } -func (g *MetricsGrabber) GrabFromApiServer() (ApiServerMetrics, error) { - output, err := g.getMetricsFromApiServer() +// GrabFromAPIServer returns metrics from API server +func (g *Grabber) GrabFromAPIServer() (APIServerMetrics, error) { + output, err := g.getMetricsFromAPIServer() if err != nil { - return ApiServerMetrics{}, nil + return APIServerMetrics{}, nil } - return parseApiServerMetrics(output) + return parseAPIServerMetrics(output) } -func (g *MetricsGrabber) Grab() (MetricsCollection, error) { - result := MetricsCollection{} +// Grab returns metrics from corresponding component +func (g *Grabber) Grab() (Collection, error) { + result := Collection{} var errs []error - if g.grabFromApiServer { - metrics, err := g.GrabFromApiServer() + if g.grabFromAPIServer { + metrics, err := g.GrabFromAPIServer() if err != nil { errs = append(errs, err) } else { - result.ApiServerMetrics = metrics + result.APIServerMetrics = metrics } } if g.grabFromScheduler { @@ -230,7 +234,7 @@ func (g *MetricsGrabber) Grab() (MetricsCollection, error) { return result, nil } -func (g *MetricsGrabber) getMetricsFromPod(client clientset.Interface, podName string, namespace string, port int) (string, error) { +func (g *Grabber) getMetricsFromPod(client clientset.Interface, podName string, namespace string, port int) (string, error) { rawOutput, err := client.CoreV1().RESTClient().Get(). Namespace(namespace). Resource("pods"). diff --git a/test/e2e/framework/metrics/scheduler_metrics.go b/test/e2e/framework/metrics/scheduler_metrics.go index 6fb7ae6614a..7378ea902e4 100644 --- a/test/e2e/framework/metrics/scheduler_metrics.go +++ b/test/e2e/framework/metrics/scheduler_metrics.go @@ -16,19 +16,21 @@ limitations under the License. package metrics +// SchedulerMetrics is metrics for scheduler type SchedulerMetrics Metrics +// Equal returns true if all metrics are the same as the arguments. func (m *SchedulerMetrics) Equal(o SchedulerMetrics) bool { return (*Metrics)(m).Equal(Metrics(o)) } -func NewSchedulerMetrics() SchedulerMetrics { +func newSchedulerMetrics() SchedulerMetrics { result := NewMetrics() return SchedulerMetrics(result) } func parseSchedulerMetrics(data string) (SchedulerMetrics, error) { - result := NewSchedulerMetrics() + result := newSchedulerMetrics() if err := parseMetrics(data, (*Metrics)(&result)); err != nil { return SchedulerMetrics{}, err } diff --git a/test/e2e/framework/metrics_util.go b/test/e2e/framework/metrics_util.go index aac99035144..d6a7099d727 100644 --- a/test/e2e/framework/metrics_util.go +++ b/test/e2e/framework/metrics_util.go @@ -65,12 +65,12 @@ const ( caFunctionMetricLabel = "function" ) -type MetricsForE2E metrics.MetricsCollection +type MetricsForE2E metrics.Collection func (m *MetricsForE2E) filterMetrics() { - interestingApiServerMetrics := make(metrics.ApiServerMetrics) - for _, metric := range InterestingApiServerMetrics { - interestingApiServerMetrics[metric] = (*m).ApiServerMetrics[metric] + interestingAPIServerMetrics := make(metrics.APIServerMetrics) + for _, metric := range InterestingAPIServerMetrics { + interestingAPIServerMetrics[metric] = (*m).APIServerMetrics[metric] } interestingControllerManagerMetrics := make(metrics.ControllerManagerMetrics) for _, metric := range InterestingControllerManagerMetrics { @@ -87,7 +87,7 @@ func (m *MetricsForE2E) filterMetrics() { interestingKubeletMetrics[kubelet][metric] = grabbed[metric] } } - (*m).ApiServerMetrics = interestingApiServerMetrics + (*m).APIServerMetrics = interestingAPIServerMetrics (*m).ControllerManagerMetrics = interestingControllerManagerMetrics (*m).KubeletMetrics = interestingKubeletMetrics } @@ -112,12 +112,11 @@ func printSample(sample *model.Sample) string { return fmt.Sprintf("[%v] = %v", strings.Join(buf, ","), sample.Value) } - func (m *MetricsForE2E) PrintHumanReadable() string { buf := bytes.Buffer{} - for _, interestingMetric := range InterestingApiServerMetrics { + for _, interestingMetric := range InterestingAPIServerMetrics { buf.WriteString(fmt.Sprintf("For %v:\n", interestingMetric)) - for _, sample := range (*m).ApiServerMetrics[interestingMetric] { + for _, sample := range (*m).APIServerMetrics[interestingMetric] { buf.WriteString(fmt.Sprintf("\t%v\n", printSample(sample))) } } @@ -156,7 +155,7 @@ func (m *MetricsForE2E) SummaryKind() string { var SchedulingLatencyMetricName = model.LabelValue(schedulermetric.SchedulerSubsystem + "_" + schedulermetric.SchedulingLatencyName) -var InterestingApiServerMetrics = []string{ +var InterestingAPIServerMetrics = []string{ "apiserver_request_total", // TODO(krzysied): apiserver_request_latencies_summary is a deprecated metric. // It should be replaced with new metric. @@ -815,7 +814,7 @@ func PrintLatencies(latencies []PodLatencyData, header string) { Logf("perc50: %v, perc90: %v, perc99: %v", metrics.Perc50, metrics.Perc90, metrics.Perc99) } -func (m *MetricsForE2E) computeClusterAutoscalerMetricsDelta(before metrics.MetricsCollection) { +func (m *MetricsForE2E) computeClusterAutoscalerMetricsDelta(before metrics.Collection) { if beforeSamples, found := before.ClusterAutoscalerMetrics[caFunctionMetric]; found { if afterSamples, found := m.ClusterAutoscalerMetrics[caFunctionMetric]; found { beforeSamplesMap := make(map[string]*model.Sample) diff --git a/test/e2e/instrumentation/monitoring/metrics_grabber.go b/test/e2e/instrumentation/monitoring/metrics_grabber.go index e0d038a6a46..afcc9039811 100644 --- a/test/e2e/instrumentation/monitoring/metrics_grabber.go +++ b/test/e2e/instrumentation/monitoring/metrics_grabber.go @@ -32,7 +32,7 @@ import ( var _ = instrumentation.SIGDescribe("MetricsGrabber", func() { f := framework.NewDefaultFramework("metrics-grabber") var c, ec clientset.Interface - var grabber *metrics.MetricsGrabber + var grabber *metrics.Grabber gin.BeforeEach(func() { var err error c = f.ClientSet @@ -44,7 +44,7 @@ var _ = instrumentation.SIGDescribe("MetricsGrabber", func() { gin.It("should grab all metrics from API server.", func() { gin.By("Connecting to /metrics endpoint") - response, err := grabber.GrabFromApiServer() + response, err := grabber.GrabFromAPIServer() framework.ExpectNoError(err) gom.Expect(response).NotTo(gom.BeEmpty()) }) diff --git a/test/e2e/storage/volume_metrics.go b/test/e2e/storage/volume_metrics.go index 8c1dccc2d05..06a83ccb178 100644 --- a/test/e2e/storage/volume_metrics.go +++ b/test/e2e/storage/volume_metrics.go @@ -42,7 +42,7 @@ var _ = utils.SIGDescribe("[Serial] Volume metrics", func() { c clientset.Interface ns string pvc *v1.PersistentVolumeClaim - metricsGrabber *metrics.MetricsGrabber + metricsGrabber *metrics.Grabber ) f := framework.NewDefaultFramework("pv") @@ -422,7 +422,7 @@ var _ = utils.SIGDescribe("[Serial] Volume metrics", func() { }) }) -func waitForDetachAndGrabMetrics(oldMetrics map[string]int64, metricsGrabber *metrics.MetricsGrabber) map[string]int64 { +func waitForDetachAndGrabMetrics(oldMetrics map[string]int64, metricsGrabber *metrics.Grabber) map[string]int64 { backoff := wait.Backoff{ Duration: 10 * time.Second, Factor: 1.2, @@ -527,7 +527,7 @@ func findVolumeStatMetric(metricKeyName string, namespace string, pvcName string } // Wait for the count of a pv controller's metric specified by metricName and dimension bigger than zero. -func waitForPVControllerSync(metricsGrabber *metrics.MetricsGrabber, metricName, dimension string) { +func waitForPVControllerSync(metricsGrabber *metrics.Grabber, metricName, dimension string) { backoff := wait.Backoff{ Duration: 10 * time.Second, Factor: 1.2, @@ -608,7 +608,7 @@ func getStatesMetrics(metricKey string, givenMetrics metrics.Metrics) map[string return states } -func waitForADControllerStatesMetrics(metricsGrabber *metrics.MetricsGrabber, metricName string, dimensions []string, stateNames []string) { +func waitForADControllerStatesMetrics(metricsGrabber *metrics.Grabber, metricName string, dimensions []string, stateNames []string) { backoff := wait.Backoff{ Duration: 10 * time.Second, Factor: 1.2,