Merge pull request #225 from nunnatsa/improve-linter

Add the ginkgolinter and fix findings

.github/workflows/test.yaml

@@ -48,9 +48,9 @@ jobs:
     steps:
     - uses: actions/checkout@v2
     - name: golangci-lint
-      uses: golangci/golangci-lint-action@v2
+      uses: golangci/golangci-lint-action@v3
       with: # TODO: remove this when the deprecated function will be removed (issue #85)
-        args: --exclude SA1019 --timeout=5m
+        args: --exclude SA1019 --timeout=5m -E ginkgolinter
 
   check-gen:
     runs-on: ubuntu-latest

Makefile

@@ -333,7 +333,7 @@ goimports:
 linter:
 	go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
 	# todo remove the exclude parameter when issue #85 is resolved
-	golangci-lint run --exclude SA1019
+	golangci-lint run --timeout=5m -E ginkgolinter --exclude SA1019
 
 .PHONY: sanity
 sanity: linter goimports test

api/v1alpha1/kubevirtmachinetemplate_webhook_test.go

@@ -55,8 +55,7 @@ var _ = Describe("Template Validation", func() {
 		})
 
 		It("should not return error", func() {
-			err := tests.newTemplate.ValidateUpdate(tests.oldTemplate)
-			Ω(err).ShouldNot(HaveOccurred())
+			Ω(tests.newTemplate.ValidateUpdate(tests.oldTemplate)).Should(Succeed())
 		})
 	})
 	Context("Template comparison with errors", func() {

clusterkubevirtadm/cmd/credentials/credentials_test.go

@@ -31,8 +31,7 @@ var _ = Describe("test credentials common function", func() {
 			client := fake.NewSimpleClientset()
 			cmdCtx.Client = client
 
-			err := ensureNamespace(context.Background(), cmdCtx, clientOperationCreate)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(ensureNamespace(context.Background(), cmdCtx, clientOperationCreate)).To(Succeed())
 
 			ns, err := client.CoreV1().Namespaces().Get(context.Background(), namespaceName, metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
@@ -44,8 +43,7 @@ var _ = Describe("test credentials common function", func() {
 			client := fake.NewSimpleClientset(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespaceName}})
 			cmdCtx.Client = client
 
-			err := ensureNamespace(context.Background(), cmdCtx, clientOperationApply)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(ensureNamespace(context.Background(), cmdCtx, clientOperationApply)).To(Succeed())
 
 			ns, err := client.CoreV1().Namespaces().Get(context.Background(), namespaceName, metav1.GetOptions{})
 			Expect(err).ToNot(HaveOccurred())
@@ -73,9 +71,7 @@ var _ = Describe("test credentials common function", func() {
 			client := fake.NewSimpleClientset(&corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespaceName}})
 			cmdCtx.Client = client
 
-			err := ensureServiceAccount(context.Background(), cmdCtx)
-
-			Expect(err).ToNot(HaveOccurred())
+			Expect(ensureServiceAccount(context.Background(), cmdCtx)).To(Succeed())
 		})
 
 		It("should do nothing if the serviceAccount is already exist", func() {
@@ -95,9 +91,7 @@ var _ = Describe("test credentials common function", func() {
 			)
 			cmdCtx.Client = client
 
-			err := ensureServiceAccount(context.Background(), cmdCtx)
-
-			Expect(err).ToNot(HaveOccurred())
+			Expect(ensureServiceAccount(context.Background(), cmdCtx)).To(Succeed())
 		})
 	})
 
@@ -115,8 +109,7 @@ var _ = Describe("test credentials common function", func() {
 			)
 			cmdCtx.Client = client
 
-			err := createOrUpdateRole(context.Background(), cmdCtx, clientOperationCreate)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(createOrUpdateRole(context.Background(), cmdCtx, clientOperationCreate)).To(Succeed())
 
 			roles, err := client.RbacV1().Roles(namespaceName).List(context.Background(), metav1.ListOptions{})
 			Expect(err).ToNot(HaveOccurred())
@@ -150,8 +143,7 @@ var _ = Describe("test credentials common function", func() {
 			)
 			cmdCtx.Client = client
 
-			err := createOrUpdateRole(context.Background(), cmdCtx, clientOperationCreate)
-			Expect(err).To(HaveOccurred())
+			Expect(createOrUpdateRole(context.Background(), cmdCtx, clientOperationCreate)).ToNot(Succeed())
 		})
 
 		It("should update the role if it is already exist, with different values", func() {
@@ -172,8 +164,7 @@ var _ = Describe("test credentials common function", func() {
 			)
 			cmdCtx.Client = client
 
-			err := createOrUpdateRole(context.Background(), cmdCtx, clientOperationApply)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(createOrUpdateRole(context.Background(), cmdCtx, clientOperationApply)).To(Succeed())
 
 			roles, err := client.RbacV1().Roles(namespaceName).List(context.Background(), metav1.ListOptions{})
 			Expect(err).ToNot(HaveOccurred())
@@ -206,8 +197,7 @@ var _ = Describe("test credentials common function", func() {
 			)
 			cmdCtx.Client = client
 
-			err := ensureRoleBinding(context.Background(), cmdCtx)
-			Expect(err).ToNot(HaveOccurred())
+			Expect(ensureRoleBinding(context.Background(), cmdCtx)).To(Succeed())
 
 			roleBindings, err := client.RbacV1().RoleBindings(namespaceName).List(context.Background(), metav1.ListOptions{})
 			Expect(err).ToNot(HaveOccurred())

controllers/kubevirtmachine_controller_test.go

@@ -358,8 +358,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		// should expect VM to be created with expected name
 		vm := &kubevirtv1.VirtualMachine{}
 		vmKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeClient.Get(gocontext.Background(), vmKey, vm)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(fakeClient.Get(gocontext.Background(), vmKey, vm)).To(Succeed())
 
 		// Should expect kubevirt machine is still not ready
 		Expect(machineContext.KubevirtMachine.Status.Ready).To(BeFalse())
@@ -369,8 +368,9 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		machineBootstrapSecretReferenceName := machineContext.Machine.Spec.Bootstrap.DataSecretName
 		machineBootstrapSecretReferenceKey := client.ObjectKey{Namespace: machineContext.Machine.GetNamespace(), Name: *machineBootstrapSecretReferenceName + "-userdata"}
 		bootstrapDataSecret := &corev1.Secret{}
-		err = fakeClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			fakeClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret),
+		).To(Succeed())
 		Expect(bootstrapDataSecret.Data).To(HaveKeyWithValue("userdata", []byte("shell-script")))
 		Expect(bootstrapDataSecret.Labels).To(HaveLen(1))
 		Expect(bootstrapDataSecret.Labels).To(HaveKeyWithValue("hello", "world"))
@@ -474,17 +474,17 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		// should expect VM to be created with expected name
 		vm := &kubevirtv1.VirtualMachine{}
 		vmKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeClient.Get(gocontext.Background(), vmKey, vm)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(fakeClient.Get(gocontext.Background(), vmKey, vm)).To(Succeed())
 
 		machineBootstrapSecretReferenceName := machineContext.Machine.Spec.Bootstrap.DataSecretName
 		machineBootstrapSecretReferenceKey := client.ObjectKey{Namespace: machineContext.Machine.GetNamespace(), Name: *machineBootstrapSecretReferenceName + "-userdata"}
 		infraClusterClient, _, err := infraClusterMock.GenerateInfraClusterClient(kubevirtMachine.Spec.InfraClusterSecretRef, kubevirtMachine.Namespace, machineContext.Context)
 		Expect(err).NotTo(HaveOccurred())
 
 		bootstrapDataSecret := &corev1.Secret{}
-		err = infraClusterClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			infraClusterClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret),
+		).To(Succeed())
 
 		Expect(bootstrapUserDataSecret.Data["userdata"]).To(Equal([]byte("shell-script")))
 	})
