Merge pull request #240 from zalando-incubator/feature/goreport-improvements

Some improvements for golint, ineffassign and misspell

pkg/apiserver/apiserver.go

@@ -164,8 +164,7 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {
 			clusterNames = append(clusterNames, cluster.Name[len(matches["team"])+1:])
 		}
 
-		s.respond(clusterNames, nil, w)
+		resp, err = clusterNames, nil
-		return
 	} else if matches := util.FindNamedStringSubmatch(clusterLogsURL, req.URL.Path); matches != nil {
 		namespace, _ := matches["namespace"]
 		resp, err = s.controller.ClusterLogs(matches["team"], namespace, matches["cluster"])
@@ -173,17 +172,15 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {
 		namespace, _ := matches["namespace"]
 		resp, err = s.controller.ClusterHistory(matches["team"], namespace, matches["cluster"])
 	} else if req.URL.Path == clustersURL {
-		res := make(map[string][]string)
+		clusterNamesPerTeam := make(map[string][]string)
 		for team, clusters := range s.controller.TeamClusterList() {
 			for _, cluster := range clusters {
-				res[team] = append(res[team], cluster.Name[len(team)+1:])
+				clusterNamesPerTeam[team] = append(clusterNamesPerTeam[team], cluster.Name[len(team)+1:])
 			}
 		}
-
+		resp, err = clusterNamesPerTeam, nil
-		s.respond(res, nil, w)
-		return
 	} else {
-		err = fmt.Errorf("page not found")
+		resp, err = nil, fmt.Errorf("page not found")
 	}
 
 	s.respond(resp, err, w)

pkg/cluster/k8sres.go

@@ -275,9 +275,9 @@ func (c *Cluster) tolerations(tolerationsSpec *[]v1.Toleration) []v1.Toleration
 				Effect:   v1.TaintEffect(podToleration["effect"]),
 			},
 		}
-	} else {
-		return []v1.Toleration{}
 	}
