Enable golangci-linter and fix issues

It's a maintained, non-archived version of gometalinter that's also
faster.

Author: DirectXMan12

.golangci.yml

@@ -0,0 +1,5 @@
+linters-settings:
+  lll:
+    line-length: 170
+  dupl:
+    threshold: 400

.travis.yml

@@ -15,7 +15,7 @@ go_import_path: sigs.k8s.io/controller-runtime
 install:
 - go get -u github.com/golang/dep/cmd/dep
 #- go get -u golang.org/x/lint/golint
-- go get -u gopkg.in/alecthomas/gometalinter.v2 && gometalinter.v2 --install
+- curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin v1.15.0
 
 script:
 - MODULES_OPT=vendor GO111MODULE=on TRACE=1 ./hack/check-everything.sh

hack/check-everything.sh

@@ -68,9 +68,9 @@ function is_installed {
 
 function fetch_go_tools {
   header_text "Checking for gometalinter.v2"
-  if ! is_installed gometalinter.v2; then
-    header_text "Installing gometalinter.v2"
-    go get -u gopkg.in/alecthomas/gometalinter.v2 && gometalinter.v2 --install
+  if ! is_installed golangci-lint; then
+    header_text "Installing golangci-lint"
+    GO111MODULE=on go get github.com/golangci/golangci-lint/cmd/[email protected]
   fi
 
   header_text "Checking for dep"

hack/verify.sh

@@ -22,14 +22,9 @@ header_text "running go vet"
 
 go vet ${MOD_OPT} ./...
 
-# go get is broken for golint.  re-enable this once it is fixed.
-#header_text "running golint"
-#
-#golint -set_exit_status ./pkg/...
-
-header_text "running gometalinter.v2"
+header_text "running golangci-lint"
 
-gometalinter.v2 --disable-all \
+golangci-lint run --disable-all \
     --deadline 5m \
     --enable=misspell \
     --enable=structcheck \
@@ -44,13 +39,11 @@ gometalinter.v2 --disable-all \
     --enable=interfacer \
     --enable=misspell \
     --enable=gocyclo \
-    --line-length=170 \
     --enable=lll \
-    --dupl-threshold=400 \
     --enable=dupl \
-    --skip=atomic \
     --enable=goimports \
     ./pkg/... ./examples/... .
+
 # TODO: Enable these as we fix them to make them pass
 #    --enable=gosec \
 #    --enable=maligned \

pkg/builder/builder_suite_test.go

@@ -66,7 +66,7 @@ var _ = BeforeSuite(func(done Done) {
 }, 60)
 
 var _ = AfterSuite(func() {
-	testenv.Stop()
+	Expect(testenv.Stop()).To(Succeed())
 
 	// Put the DefaultBindAddress back
 	metrics.DefaultBindAddress = ":8080"

pkg/cache/cache_suite_test.go

@@ -53,5 +53,5 @@ var _ = BeforeSuite(func(done Done) {
 }, 60)
 
 var _ = AfterSuite(func() {
-	testenv.Stop()
+	Expect(testenv.Stop()).To(Succeed())
 })

pkg/cache/cache_test.go

@@ -401,7 +401,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca
 						Kind:    "Pod",
 					})
 					uKnownPod2 := &unstructured.Unstructured{}
-					kscheme.Scheme.Convert(knownPod2, uKnownPod2, nil)
+					Expect(kscheme.Scheme.Convert(knownPod2, uKnownPod2, nil)).To(Succeed())
 
 					podKey := client.ObjectKey{Name: "test-pod-2", Namespace: testNamespaceTwo}
 					Expect(informerCache.Get(context.Background(), podKey, out)).To(Succeed())

pkg/client/client_suite_test.go

@@ -54,5 +54,5 @@ var _ = BeforeSuite(func(done Done) {
 }, 60)
 
 var _ = AfterSuite(func() {
-	testenv.Stop()
+	Expect(testenv.Stop()).To(Succeed())
 })

pkg/client/client_test.go

@@ -297,7 +297,7 @@ var _ = Describe("Client", func() {
 
 				By("encoding the deployment as unstructured")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -321,7 +321,7 @@ var _ = Describe("Client", func() {
 
 				By("encoding the deployment as unstructured")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Kind:    "Node",
@@ -336,8 +336,8 @@ var _ = Describe("Client", func() {
 				Expect(err).NotTo(HaveOccurred())
 				Expect(actual).NotTo(BeNil())
 				au := &unstructured.Unstructured{}
-				scheme.Convert(actual, au, nil)
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(actual, au, nil)).To(Succeed())
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				By("writing the result back to the go struct")
 
 				Expect(u).To(Equal(au))
@@ -361,7 +361,7 @@ var _ = Describe("Client", func() {
 
 				By("encoding the deployment as unstructured")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(old, u, nil)
+				Expect(scheme.Convert(old, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -383,7 +383,7 @@ var _ = Describe("Client", func() {
 
 				By("creating the pod, since required field Containers is empty")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(pod, u, nil)
+				Expect(scheme.Convert(pod, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Version: "v1",
@@ -407,7 +407,7 @@ var _ = Describe("Client", func() {
 
 				By("encoding the deployment as unstructured")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -531,7 +531,7 @@ var _ = Describe("Client", func() {
 
 				By("updating the Deployment")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -560,7 +560,7 @@ var _ = Describe("Client", func() {
 
 				By("updating the object")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Kind:    "Node",
@@ -585,7 +585,7 @@ var _ = Describe("Client", func() {
 
 				By("updating non-existent object")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -726,7 +726,7 @@ var _ = Describe("Client", func() {
 				By("updating the status of Deployment")
 				u := &unstructured.Unstructured{}
 				dep.Status.Replicas = 1
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				err = cl.Status().Update(context.TODO(), u)
 				Expect(err).NotTo(HaveOccurred())
 
@@ -753,7 +753,7 @@ var _ = Describe("Client", func() {
 				var rc int32 = 1
 				dep.Status.Replicas = 1
 				dep.Spec.Replicas = &rc
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				err = cl.Status().Update(context.TODO(), u)
 				Expect(err).NotTo(HaveOccurred())
 
@@ -778,7 +778,7 @@ var _ = Describe("Client", func() {
 				By("updating status of the object")
 				u := &unstructured.Unstructured{}
 				node.Status.Phase = corev1.NodeRunning
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				err = cl.Status().Update(context.TODO(), u)
 				Expect(err).NotTo(HaveOccurred())
 
@@ -798,7 +798,7 @@ var _ = Describe("Client", func() {
 
 				By("updating status of a non-existent object")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				err = cl.Status().Update(context.TODO(), u)
 				Expect(err).To(HaveOccurred())
 
@@ -912,7 +912,7 @@ var _ = Describe("Client", func() {
 				By("deleting the Deployment")
 				depName := dep.Name
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -940,7 +940,7 @@ var _ = Describe("Client", func() {
 				By("deleting the Node")
 				nodeName := node.Name
 				u := &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Kind:    "Node",
@@ -963,7 +963,7 @@ var _ = Describe("Client", func() {
 
 				By("Deleting node before it is ever created")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Kind:    "Node",
@@ -1097,7 +1097,7 @@ var _ = Describe("Client", func() {
 				By("patching the Deployment")
 				depName := dep.Name
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -1127,7 +1127,7 @@ var _ = Describe("Client", func() {
 				By("patching the Node")
 				nodeName := node.Name
 				u := &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Kind:    "Node",
@@ -1152,7 +1152,7 @@ var _ = Describe("Client", func() {
 
 				By("Patching node before it is ever created")
 				u := &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "",
 					Kind:    "Node",
@@ -1177,7 +1177,7 @@ var _ = Describe("Client", func() {
 				By("patching the Deployment")
 				depName := dep.Name
 				u := &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 				u.SetGroupVersionKind(schema.GroupVersionKind{
 					Group:   "apps",
 					Kind:    "Deployment",
@@ -1294,7 +1294,7 @@ var _ = Describe("Client", func() {
 
 				By("encoding the Deployment as unstructured")
 				var u runtime.Unstructured = &unstructured.Unstructured{}
-				scheme.Convert(dep, u, nil)
+				Expect(scheme.Convert(dep, u, nil)).To(Succeed())
 
 				By("fetching the created Deployment")
 				var actual unstructured.Unstructured
@@ -1321,7 +1321,7 @@ var _ = Describe("Client", func() {
 
 				By("encoding the Node as unstructured")
 				var u runtime.Unstructured = &unstructured.Unstructured{}
-				scheme.Convert(node, u, nil)
+				Expect(scheme.Convert(node, u, nil)).To(Succeed())
 
 				cl, err := client.New(cfg, client.Options{})
 				Expect(err).NotTo(HaveOccurred())
@@ -2166,7 +2166,7 @@ var _ = Describe("DelegatingReader", func() {
 			}
 			var actual appsv1.Deployment
 			key := client.ObjectKey{Namespace: "ns", Name: "name"}
-			dReader.Get(context.TODO(), key, &actual)
+			Expect(dReader.Get(context.TODO(), key, &actual)).To(Succeed())
 			Expect(1).To(Equal(cachedReader.Called))
 			Expect(0).To(Equal(clientReader.Called))
 		})
@@ -2179,7 +2179,7 @@ var _ = Describe("DelegatingReader", func() {
 			}
 			var actual unstructured.Unstructured
 			key := client.ObjectKey{Namespace: "ns", Name: "name"}
-			dReader.Get(context.TODO(), key, &actual)
+			Expect(dReader.Get(context.TODO(), key, &actual)).To(Succeed())
 			Expect(0).To(Equal(cachedReader.Called))
 			Expect(1).To(Equal(clientReader.Called))
 		})
@@ -2193,7 +2193,7 @@ var _ = Describe("DelegatingReader", func() {
 				ClientReader: clientReader,
 			}
 			var actual appsv1.DeploymentList
-			dReader.List(context.Background(), &actual)
+			Expect(dReader.List(context.Background(), &actual)).To(Succeed())
 			Expect(1).To(Equal(cachedReader.Called))
 			Expect(0).To(Equal(clientReader.Called))
 
@@ -2207,7 +2207,7 @@ var _ = Describe("DelegatingReader", func() {
 			}
 
 			var actual unstructured.UnstructuredList
-			dReader.List(context.Background(), &actual)
+			Expect(dReader.List(context.Background(), &actual)).To(Succeed())
 			Expect(0).To(Equal(cachedReader.Called))
 			Expect(1).To(Equal(clientReader.Called))
 

pkg/client/fake/client_test.go

@@ -244,8 +244,8 @@ var _ = Describe("Fake client", func() {
 	Context("with given scheme", func() {
 		BeforeEach(func(done Done) {
 			scheme := runtime.NewScheme()
-			corev1.AddToScheme(scheme)
-			appsv1.AddToScheme(scheme)
+			Expect(corev1.AddToScheme(scheme)).To(Succeed())
+			Expect(appsv1.AddToScheme(scheme)).To(Succeed())
 			cl = NewFakeClientWithScheme(scheme, dep, dep2, cm)
 			close(done)
 		})

pkg/controller/controller_suite_test.go

@@ -57,7 +57,7 @@ var _ = BeforeSuite(func(done Done) {
 }, 60)
 
 var _ = AfterSuite(func() {
-	testenv.Stop()
+	Expect(testenv.Stop()).To(Succeed())
 
 	// Put the DefaultBindAddress back
 	metrics.DefaultBindAddress = ":8080"

pkg/controller/controllerutil/controllerutil_suite_test.go

@@ -49,5 +49,5 @@ var _ = BeforeSuite(func() {
 })
 
 var _ = AfterSuite(func() {
-	t.Stop()
+	Expect(t.Stop()).To(Succeed())
 })

pkg/controller/controllerutil/controllerutil_test.go

@@ -214,7 +214,7 @@ var _ = Describe("Controllerutil", func() {
 
 		It("errors when MutateFn changes objct name on creation", func() {
 			op, err := controllerutil.CreateOrUpdate(context.TODO(), c, deploy, func() error {
-				specr()
+				Expect(specr()).To(Succeed())
 				return deploymentRenamer(deploy)()
 			})
 

pkg/controller/example_test.go

@@ -77,7 +77,10 @@ func ExampleController() {
 	}
 
 	// Start the Controller through the manager.
-	mgr.Start(signals.SetupSignalHandler())
+	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {
+		log.Error(err, "unable to continue running manager")
+		os.Exit(1)
+	}
 }
 
 // This example starts a new Controller named "pod-controller" to Watch Pods with the unstructured object and call a no-op Reconciler.
@@ -111,5 +114,8 @@ func ExampleController_unstructured() {
 	}
 
 	// Start the Controller through the manager.
-	mgr.Start(signals.SetupSignalHandler())
+	if err := mgr.Start(signals.SetupSignalHandler()); err != nil {
+		log.Error(err, "unable to continue running manager")
+		os.Exit(1)
+	}
 }

pkg/envtest/envtest_test.go

@@ -51,7 +51,7 @@ var _ = Describe("Test", func() {
 	// Cleanup CRDs
 	AfterEach(func(done Done) {
 		for _, crd := range crds {
-			c.Delete(context.TODO(), crd)
+			Expect(c.Delete(context.TODO(), crd)).To(Succeed())
 		}
 		close(done)
 	})