@@ -518,7 +518,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		err = fakeClient.Get(gocontext.Background(), vmKey, vm)
 		Expect(err).To(HaveOccurred())
 
-		Expect(machineContext.Machine.ObjectMeta.Finalizers).To(HaveLen(0))
+		Expect(machineContext.Machine.ObjectMeta.Finalizers).To(BeEmpty())
 	})
 
 	It("should create KubeVirt VM with externally managed cluster and no ssh key", func() {
@@ -549,8 +549,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		// should expect VM to be created with expected name
 		vm := &kubevirtv1.VirtualMachine{}
 		vmKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeClient.Get(gocontext.Background(), vmKey, vm)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(fakeClient.Get(gocontext.Background(), vmKey, vm)).To(Succeed())
 
 		// Should expect kubevirt machine is still not ready
 		Expect(machineContext.KubevirtMachine.Status.Ready).To(BeFalse())
@@ -560,8 +559,9 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		machineBootstrapSecretReferenceName := machineContext.Machine.Spec.Bootstrap.DataSecretName
 		machineBootstrapSecretReferenceKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: *machineBootstrapSecretReferenceName + "-userdata"}
 		bootstrapDataSecret := &corev1.Secret{}
-		err = fakeClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			fakeClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret),
+		).To(Succeed())
 		Expect(bootstrapDataSecret.Data).To(HaveKeyWithValue("userdata", []byte("shell-script")))
 		Expect(bootstrapDataSecret.Labels).To(HaveLen(1))
 		Expect(bootstrapDataSecret.Labels).To(HaveKeyWithValue("hello", "world"))