+
+	return []v1.Toleration{}
 }
 
 func (c *Cluster) generatePodTemplate(
@@ -376,7 +376,7 @@ func (c *Cluster) generatePodTemplate(
 	var names []string
 	// handle environment variables from the PodEnvironmentConfigMap. We don't use envSource here as it is impossible
 	// to track any changes to the object envSource points to. In order to emulate the envSource behavior, however, we
-	// need to make sure that PodConfigMap variables doesn't override those we set explicitely from the configuration
+	// need to make sure that PodConfigMap variables doesn't override those we set explicitly from the configuration
 	// parameters
 	envVarsMap := make(map[string]string)
 	for _, envVar := range envVars {

pkg/cluster/pg.go

@@ -180,7 +180,7 @@ func (c *Cluster) getDatabases() (map[string]string, error) {
 		dbs[datname] = owner
 	}
 
-	return dbs, nil
+	return dbs, err
 }
 
 // executeCreateDatabase creates new database with the given owner.

pkg/cluster/resources.go

@@ -419,9 +419,8 @@ func (c *Cluster) deletePodDisruptionBudget() error {
 			}
 			if k8sutil.ResourceNotFound(err2) {
 				return true, nil
-			} else {
-				return false, err2
 			}
+			return false, err2
 		})
 	if err != nil {
 		return fmt.Errorf("could not delete pod disruption budget: %v", err)

pkg/cluster/util.go

@@ -28,7 +28,7 @@ type OAuthTokenGetter interface {
 	getOAuthToken() (string, error)
 }
 
-// OAuthTokenGetter enables fetching OAuth tokens by reading Kubernetes secrets
+// SecretOauthTokenGetter enables fetching OAuth tokens by reading Kubernetes secrets
 type SecretOauthTokenGetter struct {
 	kubeClient           *k8sutil.KubernetesClient
 	OAuthTokenSecretName spec.NamespacedName

pkg/util/retryutil/retry_util.go

@@ -5,11 +5,14 @@ import (
 	"time"
 )
 
+// RetryTicker is a wrapper aroung time.Tick,
+// that allows to mock its implementation
 type RetryTicker interface {
 	Stop()
 	Tick()
 }
 
+// Ticker is a real implementation of RetryTicker interface
 type Ticker struct {
 	ticker *time.Ticker
 }
@@ -18,10 +21,7 @@ func (t *Ticker) Stop() { t.ticker.Stop() }
 
 func (t *Ticker) Tick() { <-t.ticker.C }
 
-// Retry calls ConditionFunc until either:
+// Retry is a wrapper around RetryWorker that provides a real RetryTicker
-// * it returns boolean true
-// * a timeout expires
-// * an error occurs
 func Retry(interval time.Duration, timeout time.Duration, f func() (bool, error)) error {
 	//TODO: make the retry exponential
 	if timeout < interval {
@@ -31,6 +31,10 @@ func Retry(interval time.Duration, timeout time.Duration, f func() (bool, error)
 	return RetryWorker(interval, timeout, tick, f)
 }
 
+// RetryWorker calls ConditionFunc until either:
+// * it returns boolean true
+// * a timeout expires
+// * an error occurs
 func RetryWorker(
 	interval time.Duration,
 	timeout time.Duration,

pkg/util/retryutil/retry_util_test.go

@@ -13,7 +13,7 @@ type mockTicker struct {
 func (t *mockTicker) Stop() {}
 
 func (t *mockTicker) Tick() {
-	t.counter += 1
+	t.counter++
 }
 
 func TestRetryWorkerSuccess(t *testing.T) {
@@ -36,13 +36,8 @@ func TestRetryWorkerOneFalse(t *testing.T) {
 
 	tick := &mockTicker{t, 0}
 	result := RetryWorker(1, 3, tick, func() (bool, error) {
-		counter += 1
+		counter++
-
+		return counter > 1, nil
-		if counter <= 1 {
-			return false, nil
-		} else {
-			return true, nil
-		}
 	})
 
 	if result != nil {

pkg/util/users/users.go

@@ -29,7 +29,7 @@ type DefaultUserSyncStrategy struct {
 }
 
 // ProduceSyncRequests figures out the types of changes that need to happen with the given users.
-func (s DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserMap,
+func (strategy DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserMap,
 	newUsers spec.PgUserMap) (reqs []spec.PgSyncUserRequest) {
 
 	// No existing roles are deleted or stripped of role memebership/flags
@@ -70,19 +70,19 @@ func (s DefaultUserSyncStrategy) ProduceSyncRequests(dbUsers spec.PgUserMap,
 }
 
 // ExecuteSyncRequests makes actual database changes from the requests passed in its arguments.
-func (s DefaultUserSyncStrategy) ExecuteSyncRequests(reqs []spec.PgSyncUserRequest, db *sql.DB) error {
+func (strategy DefaultUserSyncStrategy) ExecuteSyncRequests(reqs []spec.PgSyncUserRequest, db *sql.DB) error {
 	for _, r := range reqs {
 		switch r.Kind {
 		case spec.PGSyncUserAdd:
-			if err := s.createPgUser(r.User, db); err != nil {
+			if err := strategy.createPgUser(r.User, db); err != nil {
 				return fmt.Errorf("could not create user %q: %v", r.User.Name, err)
 			}
 		case spec.PGsyncUserAlter:
-			if err := s.alterPgUser(r.User, db); err != nil {
+			if err := strategy.alterPgUser(r.User, db); err != nil {
 				return fmt.Errorf("could not alter user %q: %v", r.User.Name, err)
 			}
 		case spec.PGSyncAlterSet:
-			if err := s.alterPgUserSet(r.User, db); err != nil {
+			if err := strategy.alterPgUserSet(r.User, db); err != nil {
 				return fmt.Errorf("could not set custom user %q parameters: %v", r.User.Name, err)
 			}
 		default:
@@ -102,7 +102,7 @@ func (strategy DefaultUserSyncStrategy) alterPgUserSet(user spec.PgUser, db *sql
 	return
 }
 
-func (s DefaultUserSyncStrategy) createPgUser(user spec.PgUser, db *sql.DB) (err error) {
+func (strategy DefaultUserSyncStrategy) createPgUser(user spec.PgUser, db *sql.DB) (err error) {
 	var userFlags []string
 	var userPassword string
 
@@ -129,7 +129,7 @@ func (s DefaultUserSyncStrategy) createPgUser(user spec.PgUser, db *sql.DB) (err
 	return
 }
 
-func (s DefaultUserSyncStrategy) alterPgUser(user spec.PgUser, db *sql.DB) (err error) {
+func (strategy DefaultUserSyncStrategy) alterPgUser(user spec.PgUser, db *sql.DB) (err error) {
 	var resultStmt []string
 
 	if user.Password != "" || len(user.Flags) > 0 {
@@ -206,9 +206,9 @@ func quoteParameterValue(name, val string) string {
 		// in the schema name would break the parsing code in the operator.)
 		if start == '\'' && end == '\'' {
 			return fmt.Sprintf("%s", val[1:len(val)-1])
-		} else {
-			return val
 		}
+
+		return val
 	}
 	if (start == '"' && end == '"') || (start == '\'' && end == '\'') {
 		return val