From 6facdac464affddbab65f3dcd18177025442c1b9 Mon Sep 17 00:00:00 2001 From: Harsh Thakur Date: Tue, 15 Jun 2021 09:33:42 +0000 Subject: [PATCH] Fix golang lint errors (#2068) * Fix golang lint errors Signed-off-by: Harsh Thakur * Typo for fields Signed-off-by: Harsh Thakur * Multi error fix Signed-off-by: Harsh Thakur * Multi error fix Signed-off-by: Harsh Thakur * Add nolint to logtostderr errcheck Signed-off-by: Harsh Thakur --- cmd/builder/main.go | 7 ++- cmd/fetcher/app/server.go | 6 ++- cmd/fetcher/main.go | 8 +++- cmd/fission-bundle/main.go | 14 ++++-- cmd/preupgradechecks/main.go | 7 ++- cmd/reporter/app/cmd_event.go | 13 +++++- cmd/reporter/main.go | 7 ++- crds/v1/fission.io_httptriggers.yaml | 2 +- demos/declarative-specs/hello.go | 9 ++-- pkg/apis/core/v1/types.go | 2 +- pkg/apis/core/v1/validation.go | 3 +- pkg/crd/crd.go | 4 +- pkg/executor/util/merge.go | 6 +-- pkg/mqtrigger/scalermanager.go | 8 ---- pkg/mqtrigger/scalermanager_test.go | 53 ---------------------- pkg/poolcache/poolcache_test.go | 2 +- pkg/router/functionHandler.go | 9 ++-- test/tests/mqtrigger/nats/stan-sub/main.go | 5 +- test/tests/test_huge_response/hello.go | 8 +++- 19 files changed, 82 insertions(+), 91 deletions(-) diff --git a/cmd/builder/main.go b/cmd/builder/main.go index 865de011..765b16b2 100644 --- a/cmd/builder/main.go +++ b/cmd/builder/main.go @@ -34,7 +34,12 @@ func main() { if err != nil { log.Fatalf("can't initialize zap logger: %v", err) } - defer logger.Sync() + defer func() { + err := logger.Sync() + if err != nil { + log.Fatal(err) + } + }() shareVolume := os.Args[1] if _, err := os.Stat(shareVolume); err != nil { diff --git a/cmd/fetcher/app/server.go b/cmd/fetcher/app/server.go index bb1e3c68..467b58aa 100644 --- a/cmd/fetcher/app/server.go +++ b/cmd/fetcher/app/server.go @@ -21,6 +21,7 @@ import ( "encoding/json" "flag" "fmt" + "log" "net/http" "os" @@ -136,9 +137,12 @@ func Run(logger *zap.Logger) { mux.HandleFunc("/readniess-healthz", readinessHandler) logger.Info("fetcher ready to receive requests") - http.ListenAndServe(":8000", &ochttp.Handler{ + err = http.ListenAndServe(":8000", &ochttp.Handler{ Handler: mux, }) + if err != nil { + log.Fatal(err) + } } func fetcherUsage() { diff --git a/cmd/fetcher/main.go b/cmd/fetcher/main.go index 6caea1a8..fc9bacfe 100644 --- a/cmd/fetcher/main.go +++ b/cmd/fetcher/main.go @@ -33,7 +33,11 @@ func main() { if err != nil { log.Fatalf("can't initialize zap logger: %v", err) } - defer logger.Sync() - + defer func() { + err := logger.Sync() + if err != nil { + log.Fatal(err) + } + }() app.Run(logger) } diff --git a/cmd/fission-bundle/main.go b/cmd/fission-bundle/main.go index 69af08d0..fb01c016 100644 --- a/cmd/fission-bundle/main.go +++ b/cmd/fission-bundle/main.go @@ -176,12 +176,15 @@ func registerTraceExporter(logger *zap.Logger, arguments map[string]interface{}) } func main() { + var err error + // From https://github.com/containous/traefik/pull/1817/files // Tell glog to log into STDERR. Otherwise, we risk // certain kinds of API errors getting logged into a directory not // available in a `FROM scratch` Docker container, causing glog to abort // hard with an exit code > 0. - flag.Set("logtostderr", "true") + // TODO: fix the lint error. Error checking here is causing all components to crash with error "logtostderr not found" + flag.Set("logtostderr", "true") //nolint: errcheck usage := `fission-bundle: Package of all fission microservices: controller, router, executor. @@ -236,7 +239,6 @@ Options: ` var logger *zap.Logger - var err error var config zap.Config isDebugEnv, _ := strconv.ParseBool(os.Getenv("DEBUG_ENV")) @@ -253,8 +255,12 @@ Options: if err != nil { log.Fatalf("I can't initialize zap logger: %v", err) } - defer logger.Sync() - + defer func() { + err := logger.Sync() + if err != nil { + log.Fatal(err) + } + }() version := fmt.Sprintf("Fission Bundle Version: %v", info.BuildInfo().String()) arguments, err := docopt.ParseArgs(usage, nil, version) if err != nil { diff --git a/cmd/preupgradechecks/main.go b/cmd/preupgradechecks/main.go index 4471fd3e..b2ee2d22 100644 --- a/cmd/preupgradechecks/main.go +++ b/cmd/preupgradechecks/main.go @@ -42,7 +42,12 @@ func main() { if err != nil { log.Fatalf("can't initialize zap logger: %v", err) } - defer logger.Sync() + defer func() { + err := logger.Sync() + if err != nil { + log.Fatal(err) + } + }() usage := `Package to perform operations needed prior to fission installation Usage: diff --git a/cmd/reporter/app/cmd_event.go b/cmd/reporter/app/cmd_event.go index d3c9831d..74477a72 100644 --- a/cmd/reporter/app/cmd_event.go +++ b/cmd/reporter/app/cmd_event.go @@ -16,6 +16,8 @@ limitations under the License. package app import ( + "log" + "github.com/fission/fission/pkg/tracker" "github.com/spf13/cobra" ) @@ -49,6 +51,7 @@ func eventCommandHandler(cmd *cobra.Command, args []string) error { return tracker.Tracker.SendEvent(event) } +//EventCommand reports an event to analytics func EventCommand() *cobra.Command { eventCmd := &cobra.Command{ Use: "event", @@ -61,7 +64,13 @@ func EventCommand() *cobra.Command { persistentFlags.StringP("action", "a", "", "event action") persistentFlags.StringP("label", "l", "", "event label") persistentFlags.StringP("value", "v", "", "event value") - eventCmd.MarkPersistentFlagRequired("category") - eventCmd.MarkPersistentFlagRequired("action") + err := eventCmd.MarkPersistentFlagRequired("category") + if err != nil { + log.Fatal(err) + } + err = eventCmd.MarkPersistentFlagRequired("action") + if err != nil { + log.Fatal(err) + } return eventCmd } diff --git a/cmd/reporter/main.go b/cmd/reporter/main.go index f4e4f1bf..7d157fc6 100644 --- a/cmd/reporter/main.go +++ b/cmd/reporter/main.go @@ -17,9 +17,14 @@ limitations under the License. package main import ( + "log" + "github.com/fission/fission/cmd/reporter/app" ) func main() { - app.App().Execute() + err := app.App().Execute() + if err != nil { + log.Fatal(err) + } } diff --git a/crds/v1/fission.io_httptriggers.yaml b/crds/v1/fission.io_httptriggers.yaml index 69d3b59b..7b51c986 100644 --- a/crds/v1/fission.io_httptriggers.yaml +++ b/crds/v1/fission.io_httptriggers.yaml @@ -77,7 +77,7 @@ spec: type: string type: object method: - description: 'Deprecated: Use Methods instead of Method. HTTP method to access a function.' + description: Use Methods instead of Method. This field is going to be deprecated in a future release HTTP method to access a function. type: string methods: description: HTTP methods to access a function diff --git a/demos/declarative-specs/hello.go b/demos/declarative-specs/hello.go index 03c28d29..92f3ba18 100644 --- a/demos/declarative-specs/hello.go +++ b/demos/declarative-specs/hello.go @@ -1,12 +1,15 @@ package main import ( + "log" "net/http" ) // Handler is the entry point for this fission function - -func Handler(w http.ResponseWriter, r *http.Request) { +func Handler(w http.ResponseWriter, r *http.Request) { //nolint: deadcode msg := "Hello, CNCF Webinar!\n" - w.Write([]byte(msg)) + _, err := w.Write([]byte(msg)) + if err != nil { + log.Fatal(err) + } } diff --git a/pkg/apis/core/v1/types.go b/pkg/apis/core/v1/types.go index a02530a9..bab225fb 100644 --- a/pkg/apis/core/v1/types.go +++ b/pkg/apis/core/v1/types.go @@ -658,7 +658,7 @@ type ( // +optional Prefix *string `json:"prefix,omitempty"` - // Deprecated: Use Methods instead of Method. + // Use Methods instead of Method. This field is going to be deprecated in a future release // HTTP method to access a function. // +optional Method string `json:"method"` diff --git a/pkg/apis/core/v1/validation.go b/pkg/apis/core/v1/validation.go index 0ca49f32..ec051a38 100644 --- a/pkg/apis/core/v1/validation.go +++ b/pkg/apis/core/v1/validation.go @@ -17,6 +17,7 @@ limitations under the License. package v1 import ( + "errors" "fmt" "net/http" "regexp" @@ -585,7 +586,7 @@ func (e *Environment) Validate() error { if e.Spec.Runtime.PodSpec != nil { for _, container := range e.Spec.Runtime.PodSpec.Containers { if container.Command == nil && container.Image == e.Spec.Runtime.Image && container.Name != e.ObjectMeta.Name { - multierror.Append(result, fmt.Errorf("container with image same as runtime image in podspec, must have name same as environment name")) + result = multierror.Append(result, errors.New("container with image same as runtime image in podspec, must have name same as environment name")) } } } diff --git a/pkg/crd/crd.go b/pkg/crd/crd.go index 172e93a7..729e889d 100644 --- a/pkg/crd/crd.go +++ b/pkg/crd/crd.go @@ -42,10 +42,10 @@ func EnsureFissionCRDs(logger *zap.Logger, clientset *apiextensionsclient.Client for _, crdName := range crdsExpected { crd, err := clientset.ApiextensionsV1().CustomResourceDefinitions().Get(context.TODO(), crdName, metav1.GetOptions{}) if err != nil { - multierror.Append(errs, fmt.Errorf("CRD %s not found: %s", crdName, err)) + errs = multierror.Append(errs, fmt.Errorf("CRD %s not found: %s", crdName, err)) } if crd == nil { - multierror.Append(errs, fmt.Errorf("CRD %s not found", crdName)) + errs = multierror.Append(errs, fmt.Errorf("CRD %s not found", crdName)) } } return errs.ErrorOrNil() diff --git a/pkg/executor/util/merge.go b/pkg/executor/util/merge.go index e8c7355f..bd5c2dad 100644 --- a/pkg/executor/util/merge.go +++ b/pkg/executor/util/merge.go @@ -154,15 +154,15 @@ func MergePodSpec(srcPodSpec *apiv1.PodSpec, targetPodSpec *apiv1.PodSpec) (*api srcPodSpec.AutomountServiceAccountToken = targetPodSpec.AutomountServiceAccountToken } - if targetPodSpec.HostNetwork != false { + if targetPodSpec.HostNetwork { srcPodSpec.HostNetwork = targetPodSpec.HostNetwork } - if targetPodSpec.HostPID != false { + if targetPodSpec.HostPID { srcPodSpec.HostPID = targetPodSpec.HostPID } - if targetPodSpec.HostIPC != false { + if targetPodSpec.HostIPC { srcPodSpec.HostIPC = targetPodSpec.HostIPC } diff --git a/pkg/mqtrigger/scalermanager.go b/pkg/mqtrigger/scalermanager.go index 1ba8a8d0..765a7a6c 100644 --- a/pkg/mqtrigger/scalermanager.go +++ b/pkg/mqtrigger/scalermanager.go @@ -296,14 +296,6 @@ func checkAndUpdateTriggerFields(mqt, newMqt *fv1.MessageQueueTrigger) bool { return updated } -func getResourceVersion(scaledObjectName string, kedaClient dynamic.ResourceInterface) (version string, err error) { - scaledObject, err := kedaClient.Get(context.TODO(), scaledObjectName, metav1.GetOptions{}) - if err != nil { - return "", err - } - return scaledObject.GetResourceVersion(), nil -} - func getAuthTriggerSpec(mqt *fv1.MessageQueueTrigger, authenticationRef string, kubeClient kubernetes.Interface) (*unstructured.Unstructured, error) { secret, err := kubeClient.CoreV1().Secrets(apiv1.NamespaceDefault).Get(context.TODO(), mqt.Spec.Secret, metav1.GetOptions{}) if err != nil { diff --git a/pkg/mqtrigger/scalermanager_test.go b/pkg/mqtrigger/scalermanager_test.go index 720697d8..ef06f597 100644 --- a/pkg/mqtrigger/scalermanager_test.go +++ b/pkg/mqtrigger/scalermanager_test.go @@ -369,59 +369,6 @@ func Test_checkAndUpdateTriggerFields(t *testing.T) { } } -func newUnstructured(apiVersion, kind, namespace, name, resourceVersion string) *unstructured.Unstructured { - return &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": apiVersion, - "kind": kind, - "metadata": map[string]interface{}{ - "namespace": namespace, - "name": name, - "resourceVersion": resourceVersion, - }, - }, - } -} - -// commented because this fails with k8s.io/client-go v0.17.2 -// func Test_getResourceVersion(t *testing.T) { -// scheme := runtime.NewScheme() -// fakeDynamicClient := &dynfake.FakeDynamicClient -// client := dynfake.NewSimpleDynamicClient(scheme, newUnstructured(apiVersion, "ScaledObject", "default", "test-1", "12345")) -// dynamicResourceClient := client.Resource(schema.GroupVersionResource{ -// Group: Group, -// Version: Version, -// Resource: "scaledobjects", -// }) -// fmt.Println(dynamicResourceClient.List(metav1.ListOptions{})) -// fmt.Println(dynamicResourceClient.Get("test-1", metav1.GetOptions{})) -// type args struct { -// scaledObjectName string -// kedaClient dynamic.ResourceInterface -// } -// tests := []struct { -// name string -// args args -// wantVersion string -// wantErr bool -// }{ -// {"Valid Resource", args{"test-1", dynamicResourceClient}, "12345", false}, -// {"Invalid Resource", args{"test-2", dynamicResourceClient}, "", true}, -// } -// for _, tt := range tests { -// t.Run(tt.name, func(t *testing.T) { -// gotVersion, err := getResourceVersion(tt.args.scaledObjectName, tt.args.kedaClient) -// if (err != nil) != tt.wantErr { -// t.Errorf("getResourceVersion() error = %v, wantErr %v", err, tt.wantErr) -// return -// } -// if gotVersion != tt.wantVersion { -// t.Errorf("getResourceVersion() = %v, want %v", gotVersion, tt.wantVersion) -// } -// }) -// } -// } - func Test_getAuthTriggerSpec(t *testing.T) { // Valid - with Secret diff --git a/pkg/poolcache/poolcache_test.go b/pkg/poolcache/poolcache_test.go index ccb01f31..741d3a7a 100644 --- a/pkg/poolcache/poolcache_test.go +++ b/pkg/poolcache/poolcache_test.go @@ -39,7 +39,7 @@ func TestPoolCache(t *testing.T) { checkErr(c.DeleteValue("func", "ip")) - _, active, err = c.GetValue("func", 5) + _, _, err = c.GetValue("func", 5) if err == nil { log.Panicf("found deleted element") } diff --git a/pkg/router/functionHandler.go b/pkg/router/functionHandler.go index 67b50a1a..2f939a7a 100644 --- a/pkg/router/functionHandler.go +++ b/pkg/router/functionHandler.go @@ -169,7 +169,10 @@ func (roundTripper *RetryingRoundTripper) RoundTrip(req *http.Request) (*http.Re // close req body defer func() { if req.Body != nil { - req.Body.(*fakeCloseReadCloser).RealClose() + err := req.Body.(*fakeCloseReadCloser).RealClose() + if err != nil { + roundTripper.logger.Error("Error closing body", zap.Error(err)) + } } }() @@ -670,10 +673,10 @@ func (fh functionHandler) getProxyErrorHandler(start time.Time, rrt *RetryingRou fh.logger.Debug(msg, zap.Any("function", fh.function), zap.Any("request_header", req.Header)) case context.DeadlineExceeded: status = http.StatusGatewayTimeout - msg = "function not responses before the timeout" + msg := "function not responses before the timeout" fh.logger.Error(msg, zap.Any("function", fh.function), zap.Any("request_header", req.Header)) default: - code, msg := ferror.GetHTTPError(err) + code, _ := ferror.GetHTTPError(err) status = code msg = "error sending request to function" fh.logger.Error(msg, zap.Error(err), zap.Any("function", fh.function), zap.Any("request_header", req.Header), zap.Any("code", code)) diff --git a/test/tests/mqtrigger/nats/stan-sub/main.go b/test/tests/mqtrigger/nats/stan-sub/main.go index 9f698ba0..40e65d57 100644 --- a/test/tests/mqtrigger/nats/stan-sub/main.go +++ b/test/tests/mqtrigger/nats/stan-sub/main.go @@ -175,7 +175,10 @@ func main() { fmt.Printf("\nReceived signal %s, unsubscribing and closing connection...\n\n", sig.String()) // Do not unsubscribe a durable on exit, except if asked to. if durable == "" || unsubscribe { - sub.Unsubscribe() + err := sub.Unsubscribe() + if err != nil { + log.Fatal(err) + } } sc.Close() cleanupDone <- true diff --git a/test/tests/test_huge_response/hello.go b/test/tests/test_huge_response/hello.go index 7e7aeb9f..d35d8021 100644 --- a/test/tests/test_huge_response/hello.go +++ b/test/tests/test_huge_response/hello.go @@ -2,15 +2,19 @@ package main import ( "io/ioutil" + "log" "net/http" ) // Handler is the entry point for this fission function -func Handler(w http.ResponseWriter, r *http.Request) { +func Handler(w http.ResponseWriter, r *http.Request) { //nolint: deadcode bytes, err := ioutil.ReadAll(r.Body) if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - w.Write(bytes) + _, err = w.Write(bytes) + if err != nil { + log.Fatal(err) + } }