From 1847d9bed62ffa181d9035f834828f3a62909848 Mon Sep 17 00:00:00 2001 From: Olivier Lemasle Date: Mon, 21 Nov 2022 14:46:20 +0100 Subject: [PATCH] Use Golangci-lint --- .golangci.yml | 36 ++++++++ Makefile | 44 ++++++++-- hack/gofmt-all.sh | 41 --------- pkg/apiserver/apiserver.go | 13 +-- pkg/apiserver/cmapis.go | 2 +- pkg/apiserver/emapis.go | 2 +- pkg/apiserver/endpoints/handlers/get.go | 3 +- pkg/apiserver/installer/apiserver_test.go | 89 +++++++++----------- pkg/apiserver/installer/cmhandlers.go | 5 +- pkg/apiserver/installer/conversion.go | 8 +- pkg/apiserver/installer/emhandlers.go | 6 +- pkg/apiserver/installer/installer.go | 6 +- pkg/cmd/builder.go | 5 +- pkg/cmd/builder_test.go | 1 + pkg/dynamicmapper/mapper.go | 9 +- pkg/provider/errors.go | 6 +- pkg/provider/fake/fake.go | 1 + pkg/provider/interfaces.go | 5 +- pkg/provider/interfaces_test.go | 1 + pkg/registry/custom_metrics/reststorage.go | 9 +- pkg/registry/external_metrics/reststorage.go | 1 + test-adapter/main.go | 13 ++- test-adapter/provider/provider.go | 10 ++- 23 files changed, 174 insertions(+), 142 deletions(-) create mode 100644 .golangci.yml delete mode 100755 hack/gofmt-all.sh diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 000000000..e002758f3 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,36 @@ +run: + deadline: 5m + +linters: + disable-all: true + enable: + - bodyclose + - dogsled + - dupl + - errcheck + - exportloopref + - gocritic + - gocyclo + - gofmt + - goimports + - gosec + - goprintffuncname + - gosimple + - govet + - ineffassign + - misspell + - nakedret + - nolintlint + - revive + - staticcheck + - typecheck + - unconvert + - unused + - whitespace + +linters-settings: + goimports: + local-prefixes: sigs.k8s.io/custom-metrics-apiserver + misspell: + ignore-words: + - "creater" # Cf. e.g. https://pkg.go.dev/k8s.io/apimachinery@v0.24.3/pkg/runtime#ObjectCreater diff --git a/Makefile b/Makefile index 6e4bfb6b5..8ddbc4b3f 100644 --- a/Makefile +++ b/Makefile @@ -3,12 +3,19 @@ IMAGE?=k8s-test-metrics-adapter TEMP_DIR:=$(shell mktemp -d) ARCH?=amd64 OUT_DIR?=./_output +GOPATH:=$(shell go env GOPATH) VERSION?=latest +GOLANGCI_VERSION:=1.50.1 + .PHONY: all all: build-test-adapter + +# Generate +# -------- + generated_openapis := core custommetrics externalmetrics generated_files := $(generated_openapis:%=pkg/generated/openapi/%/zz_generated.openapi.go) @@ -26,16 +33,39 @@ pkg/generated/openapi/%/zz_generated.openapi.go: go.mod go.sum -o ./ \ -r /dev/null + +# Build +# ----- + .PHONY: build-test-adapter build-test-adapter: $(generated_files) CGO_ENABLED=0 GOOS=linux GOARCH=$(ARCH) go build -o $(OUT_DIR)/$(ARCH)/test-adapter sigs.k8s.io/custom-metrics-apiserver/test-adapter -.PHONY: gofmt -gofmt: - ./hack/gofmt-all.sh + +# Format and lint +# --------------- + +HAS_GOLANGCI_VERSION:=$(shell $(GOPATH)/bin/golangci-lint version --format=short) +.PHONY: golangci +golangci: +ifneq ($(HAS_GOLANGCI_VERSION), $(GOLANGCI_VERSION)) + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(GOPATH)/bin v$(GOLANGCI_VERSION) +endif + +.PHONY: verify-lint +verify-lint: golangci + $(GOPATH)/bin/golangci-lint run --modules-download-mode=readonly || (echo 'Run "make update-lint"' && exit 1) + +.PHONY: update-lint +update-lint: golangci + $(GOPATH)/bin/golangci-lint run --fix --modules-download-mode=readonly + + +# Verify +# ------ .PHONY: verify -verify: verify-deps verify-gofmt +verify: verify-deps verify-lint .PHONY: verify-deps verify-deps: @@ -43,9 +73,9 @@ verify-deps: go mod tidy @git diff --exit-code -- go.sum go.mod -.PHONY: verify-gofmt -verify-gofmt: - ./hack/gofmt-all.sh -v + +# Test +# ---- .PHONY: test test: diff --git a/hack/gofmt-all.sh b/hack/gofmt-all.sh deleted file mode 100755 index 1dbfc3441..000000000 --- a/hack/gofmt-all.sh +++ /dev/null @@ -1,41 +0,0 @@ -#!/bin/bash - -# Copyright 2017 The Kubernetes 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. - -set -o errexit -set -o nounset -set -o pipefail - -verify=0 -if [[ ${1:-} = "--verify" || ${1:-} = "-v" ]]; then - verify=1 -fi - -find_files() { - find . -not \( \( \ - -wholename './_output' \ - -o -wholename './vendor' \ - \) -prune \) -name '*.go' -} - -if [[ $verify -eq 1 ]]; then - diff=$(find_files | xargs gofmt -s -d 2>&1) - if [[ -n "${diff}" ]]; then - echo "gofmt -s -w $(echo "${diff}" | awk '/^diff / { print $2 }' | tr '\n' ' ')" - exit 1 - fi -else - find_files | xargs gofmt -s -w -fi diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index f7fba4a5d..834770baf 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -21,12 +21,13 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/version" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/client-go/informers" - cminstall "k8s.io/metrics/pkg/apis/custom_metrics/install" eminstall "k8s.io/metrics/pkg/apis/external_metrics/install" + "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/installer" "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" ) @@ -41,7 +42,7 @@ func init() { eminstall.Install(Scheme) // we need custom conversion functions to list resources with options - installer.RegisterConversions(Scheme) + utilruntime.Must(installer.RegisterConversions(Scheme)) // we need to add the options to empty v1 // TODO fix the server code to avoid this @@ -69,24 +70,24 @@ type CustomMetricsAdapterServer struct { externalMetricsProvider provider.ExternalMetricsProvider } -type completedConfig struct { +type CompletedConfig struct { genericapiserver.CompletedConfig } // Complete fills in any fields not set that are required to have valid data. It's mutating the receiver. -func (c *Config) Complete(informers informers.SharedInformerFactory) completedConfig { +func (c *Config) Complete(informers informers.SharedInformerFactory) CompletedConfig { c.GenericConfig.Version = &version.Info{ Major: "1", Minor: "0", } - return completedConfig{c.GenericConfig.Complete(informers)} + return CompletedConfig{c.GenericConfig.Complete(informers)} } // New returns a new instance of CustomMetricsAdapterServer from the given config. // name is used to differentiate for logging. // Each of the arguments: customMetricsProvider, externalMetricsProvider can be set either to // a provider implementation, or to nil to disable one of the APIs. -func (c completedConfig) New(name string, customMetricsProvider provider.CustomMetricsProvider, externalMetricsProvider provider.ExternalMetricsProvider) (*CustomMetricsAdapterServer, error) { +func (c CompletedConfig) New(name string, customMetricsProvider provider.CustomMetricsProvider, externalMetricsProvider provider.ExternalMetricsProvider) (*CustomMetricsAdapterServer, error) { genericServer, err := c.CompletedConfig.New(name, genericapiserver.NewEmptyDelegate()) // completion is done in Complete, no need for a second time if err != nil { return nil, err diff --git a/pkg/apiserver/cmapis.go b/pkg/apiserver/cmapis.go index 3e63db31d..22cbe22e7 100644 --- a/pkg/apiserver/cmapis.go +++ b/pkg/apiserver/cmapis.go @@ -24,8 +24,8 @@ import ( genericapi "k8s.io/apiserver/pkg/endpoints" "k8s.io/apiserver/pkg/endpoints/discovery" genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/metrics/pkg/apis/custom_metrics" + specificapi "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/installer" "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" metricstorage "sigs.k8s.io/custom-metrics-apiserver/pkg/registry/custom_metrics" diff --git a/pkg/apiserver/emapis.go b/pkg/apiserver/emapis.go index 617c30753..ea8975c53 100644 --- a/pkg/apiserver/emapis.go +++ b/pkg/apiserver/emapis.go @@ -24,8 +24,8 @@ import ( genericapi "k8s.io/apiserver/pkg/endpoints" "k8s.io/apiserver/pkg/endpoints/discovery" genericapiserver "k8s.io/apiserver/pkg/server" - "k8s.io/metrics/pkg/apis/external_metrics" + specificapi "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/installer" "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" metricstorage "sigs.k8s.io/custom-metrics-apiserver/pkg/registry/external_metrics" diff --git a/pkg/apiserver/endpoints/handlers/get.go b/pkg/apiserver/endpoints/handlers/get.go index 9b73a68c4..d0fab3367 100644 --- a/pkg/apiserver/endpoints/handlers/get.go +++ b/pkg/apiserver/endpoints/handlers/get.go @@ -17,7 +17,6 @@ limitations under the License. package handlers import ( - "fmt" "net/http" "net/url" "strings" @@ -120,7 +119,7 @@ func ListResourceWithOptions(r cm_rest.ListerWithOptions, scope handlers.Request trace.Step("Listing from storage done") responsewriters.WriteObjectNegotiated(scope.Serializer, negotiation.DefaultEndpointRestrictions, scope.Kind.GroupVersion(), w, req, http.StatusOK, result) - trace.Step(fmt.Sprintf("Writing http response done")) + trace.Step("Writing http response done") } } diff --git a/pkg/apiserver/installer/apiserver_test.go b/pkg/apiserver/installer/apiserver_test.go index 433060f58..d8daa4915 100644 --- a/pkg/apiserver/installer/apiserver_test.go +++ b/pkg/apiserver/installer/apiserver_test.go @@ -19,7 +19,7 @@ package installer import ( "context" "fmt" - "io/ioutil" + "io" "net/http" "net/http/httptest" "testing" @@ -33,16 +33,17 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/runtime/serializer" "k8s.io/apimachinery/pkg/types" + utilruntime "k8s.io/apimachinery/pkg/util/runtime" genericapi "k8s.io/apiserver/pkg/endpoints" genericapifilters "k8s.io/apiserver/pkg/endpoints/filters" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/metrics/pkg/apis/custom_metrics" installcm "k8s.io/metrics/pkg/apis/custom_metrics/install" cmv1beta1 "k8s.io/metrics/pkg/apis/custom_metrics/v1beta1" + "k8s.io/metrics/pkg/apis/external_metrics" installem "k8s.io/metrics/pkg/apis/external_metrics/install" emv1beta1 "k8s.io/metrics/pkg/apis/external_metrics/v1beta1" - "k8s.io/metrics/pkg/apis/external_metrics" "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" custommetricstorage "sigs.k8s.io/custom-metrics-apiserver/pkg/registry/custom_metrics" externalmetricstorage "sigs.k8s.io/custom-metrics-apiserver/pkg/registry/external_metrics" @@ -73,7 +74,7 @@ func init() { installem.Install(Scheme) // we need custom conversion functions to list resources with options - RegisterConversions(Scheme) + utilruntime.Must(RegisterConversions(Scheme)) // we need to add the options to empty v1 // TODO fix the server code to avoid this @@ -97,7 +98,7 @@ func init() { func extractBody(response *http.Response, object runtime.Object) error { defer response.Body.Close() - body, err := ioutil.ReadAll(response.Body) + body, err := io.ReadAll(response.Body) if err != nil { return err } @@ -106,35 +107,38 @@ func extractBody(response *http.Response, object runtime.Object) error { func extractBodyString(response *http.Response) (string, error) { defer response.Body.Close() - body, err := ioutil.ReadAll(response.Body) + body, err := io.ReadAll(response.Body) if err != nil { return "", err } return string(body), err } +func apiGroupVersion(gv schema.GroupVersion, groupInfo genericapiserver.APIGroupInfo) *genericapi.APIGroupVersion { + return &genericapi.APIGroupVersion{ + Root: prefix, + GroupVersion: gv, + MetaGroupVersion: groupInfo.MetaGroupVersion, + ParameterCodec: groupInfo.ParameterCodec, + Serializer: groupInfo.NegotiatedSerializer, + Creater: groupInfo.Scheme, + Convertor: groupInfo.Scheme, + UnsafeConvertor: runtime.UnsafeObjectConvertor(groupInfo.Scheme), + Typer: groupInfo.Scheme, + Namer: runtime.Namer(meta.NewAccessor()), + } +} + func handleCustomMetrics(prov provider.CustomMetricsProvider) http.Handler { container := restful.NewContainer() container.Router(restful.CurlyRouter{}) mux := container.ServeMux resourceStorage := custommetricstorage.NewREST(prov) group := &MetricsAPIGroupVersion{ - DynamicStorage: resourceStorage, - APIGroupVersion: &genericapi.APIGroupVersion{ - Root: prefix, - GroupVersion: customMetricsGroupVersion, - MetaGroupVersion: customMetricsGroupInfo.MetaGroupVersion, - - ParameterCodec: customMetricsGroupInfo.ParameterCodec, - Serializer: customMetricsGroupInfo.NegotiatedSerializer, - Creater: customMetricsGroupInfo.Scheme, - Convertor: customMetricsGroupInfo.Scheme, - UnsafeConvertor: runtime.UnsafeObjectConvertor(customMetricsGroupInfo.Scheme), - Typer: customMetricsGroupInfo.Scheme, - Namer: runtime.Namer(meta.NewAccessor()), - }, - ResourceLister: provider.NewCustomMetricResourceLister(prov), - Handlers: &CMHandlers{}, + DynamicStorage: resourceStorage, + APIGroupVersion: apiGroupVersion(customMetricsGroupVersion, customMetricsGroupInfo), + ResourceLister: provider.NewCustomMetricResourceLister(prov), + Handlers: &CMHandlers{}, } if err := group.InstallREST(container); err != nil { @@ -154,22 +158,10 @@ func handleExternalMetrics(prov provider.ExternalMetricsProvider) http.Handler { resourceStorage := externalmetricstorage.NewREST(prov) group := &MetricsAPIGroupVersion{ - DynamicStorage: resourceStorage, - APIGroupVersion: &genericapi.APIGroupVersion{ - Root: prefix, - GroupVersion: externalMetricsGroupVersion, - MetaGroupVersion: externalMetricsGroupInfo.MetaGroupVersion, - - ParameterCodec: externalMetricsGroupInfo.ParameterCodec, - Serializer: externalMetricsGroupInfo.NegotiatedSerializer, - Creater: externalMetricsGroupInfo.Scheme, - Convertor: externalMetricsGroupInfo.Scheme, - UnsafeConvertor: runtime.UnsafeObjectConvertor(externalMetricsGroupInfo.Scheme), - Typer: externalMetricsGroupInfo.Scheme, - Namer: runtime.Namer(meta.NewAccessor()), - }, - ResourceLister: provider.NewExternalMetricResourceLister(prov), - Handlers: &EMHandlers{}, + DynamicStorage: resourceStorage, + APIGroupVersion: apiGroupVersion(externalMetricsGroupVersion, externalMetricsGroupInfo), + ResourceLister: provider.NewExternalMetricResourceLister(prov), + Handlers: &EMHandlers{}, } if err := group.InstallREST(container); err != nil { @@ -192,29 +184,28 @@ type fakeCMProvider struct { func (p *fakeCMProvider) valuesFor(name types.NamespacedName, info provider.CustomMetricInfo, metricSelector labels.Selector) (string, []custom_metrics.MetricValue, bool) { if info.Namespaced { - metricId := name.Namespace + "/" + info.GroupResource.String() + "/" + name.Name + "/" + info.Metric - values, ok := p.namespacedValues[metricId] - return metricId, values, ok - } else { - metricId := info.GroupResource.String() + "/" + name.Name + "/" + info.Metric - values, ok := p.rootValues[metricId] - return metricId, values, ok + metricID := name.Namespace + "/" + info.GroupResource.String() + "/" + name.Name + "/" + info.Metric + values, ok := p.namespacedValues[metricID] + return metricID, values, ok } + metricID := info.GroupResource.String() + "/" + name.Name + "/" + info.Metric + values, ok := p.rootValues[metricID] + return metricID, values, ok } func (p *fakeCMProvider) GetMetricByName(ctx context.Context, name types.NamespacedName, info provider.CustomMetricInfo, metricSelector labels.Selector) (*custom_metrics.MetricValue, error) { - metricId, values, ok := p.valuesFor(name, info, metricSelector) + metricID, values, ok := p.valuesFor(name, info, metricSelector) if !ok { - return nil, fmt.Errorf("non-existent metric requested (id: %s)", metricId) + return nil, fmt.Errorf("non-existent metric requested (id: %s)", metricID) } return &values[0], nil } func (p *fakeCMProvider) GetMetricBySelector(ctx context.Context, namespace string, selector labels.Selector, info provider.CustomMetricInfo, metricSelector labels.Selector) (*custom_metrics.MetricValueList, error) { - metricId, values, ok := p.valuesFor(types.NamespacedName{Namespace: namespace, Name: "*"}, info, metricSelector) + metricID, values, ok := p.valuesFor(types.NamespacedName{Namespace: namespace, Name: "*"}, info, metricSelector) if !ok { - return nil, fmt.Errorf("non-existent metric requested (id: %s)", metricId) + return nil, fmt.Errorf("non-existent metric requested (id: %s)", metricID) } var trimmedValues custom_metrics.MetricValueList @@ -226,7 +217,7 @@ func (p *fakeCMProvider) GetMetricBySelector(ctx context.Context, namespace stri subsetCounts = p.rootSubsetCounts } - if trimmedCount, ok := subsetCounts[metricId]; ok { + if trimmedCount, ok := subsetCounts[metricID]; ok { trimmedValues = custom_metrics.MetricValueList{ Items: make([]custom_metrics.MetricValue, 0, trimmedCount), } diff --git a/pkg/apiserver/installer/cmhandlers.go b/pkg/apiserver/installer/cmhandlers.go index f7827461a..0e73da895 100644 --- a/pkg/apiserver/installer/cmhandlers.go +++ b/pkg/apiserver/installer/cmhandlers.go @@ -19,12 +19,13 @@ package installer import ( "net/http" + "github.com/emicklei/go-restful/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/endpoints/handlers" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/metrics" - "github.com/emicklei/go-restful/v3" "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/registry/rest" ) @@ -106,7 +107,7 @@ func (ch *CMHandlers) registerResourceHandlers(a *MetricsAPIInstaller, ws *restf } mediaTypes, streamMediaTypes := negotiation.MediaTypesForSerializer(a.group.Serializer) - allMediaTypes := append(mediaTypes, streamMediaTypes...) + allMediaTypes := append(mediaTypes, streamMediaTypes...) //nolint: gocritic ws.Produces(allMediaTypes...) reqScope := handlers.RequestScope{ diff --git a/pkg/apiserver/installer/conversion.go b/pkg/apiserver/installer/conversion.go index 543c473b7..c015d9888 100644 --- a/pkg/apiserver/installer/conversion.go +++ b/pkg/apiserver/installer/conversion.go @@ -25,7 +25,7 @@ import ( cmv1beta2 "k8s.io/metrics/pkg/apis/custom_metrics/v1beta2" ) -func Convert_url_Values_To_v1beta1_MetricListOptions(in *url.Values, out *cmv1beta1.MetricListOptions, s conversion.Scope) error { +func ConvertURLValuesToV1beta1MetricListOptions(in *url.Values, out *cmv1beta1.MetricListOptions, s conversion.Scope) error { if values, ok := map[string][]string(*in)["labelSelector"]; ok && len(values) > 0 { if err := runtime.Convert_Slice_string_To_string(&values, &out.LabelSelector, s); err != nil { return err @@ -43,7 +43,7 @@ func Convert_url_Values_To_v1beta1_MetricListOptions(in *url.Values, out *cmv1be return nil } -func Convert_url_Values_To_v1beta2_MetricListOptions(in *url.Values, out *cmv1beta2.MetricListOptions, s conversion.Scope) error { +func ConvertURLValuesToV1beta2MetricListOptions(in *url.Values, out *cmv1beta2.MetricListOptions, s conversion.Scope) error { if values, ok := map[string][]string(*in)["labelSelector"]; ok && len(values) > 0 { if err := runtime.Convert_Slice_string_To_string(&values, &out.LabelSelector, s); err != nil { return err @@ -64,12 +64,12 @@ func Convert_url_Values_To_v1beta2_MetricListOptions(in *url.Values, out *cmv1be // RegisterConversions adds conversion functions to the given scheme. func RegisterConversions(s *runtime.Scheme) error { if err := s.AddConversionFunc((*url.Values)(nil), (*cmv1beta1.MetricListOptions)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_url_Values_To_v1beta1_MetricListOptions(a.(*url.Values), b.(*cmv1beta1.MetricListOptions), scope) + return ConvertURLValuesToV1beta1MetricListOptions(a.(*url.Values), b.(*cmv1beta1.MetricListOptions), scope) }); err != nil { return err } if err := s.AddConversionFunc((*url.Values)(nil), (*cmv1beta2.MetricListOptions)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_url_Values_To_v1beta2_MetricListOptions(a.(*url.Values), b.(*cmv1beta2.MetricListOptions), scope) + return ConvertURLValuesToV1beta2MetricListOptions(a.(*url.Values), b.(*cmv1beta2.MetricListOptions), scope) }); err != nil { return err } diff --git a/pkg/apiserver/installer/emhandlers.go b/pkg/apiserver/installer/emhandlers.go index 6a85cabc8..f47be6186 100644 --- a/pkg/apiserver/installer/emhandlers.go +++ b/pkg/apiserver/installer/emhandlers.go @@ -19,13 +19,13 @@ package installer import ( "net/http" + "github.com/emicklei/go-restful/v3" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apiserver/pkg/endpoints/handlers" "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/endpoints/metrics" "k8s.io/apiserver/pkg/registry/rest" - - "github.com/emicklei/go-restful/v3" ) type EMHandlers struct{} @@ -72,7 +72,7 @@ func (ch *EMHandlers) registerResourceHandlers(a *MetricsAPIInstaller, ws *restf externalMetricPath := "namespaces" + "/{namespace}/{resource}" mediaTypes, streamMediaTypes := negotiation.MediaTypesForSerializer(a.group.Serializer) - allMediaTypes := append(mediaTypes, streamMediaTypes...) + allMediaTypes := append(mediaTypes, streamMediaTypes...) //nolint: gocritic ws.Produces(allMediaTypes...) reqScope := handlers.RequestScope{ diff --git a/pkg/apiserver/installer/installer.go b/pkg/apiserver/installer/installer.go index 8bf189922..82ba29009 100644 --- a/pkg/apiserver/installer/installer.go +++ b/pkg/apiserver/installer/installer.go @@ -23,6 +23,8 @@ import ( "strings" "time" + "github.com/emicklei/go-restful/v3" + "k8s.io/apimachinery/pkg/conversion" "k8s.io/apimachinery/pkg/runtime/schema" utilerrors "k8s.io/apimachinery/pkg/util/errors" @@ -32,7 +34,6 @@ import ( "k8s.io/apiserver/pkg/endpoints/handlers/negotiation" "k8s.io/apiserver/pkg/registry/rest" - "github.com/emicklei/go-restful/v3" cm_handlers "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/endpoints/handlers" cm_rest "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/registry/rest" ) @@ -189,8 +190,7 @@ func addObjectParams(ws *restful.WebService, route *restful.RouteBuilder, obj in return err } st := sv.Type() - switch st.Kind() { - case reflect.Struct: + if st.Kind() == reflect.Struct { for i := 0; i < st.NumField(); i++ { name := st.Field(i).Name sf, ok := st.FieldByName(name) diff --git a/pkg/cmd/builder.go b/pkg/cmd/builder.go index 85e8fb2b1..7e33ac298 100644 --- a/pkg/cmd/builder.go +++ b/pkg/cmd/builder.go @@ -22,6 +22,7 @@ import ( "time" "github.com/spf13/pflag" + apimeta "k8s.io/apimachinery/pkg/api/meta" openapinamer "k8s.io/apiserver/pkg/endpoints/openapi" genericapiserver "k8s.io/apiserver/pkg/server" @@ -246,7 +247,7 @@ func (b *AdapterBase) defaultOpenAPIConfig() *openapicommon.Config { return openAPIConfig } -// Config fetches the configuration used to ulitmately create the custom metrics adapter's +// Config fetches the configuration used to ultimately create the custom metrics adapter's // API server. While this method is idempotent, it does "cement" values of some of the other // fields, so make sure to only call it just before `Server` or `Run`. // Normal users should not need to call this method -- it's for advanced use cases. @@ -273,7 +274,7 @@ func (b *AdapterBase) Config() (*apiserver.Config, error) { return b.config, nil } -// Server fetches API server object used to ulitmately run the custom metrics adapter. +// Server fetches API server object used to ultimately run the custom metrics adapter. // While this method is idempotent, it does "cement" values of some of the other // fields, so make sure to only call it just before `Run`. // Normal users should not need to call this method -- it's for advanced use cases. diff --git a/pkg/cmd/builder_test.go b/pkg/cmd/builder_test.go index 458f5dcfc..0eba23476 100644 --- a/pkg/cmd/builder_test.go +++ b/pkg/cmd/builder_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/kube-openapi/pkg/builder" "sigs.k8s.io/custom-metrics-apiserver/pkg/provider/fake" diff --git a/pkg/dynamicmapper/mapper.go b/pkg/dynamicmapper/mapper.go index e6aaded6b..5831974f5 100644 --- a/pkg/dynamicmapper/mapper.go +++ b/pkg/dynamicmapper/mapper.go @@ -75,35 +75,36 @@ func (m *RegeneratingDiscoveryRESTMapper) KindsFor(resource schema.GroupVersionR defer m.mu.RUnlock() return m.delegate.KindsFor(resource) - } + func (m *RegeneratingDiscoveryRESTMapper) ResourceFor(input schema.GroupVersionResource) (schema.GroupVersionResource, error) { m.mu.RLock() defer m.mu.RUnlock() return m.delegate.ResourceFor(input) - } + func (m *RegeneratingDiscoveryRESTMapper) ResourcesFor(input schema.GroupVersionResource) ([]schema.GroupVersionResource, error) { m.mu.RLock() defer m.mu.RUnlock() return m.delegate.ResourcesFor(input) - } + func (m *RegeneratingDiscoveryRESTMapper) RESTMapping(gk schema.GroupKind, versions ...string) (*meta.RESTMapping, error) { m.mu.RLock() defer m.mu.RUnlock() return m.delegate.RESTMapping(gk, versions...) - } + func (m *RegeneratingDiscoveryRESTMapper) RESTMappings(gk schema.GroupKind, versions ...string) ([]*meta.RESTMapping, error) { m.mu.RLock() defer m.mu.RUnlock() return m.delegate.RESTMappings(gk, versions...) } + func (m *RegeneratingDiscoveryRESTMapper) ResourceSingularizer(resource string) (singular string, err error) { m.mu.RLock() defer m.mu.RUnlock() diff --git a/pkg/provider/errors.go b/pkg/provider/errors.go index 821f2addb..855e9b023 100644 --- a/pkg/provider/errors.go +++ b/pkg/provider/errors.go @@ -30,7 +30,7 @@ import ( // NewMetricNotFoundError returns a StatusError indicating the given metric could not be found. // It is similar to NewNotFound, but more specialized func NewMetricNotFoundError(resource schema.GroupResource, metricName string) *apierr.StatusError { - return &apierr.StatusError{metav1.Status{ + return &apierr.StatusError{ErrStatus: metav1.Status{ Status: metav1.StatusFailure, Code: int32(http.StatusNotFound), Reason: metav1.StatusReasonNotFound, @@ -41,7 +41,7 @@ func NewMetricNotFoundError(resource schema.GroupResource, metricName string) *a // NewMetricNotFoundForError returns a StatusError indicating the given metric could not be found for // the given named object. It is similar to NewNotFound, but more specialized func NewMetricNotFoundForError(resource schema.GroupResource, metricName string, resourceName string) *apierr.StatusError { - return &apierr.StatusError{metav1.Status{ + return &apierr.StatusError{ErrStatus: metav1.Status{ Status: metav1.StatusFailure, Code: int32(http.StatusNotFound), Reason: metav1.StatusReasonNotFound, @@ -52,7 +52,7 @@ func NewMetricNotFoundForError(resource schema.GroupResource, metricName string, // NewMetricNotFoundForError returns a StatusError indicating the given metric could not be found for // the given named object. It is similar to NewNotFound, but more specialized func NewMetricNotFoundForSelectorError(resource schema.GroupResource, metricName string, resourceName string, selector labels.Selector) *apierr.StatusError { - return &apierr.StatusError{metav1.Status{ + return &apierr.StatusError{ErrStatus: metav1.Status{ Status: metav1.StatusFailure, Code: int32(http.StatusNotFound), Reason: metav1.StatusReasonNotFound, diff --git a/pkg/provider/fake/fake.go b/pkg/provider/fake/fake.go index f36e4988e..756606343 100644 --- a/pkg/provider/fake/fake.go +++ b/pkg/provider/fake/fake.go @@ -24,6 +24,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/metrics/pkg/apis/custom_metrics" "k8s.io/metrics/pkg/apis/external_metrics" + "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" ) diff --git a/pkg/provider/interfaces.go b/pkg/provider/interfaces.go index 656717167..310eb0c7c 100644 --- a/pkg/provider/interfaces.go +++ b/pkg/provider/interfaces.go @@ -44,9 +44,8 @@ type ExternalMetricInfo struct { func (i CustomMetricInfo) String() string { if i.Namespaced { return fmt.Sprintf("%s/%s(namespaced)", i.GroupResource.String(), i.Metric) - } else { - return fmt.Sprintf("%s/%s", i.GroupResource.String(), i.Metric) } + return fmt.Sprintf("%s/%s", i.GroupResource.String(), i.Metric) } // Normalized returns a copy of the current MetricInfo with the GroupResource resolved using the @@ -93,7 +92,7 @@ type CustomMetricsProvider interface { // ListAllMetrics provides a list of all available metrics at // the current time. Note that this is not allowed to return - // an error, so it is reccomended that implementors cache and + // an error, so it is recommended that implementors cache and // periodically update this list, instead of querying every time. ListAllMetrics() []CustomMetricInfo } diff --git a/pkg/provider/interfaces_test.go b/pkg/provider/interfaces_test.go index d970ab52e..6efafc13c 100644 --- a/pkg/provider/interfaces_test.go +++ b/pkg/provider/interfaces_test.go @@ -21,6 +21,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime/schema" diff --git a/pkg/registry/custom_metrics/reststorage.go b/pkg/registry/custom_metrics/reststorage.go index c8a6eb2ed..21876780d 100644 --- a/pkg/registry/custom_metrics/reststorage.go +++ b/pkg/registry/custom_metrics/reststorage.go @@ -20,9 +20,6 @@ import ( "context" "fmt" - cm_rest "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/registry/rest" - "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" - metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -32,6 +29,9 @@ import ( genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/metrics/pkg/apis/custom_metrics" + + cm_rest "sigs.k8s.io/custom-metrics-apiserver/pkg/apiserver/registry/rest" + "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" ) type REST struct { @@ -121,9 +121,8 @@ func (r *REST) List(ctx context.Context, options *metainternalversion.ListOption // handle namespaced and root metrics if name == "*" { return r.handleWildcardOp(ctx, namespace, groupResource, selector, metricName, metricLabelSelector) - } else { - return r.handleIndividualOp(ctx, namespace, groupResource, name, metricName, metricLabelSelector) } + return r.handleIndividualOp(ctx, namespace, groupResource, name, metricName, metricLabelSelector) } func (r *REST) handleIndividualOp(ctx context.Context, namespace string, groupResource schema.GroupResource, name string, metricName string, metricLabelSelector labels.Selector) (*custom_metrics.MetricValueList, error) { diff --git a/pkg/registry/external_metrics/reststorage.go b/pkg/registry/external_metrics/reststorage.go index 386831868..c31549241 100644 --- a/pkg/registry/external_metrics/reststorage.go +++ b/pkg/registry/external_metrics/reststorage.go @@ -27,6 +27,7 @@ import ( genericapirequest "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/metrics/pkg/apis/external_metrics" + "sigs.k8s.io/custom-metrics-apiserver/pkg/provider" ) diff --git a/test-adapter/main.go b/test-adapter/main.go index fa76834bd..671eed058 100644 --- a/test-adapter/main.go +++ b/test-adapter/main.go @@ -20,6 +20,7 @@ import ( "flag" "net/http" "os" + "time" "github.com/emicklei/go-restful/v3" "k8s.io/apimachinery/pkg/util/wait" @@ -34,7 +35,7 @@ import ( type SampleAdapter struct { basecmd.AdapterBase - // Message is printed on succesful startup + // Message is printed on successful startup Message string } @@ -62,7 +63,9 @@ func main() { cmd.Flags().StringVar(&cmd.Message, "msg", "starting adapter...", "startup message") cmd.Flags().AddGoFlagSet(flag.CommandLine) // make sure we get the klog flags - cmd.Flags().Parse(os.Args) + if err := cmd.Flags().Parse(os.Args); err != nil { + klog.Fatalf("unable to parse flags: %v", err) + } testProvider, webService := cmd.makeProviderOrDie() cmd.WithCustomMetrics(testProvider) @@ -73,7 +76,11 @@ func main() { restful.DefaultContainer.Add(webService) go func() { // Open port for POSTing fake metrics - klog.Fatal(http.ListenAndServe(":8080", nil)) + server := &http.Server{ + Addr: ":8080", + ReadHeaderTimeout: 3 * time.Second, + } + klog.Fatal(server.ListenAndServe()) }() if err := cmd.Run(wait.NeverStop); err != nil { klog.Fatalf("unable to run custom metrics adapter: %v", err) diff --git a/test-adapter/provider/provider.go b/test-adapter/provider/provider.go index d11840c73..1d3cc530b 100644 --- a/test-adapter/provider/provider.go +++ b/test-adapter/provider/provider.go @@ -164,7 +164,9 @@ func (p *testingProvider) updateMetric(request *restful.Request, response *restf value := new(resource.Quantity) err := request.ReadEntity(value) if err != nil { - response.WriteErrorString(http.StatusBadRequest, err.Error()) + if err := response.WriteErrorString(http.StatusBadRequest, err.Error()); err != nil { + klog.Errorf("Error writing error: %s", err) + } return } @@ -175,7 +177,9 @@ func (p *testingProvider) updateMetric(request *restful.Request, response *restf if len(sel) > 0 { metricLabels, err = labels.ConvertSelectorToLabelsMap(sel) if err != nil { - response.WriteErrorString(http.StatusBadRequest, err.Error()) + if err := response.WriteErrorString(http.StatusBadRequest, err.Error()); err != nil { + klog.Errorf("Error writing error: %s", err) + } return } } @@ -240,7 +244,7 @@ func (p *testingProvider) metricFor(value resource.Quantity, name types.Namespac Metric: custom_metrics.MetricIdentifier{ Name: info.Metric, }, - Timestamp: metav1.Time{time.Now()}, + Timestamp: metav1.Time{Time: time.Now()}, Value: value, }