@@ -595,8 +595,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		// should expect VM to be created with expected name
 		vm := &kubevirtv1.VirtualMachine{}
 		vmKey := client.ObjectKey{Namespace: customNamespace, Name: kubevirtMachine.Name}
-		err = fakeClient.Get(gocontext.Background(), vmKey, vm)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(fakeClient.Get(gocontext.Background(), vmKey, vm)).To(Succeed())
 
 		// Should expect kubevirt machine is still not ready
 		Expect(machineContext.KubevirtMachine.Status.Ready).To(BeFalse())
@@ -606,8 +605,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		machineBootstrapSecretReferenceName := machineContext.Machine.Spec.Bootstrap.DataSecretName
 		machineBootstrapSecretReferenceKey := client.ObjectKey{Namespace: customNamespace, Name: *machineBootstrapSecretReferenceName + "-userdata"}
 		bootstrapDataSecret := &corev1.Secret{}
-		err = fakeClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(fakeClient.Get(gocontext.Background(), machineBootstrapSecretReferenceKey, bootstrapDataSecret)).To(Succeed())
 		Expect(bootstrapDataSecret.Data).To(HaveKeyWithValue("userdata", []byte("shell-script")))
 		Expect(bootstrapDataSecret.Labels).To(HaveLen(1))
 		Expect(bootstrapDataSecret.Labels).To(HaveKeyWithValue("hello", "world"))
@@ -654,8 +652,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 		// should expect VM to be created with expected name
 		vm := &kubevirtv1.VirtualMachine{}
 		vmKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeClient.Get(gocontext.Background(), vmKey, vm)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(fakeClient.Get(gocontext.Background(), vmKey, vm)).To(Succeed())
 
 		Expect(machineContext.KubevirtMachine.Status.Ready).To(BeTrue())
 		Expect(*machineContext.KubevirtMachine.Spec.ProviderID).To(Equal("kubevirt://" + kubevirtMachineName))
@@ -766,8 +763,9 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 			Expect(err).ShouldNot(HaveOccurred())
 
 			newKubevirtMachine := &infrav1.KubevirtMachine{}
-			err = kubevirtMachineReconciler.Client.Get(machineContext, kubevirtMachineKey, newKubevirtMachine)
-			Expect(err).ShouldNot(HaveOccurred())
+			Expect(
+				kubevirtMachineReconciler.Client.Get(machineContext, kubevirtMachineKey, newKubevirtMachine),
+			).To(Succeed())
 
 			conditions := newKubevirtMachine.GetConditions()
 			Expect(conditions[1].Type).To(Equal(infrav1.VMProvisionedCondition))
