Integrate in CI and align with the golangci-lint default rules

.github/workflows/run_e2e.yaml

@@ -11,17 +11,16 @@ jobs:
     name: End-2-End tests
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v1
-    - uses: actions/setup-go@v2
+    - uses: actions/checkout@v3
+    - uses: actions/setup-go@v3
       with:
-          go-version: "^1.17.4"
+          go-version: 1.18
+          cache: false
     - name: Make dependencies
       run: make deps mocks
     - name: Code generation
       run: make codegen
     - name: Compile
       run: make linux
-    - name: Run unit tests
-      run: make test
     - name: Run end-2-end tests
       run: make e2e

.github/workflows/run_golangci-lint.yaml

@@ -0,0 +1,28 @@
+name: operator-golangci-lint
+
+on:
+  pull_request:
+  push:
+    branches:
+      - master
+
+jobs:
+  golangci:
+    name: Golang code linting
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: 1.18
+          cache: false
+      - uses: actions/checkout@v3
+      - name: Generate mocks
+        run: make tools mocks
+      - name: run golangci-lint
+        uses: golangci/golangci-lint-action@v3
+        with:
+          version: v1.46.2
+          args: --timeout=10m --issues-exit-code=0 ./...
+          skip-pkg-cache: true
+          skip-build-cache: true
+          skip-go-installation: true

.github/workflows/run_tests.yaml

@@ -11,18 +11,21 @@ jobs:
     name: Unit tests and coverage
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v2
-    - uses: actions/setup-go@v2
+    - uses: actions/checkout@v3
+    - uses: actions/setup-go@v3
       with:
-          go-version: "^1.17.4"
+          go-version: 1.18
+          cache: false
     - name: Make dependencies
       run: make deps mocks
+    - name: Validate code generating
+      run: make verify-codegen
     - name: Compile
       run: make linux
     - name: Run unit tests
       run: go test -race -covermode atomic -coverprofile=coverage.out ./...
     - name: Convert coverage to lcov
-      uses: jandelgado/gcov2lcov-action@v1.0.8
+      uses: jandelgado/gcov2lcov-action@v1.0.9
     - name: Coveralls
       uses: coverallsapp/github-action@master
       with:

Makefile

@@ -48,7 +48,8 @@ SHELL := env PATH=$(PATH) $(SHELL)
 default: local
 
 clean:
-	rm -rf build scm-source.json
+	rm -rf build e2e/manifests docker/build scm-source.json
+	rm -f mocks/httpclient.go mocks/volumes.go
 
 local: ${SOURCES}
 	hack/verify-codegen.sh
@@ -99,12 +100,17 @@ vet:
 deps: tools
 	GO111MODULE=on go mod vendor
 
-test:
-	hack/verify-codegen.sh
+test: mocks verify-codegen
 	GO111MODULE=on go test ./...
 
+verify-codegen:
+	hack/verify-codegen.sh
+
 codegen:
 	hack/update-codegen.sh
 
+lint:
+	golangci-lint run
+
 e2e: docker # build operator image to be tested
 	cd e2e; make e2etest

pkg/apis/acid.zalan.do/v1/const.go

@@ -1,6 +1,6 @@
 package v1
 
-// 	ClusterStatusUnknown etc : status of a Postgres cluster known to the operator
+// ClusterStatusUnknown etc : status of a Postgres cluster known to the operator
 const (
 	ClusterStatusUnknown      = ""
 	ClusterStatusCreating     = "Creating"

pkg/apis/acid.zalan.do/v1/crds.go

@@ -748,9 +748,9 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{
 							},
 						},
 						OneOf: []apiextv1.JSONSchemaProps{
-							apiextv1.JSONSchemaProps{Required: []string{"s3_wal_path"}},
-							apiextv1.JSONSchemaProps{Required: []string{"gs_wal_path"}},
-							apiextv1.JSONSchemaProps{Required: []string{"standby_host"}},
+							{Required: []string{"s3_wal_path"}},
+							{Required: []string{"gs_wal_path"}},
+							{Required: []string{"standby_host"}},
 						},
 					},
 					"streams": {

pkg/apis/acid.zalan.do/v1/util.go

@@ -85,22 +85,22 @@ func validateCloneClusterDescription(clone *CloneDescription) error {
 }
 
 // Success of the current Status
-func (postgresStatus PostgresStatus) Success() bool {
-	return postgresStatus.PostgresClusterStatus != ClusterStatusAddFailed &&
-		postgresStatus.PostgresClusterStatus != ClusterStatusUpdateFailed &&
-		postgresStatus.PostgresClusterStatus != ClusterStatusSyncFailed
+func (ps *PostgresStatus) Success() bool {
+	return ps.PostgresClusterStatus != ClusterStatusAddFailed &&
+		ps.PostgresClusterStatus != ClusterStatusUpdateFailed &&
+		ps.PostgresClusterStatus != ClusterStatusSyncFailed
 }
 
 // Running status of cluster
-func (postgresStatus PostgresStatus) Running() bool {
-	return postgresStatus.PostgresClusterStatus == ClusterStatusRunning
+func (ps *PostgresStatus) Running() bool {
+	return ps.PostgresClusterStatus == ClusterStatusRunning
 }
 
 // Creating status of cluster
-func (postgresStatus PostgresStatus) Creating() bool {
-	return postgresStatus.PostgresClusterStatus == ClusterStatusCreating
+func (ps *PostgresStatus) Creating() bool {
+	return ps.PostgresClusterStatus == ClusterStatusCreating
 }
 
-func (postgresStatus PostgresStatus) String() string {
-	return postgresStatus.PostgresClusterStatus
+func (ps *PostgresStatus) String() string {
+	return ps.PostgresClusterStatus
 }

pkg/apis/acid.zalan.do/v1/util_test.go

@@ -781,8 +781,6 @@ func TestPostgresListMeta(t *testing.T) {
 			if a := tt.out.GetListMeta(); reflect.DeepEqual(a, tt.out.ListMeta) {
 				t.Errorf("GetObjectMeta expected: %v, got: %v", tt.out.ListMeta, a)
 			}
-
-			return
 		})
 	}
 }

pkg/cluster/cluster.go

@@ -254,13 +254,13 @@ func (c *Cluster) Create() error {
 
 	defer func() {
 		if err == nil {
-			c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning) //TODO: are you sure it's running?
+			_, _ = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning) //TODO: are you sure it's running?
 		} else {
-			c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusAddFailed)
+			_, _ = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusAddFailed)
 		}
 	}()
 
