Skip to content

Commit

Permalink
Enable golangci-linter and fix issues
Browse files Browse the repository at this point in the history
It's a maintained, non-archived version of gometalinter that's also
faster.
  • Loading branch information
DirectXMan12 committed May 4, 2019
1 parent 5d33529 commit dd63d2f
Show file tree
Hide file tree
Showing 30 changed files with 178 additions and 149 deletions.
5 changes: 5 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
linters-settings:
lll:
line-length: 170
dupl:
threshold: 400
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
- GO111MODULE=on TRACE=1 ./hack/check-everything.sh
Expand Down
2 changes: 1 addition & 1 deletion hack/check-everything.sh
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function fetch_kb_tools {

header_text "using tools"

which gometalinter.v2
which golangci-lint
which dep
fetch_kb_tools
setup_envs
Expand Down
14 changes: 4 additions & 10 deletions hack/verify.sh
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,9 @@ header_text "running go vet"

go vet ./...

# 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 \
Expand All @@ -44,17 +39,16 @@ 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 \
# --enable=safesql \

header_text "running dep check"
dep check
go mod tidy
2 changes: 1 addition & 1 deletion pkg/builder/builder_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion pkg/cache/cache_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,5 @@ var _ = BeforeSuite(func(done Done) {
}, 60)

var _ = AfterSuite(func() {
testenv.Stop()
Expect(testenv.Stop()).To(Succeed())
})
2 changes: 1 addition & 1 deletion pkg/cache/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion pkg/client/client_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,5 @@ var _ = BeforeSuite(func(done Done) {
}, 60)

var _ = AfterSuite(func() {
testenv.Stop()
Expect(testenv.Stop()).To(Succeed())
})
54 changes: 27 additions & 27 deletions pkg/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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))
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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())

Expand All @@ -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())

Expand All @@ -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())

Expand All @@ -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())

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand Down Expand Up @@ -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
Expand All @@ -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())
Expand Down Expand Up @@ -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))
})
Expand All @@ -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))
})
Expand All @@ -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))

Expand All @@ -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))

Expand Down
4 changes: 2 additions & 2 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/controller_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/controllerutil/controllerutil_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@ var _ = BeforeSuite(func() {
})

var _ = AfterSuite(func() {
t.Stop()
Expect(t.Stop()).To(Succeed())
})
2 changes: 1 addition & 1 deletion pkg/controller/controllerutil/controllerutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)()
})

Expand Down
10 changes: 8 additions & 2 deletions pkg/controller/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion pkg/envtest/envtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
Loading

0 comments on commit dd63d2f

Please sign in to comment.