@@ -795,8 +793,7 @@ var _ = Describe("reconcile a kubevirt machine", func() {
 				Expect(err).ShouldNot(HaveOccurred())
 
 				newKubevirtMachine := &infrav1.KubevirtMachine{}
-				err = kubevirtMachineReconciler.Client.Get(machineContext, kubevirtMachineKey, newKubevirtMachine)
-				Expect(err).ShouldNot(HaveOccurred())
+				Expect(kubevirtMachineReconciler.Client.Get(machineContext, kubevirtMachineKey, newKubevirtMachine)).To(Succeed())
 
 				conditions := newKubevirtMachine.GetConditions()
 				Expect(conditions[1].Type).To(Equal(infrav1.VMProvisionedCondition))
@@ -1121,10 +1118,11 @@ var _ = Describe("updateNodeProviderID", func() {
 		Expect(out).To(Equal(ctrl.Result{}))
 		workloadClusterNode := &corev1.Node{}
 		workloadClusterNodeKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeWorkloadClusterClient.Get(machineContext, workloadClusterNodeKey, workloadClusterNode)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			fakeWorkloadClusterClient.Get(machineContext, workloadClusterNodeKey, workloadClusterNode),
+		).To(Succeed())
 		Expect(workloadClusterNode.Spec.ProviderID).To(Equal(expectedProviderId))
-		Expect(kubevirtMachine.Status.NodeUpdated).To(Equal(true))
+		Expect(kubevirtMachine.Status.NodeUpdated).To(BeTrue())
 	})
 
 	It("GenerateWorkloadClusterClient failure", func() {
@@ -1136,10 +1134,11 @@ var _ = Describe("updateNodeProviderID", func() {
 		Expect(out).To(Equal(ctrl.Result{RequeueAfter: 10 * time.Second}))
 		workloadClusterNode := &corev1.Node{}
 		workloadClusterNodeKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeWorkloadClusterClient.Get(machineContext, workloadClusterNodeKey, workloadClusterNode)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			fakeWorkloadClusterClient.Get(machineContext, workloadClusterNodeKey, workloadClusterNode),
+		).To(Succeed())
 		Expect(workloadClusterNode.Spec.ProviderID).NotTo(Equal(expectedProviderId))
-		Expect(kubevirtMachine.Status.NodeUpdated).To(Equal(false))
+		Expect(kubevirtMachine.Status.NodeUpdated).To(BeFalse())
 	})
 
 	It("Node doesn't exist", func() {
@@ -1151,9 +1150,10 @@ var _ = Describe("updateNodeProviderID", func() {
 		Expect(out).To(Equal(ctrl.Result{RequeueAfter: 10 * time.Second}))
 		workloadClusterNode := &corev1.Node{}
 		workloadClusterNodeKey := client.ObjectKey{Namespace: kubevirtMachine.Namespace, Name: kubevirtMachine.Name}
-		err = fakeWorkloadClusterClient.Get(machineContext, workloadClusterNodeKey, workloadClusterNode)
-		Expect(err).NotTo(HaveOccurred())
+		Expect(
+			fakeWorkloadClusterClient.Get(machineContext, workloadClusterNodeKey, workloadClusterNode),
+		).To(Succeed())
 		Expect(workloadClusterNode.Spec.ProviderID).NotTo(Equal(expectedProviderId))
-		Expect(kubevirtMachine.Status.NodeUpdated).To(Equal(false))
+		Expect(kubevirtMachine.Status.NodeUpdated).To(BeFalse())
 	})
 })

controllers/vmi_eviction_controller_test.go

@@ -289,8 +289,9 @@ var _ = Describe("Test VMI Controller", func() {
 
 				// check that the VMI was not deleted
 				readVMI := &kubevirtv1.VirtualMachineInstance{}
-				err = fakeClient.Get(gocontext.TODO(), client.ObjectKey{Namespace: clusterNamespace, Name: nodeName}, readVMI)
-				Expect(err).ShouldNot(HaveOccurred())
+				Expect(
+					fakeClient.Get(gocontext.TODO(), client.ObjectKey{Namespace: clusterNamespace, Name: nodeName}, readVMI),
+				).To(Succeed())
 				Expect(readVMI).ToNot(BeNil())
 			})
 		})