-	c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusCreating)
+	_, _ = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusCreating)
 	c.eventRecorder.Event(c.GetReference(), v1.EventTypeNormal, "Create", "Started creation of new cluster resources")
 
 	for _, role := range []PostgresRole{Master, Replica} {
@@ -368,7 +368,7 @@ func (c *Cluster) Create() error {
 	//
 	// Do not consider connection pooler as a strict requirement, and if
 	// something fails, report warning
-	c.createConnectionPooler(c.installLookupFunction)
+	_, _ = c.createConnectionPooler(c.installLookupFunction)
 
 	if len(c.Spec.Streams) > 0 {
 		if err = c.syncStreams(); err != nil {
@@ -610,7 +610,7 @@ func compareEnv(a, b []v1.EnvVar) bool {
 	if len(a) != len(b) {
 		return false
 	}
-	equal := true
+	var equal bool
 	for _, enva := range a {
 		hasmatch := false
 		for _, envb := range b {
@@ -622,7 +622,7 @@ func compareEnv(a, b []v1.EnvVar) bool {
 					if enva.Value == "" && envb.Value == "" {
 						equal = reflect.DeepEqual(enva.ValueFrom, envb.ValueFrom)
 					} else {
-						equal = (enva.Value == envb.Value)
+						equal = enva.Value == envb.Value
 					}
 				}
 				if !equal {
@@ -763,14 +763,14 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdating)
+	_, _ = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdating)
 	c.setSpec(newSpec)
 
 	defer func() {
 		if updateFailed {
-			c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdateFailed)
+			_, _ = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusUpdateFailed)
 		} else {
-			c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning)
+			_, _ = c.KubeClient.SetPostgresCRDStatus(c.clusterName(), acidv1.ClusterStatusRunning)
 		}
 	}()
 
@@ -831,7 +831,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 
 	// Volume
 	if c.OpConfig.StorageResizeMode != "off" {
-		c.syncVolumes()
+		_ = c.syncVolumes()
 	} else {
 		c.logger.Infof("Storage resize is disabled (storage_resize_mode is off). Skipping volume sync.")
 	}
@@ -1091,7 +1091,7 @@ func (c *Cluster) processPodEventQueue(stopCh <-chan struct{}) {
 		case <-stopCh:
 			return
 		default:
-			if _, err := c.podEventsQueue.Pop(cache.PopProcessFunc(c.processPodEvent)); err != nil {
+			if _, err := c.podEventsQueue.Pop(c.processPodEvent); err != nil {
 				c.logger.Errorf("error when processing pod event queue %v", err)
 			}
 		}
@@ -1382,7 +1382,7 @@ func (c *Cluster) initTeamMembers(teamID string, isPostgresSuperuserTeam bool) e
 func (c *Cluster) initHumanUsers() error {
 
 	var clusterIsOwnedBySuperuserTeam bool
-	superuserTeams := []string{}
+	var superuserTeams []string
 
 	if c.OpConfig.EnablePostgresTeamCRD && c.OpConfig.EnablePostgresTeamCRDSuperusers && c.Config.PgTeamMap != nil {
 		superuserTeams = c.Config.PgTeamMap.GetAdditionalSuperuserTeams(c.Spec.TeamID, true)

pkg/cluster/cluster_test.go

@@ -222,7 +222,7 @@ type mockTeamsAPIClient struct {
 	members []string
 }
 
-func (m *mockTeamsAPIClient) TeamInfo(teamID, token string) (tm *teams.Team, statusCode int, err error) {
+func (m *mockTeamsAPIClient) TeamInfo(teamID, _ string) (tm *teams.Team, statusCode int, err error) {
 	if len(m.members) > 0 {
 		return &teams.Team{Members: m.members}, http.StatusOK, nil
 	}
@@ -314,7 +314,7 @@ type mockTeamsAPIClientMultipleTeams struct {
 	teams []mockTeam
 }
 
-func (m *mockTeamsAPIClientMultipleTeams) TeamInfo(teamID, token string) (tm *teams.Team, statusCode int, err error) {
+func (m *mockTeamsAPIClientMultipleTeams) TeamInfo(teamID, _ string) (tm *teams.Team, statusCode int, err error) {
 	for _, team := range m.teams {
 		if team.teamID == teamID {
 			return &teams.Team{Members: team.members}, http.StatusOK, nil
@@ -974,7 +974,7 @@ func TestInitSystemUsers(t *testing.T) {
 
 func TestPreparedDatabases(t *testing.T) {
 	cl.Spec.PreparedDatabases = map[string]acidv1.PreparedDatabase{}
-	cl.initPreparedDatabaseRoles()
+	_ = cl.initPreparedDatabaseRoles()
 
 	for _, role := range []string{"acid_test_owner", "acid_test_reader", "acid_test_writer",
 		"acid_test_data_owner", "acid_test_data_reader", "acid_test_data_writer"} {
@@ -995,7 +995,7 @@ func TestPreparedDatabases(t *testing.T) {
 			},
 		},
 	}
-	cl.initPreparedDatabaseRoles()
+	_ = cl.initPreparedDatabaseRoles()
 
 	for _, role := range []string{
 		"foo_owner", "foo_reader", "foo_writer",

pkg/cluster/connection_pooler.go

@@ -163,22 +163,24 @@ func (c *Cluster) createConnectionPooler(LookupFunction InstallFunction) (SyncRe
 	return reason, nil
 }
 
-//
 // Generate pool size related environment variables.
 //
 // MAX_DB_CONN would specify the global maximum for connections to a target
-// 	database.
+//
+//	database.
 //
 // MAX_CLIENT_CONN is not configurable at the moment, just set it high enough.
 //
 // DEFAULT_SIZE is a pool size per db/user (having in mind the use case when
-// 	most of the queries coming through a connection pooler are from the same
-// 	user to the same db). In case if we want to spin up more connection pooler
-// 	instances, take this into account and maintain the same number of
-// 	connections.
+//
+//	most of the queries coming through a connection pooler are from the same
+//	user to the same db). In case if we want to spin up more connection pooler
+//	instances, take this into account and maintain the same number of
+//	connections.
 //
 // MIN_SIZE is a pool's minimal size, to prevent situation when sudden workload
-// 	have to wait for spinning up a new connections.
+//
+//	have to wait for spinning up a new connections.
 //
 // RESERVE_SIZE is how many additional connections to allow for a pooler.
 func (c *Cluster) getConnectionPoolerEnvVars() []v1.EnvVar {
@@ -546,7 +548,7 @@ func (c *Cluster) listPoolerPods(listOptions metav1.ListOptions) ([]v1.Pod, erro
 	return pods.Items, nil
 }
 
-//delete connection pooler
+// delete connection pooler
 func (c *Cluster) deleteConnectionPooler(role PostgresRole) (err error) {
 	c.logger.Infof("deleting connection pooler spilo-role=%s", role)
 
@@ -605,7 +607,7 @@ func (c *Cluster) deleteConnectionPooler(role PostgresRole) (err error) {
 	return nil
 }
 
-//delete connection pooler
+// delete connection pooler
 func (c *Cluster) deleteConnectionPoolerSecret() (err error) {
 	// Repeat the same for the secret object
 	secretName := c.credentialSecretName(c.OpConfig.ConnectionPooler.User)
@@ -654,7 +656,7 @@ func updateConnectionPoolerDeployment(KubeClient k8sutil.KubernetesClient, newDe
 	return deployment, nil
 }
 
-//updateConnectionPoolerAnnotations updates the annotations of connection pooler deployment
+// updateConnectionPoolerAnnotations updates the annotations of connection pooler deployment
 func updateConnectionPoolerAnnotations(KubeClient k8sutil.KubernetesClient, deployment *appsv1.Deployment, annotations map[string]string) (*appsv1.Deployment, error) {
 	patchData, err := metaAnnotationsPatch(annotations)
 	if err != nil {
@@ -664,7 +666,7 @@ func updateConnectionPoolerAnnotations(KubeClient k8sutil.KubernetesClient, depl
 		context.TODO(),
 		deployment.Name,
 		types.MergePatchType,
-		[]byte(patchData),
+		patchData,
 		metav1.PatchOptions{},
 		"")
 	if err != nil {

pkg/cluster/connection_pooler_new_test.go

@@ -29,7 +29,7 @@ func TestFakeClient(t *testing.T) {
 		},
 	}
 
-	clientSet.AppsV1().Deployments(namespace).Create(context.TODO(), deployment, metav1.CreateOptions{})
+	_, _ = clientSet.AppsV1().Deployments(namespace).Create(context.TODO(), deployment, metav1.CreateOptions{})
 
 	deployment2, _ := clientSet.AppsV1().Deployments(namespace).Get(context.TODO(), "my-deployment1", metav1.GetOptions{})