From a7cd8599199f791206f22a94e3ec9949bacf3c7b Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov Date: Mon, 19 Feb 2018 17:46:31 +0100 Subject: [PATCH] Some improvements for golint, ineffassign and misspell --- pkg/apiserver/apiserver.go | 9 +++------ pkg/cluster/k8sres.go | 6 +++--- pkg/cluster/pg.go | 2 +- pkg/cluster/resources.go | 3 +-- pkg/cluster/util.go | 2 +- pkg/util/retryutil/retry_util.go | 12 ++++++++---- pkg/util/retryutil/retry_util_test.go | 8 ++++---- pkg/util/users/users.go | 18 +++++++++--------- 8 files changed, 30 insertions(+), 30 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 2b0b2f98e..e174ab750 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -163,8 +163,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) - return + resp, err = clusterNames, nil } else if matches := util.FindNamedStringSubmatch(clusterLogsURL, req.URL.Path); matches != nil { resp, err = s.controller.ClusterLogs(matches["team"], matches["cluster"]) } else if matches := util.FindNamedStringSubmatch(clusterHistoryURL, req.URL.Path); matches != nil { @@ -176,11 +175,9 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) { res[team] = append(res[team], cluster.Name[len(team)+1:]) } } - - s.respond(res, nil, w) - return + err = nil } else { - err = fmt.Errorf("page not found") + resp, err = nil, fmt.Errorf("page not found") } s.respond(resp, err, w) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 1bc18422e..db1b851ba 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -274,9 +274,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( @@ -373,7 +373,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 { diff --git a/pkg/cluster/pg.go b/pkg/cluster/pg.go index 0d7f471aa..09c2e16c1 100644 --- a/pkg/cluster/pg.go +++ b/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. diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 4f770aaff..c751a5eff 100644 --- a/pkg/cluster/resources.go +++ b/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) diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index 9bd292271..b1435ae56 100644 --- a/pkg/cluster/util.go +++ b/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 diff --git a/pkg/util/retryutil/retry_util.go b/pkg/util/retryutil/retry_util.go index 4b5f3f98c..cbae3bb1b 100644 --- a/pkg/util/retryutil/retry_util.go +++ b/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: -// * it returns boolean true -// * a timeout expires -// * an error occurs +// Retry is a wrapper around RetryWorker that provides a real RetryTicker 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, diff --git a/pkg/util/retryutil/retry_util_test.go b/pkg/util/retryutil/retry_util_test.go index 05ae60d4b..d4edf9613 100644 --- a/pkg/util/retryutil/retry_util_test.go +++ b/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,13 @@ func TestRetryWorkerOneFalse(t *testing.T) { tick := &mockTicker{t, 0} result := RetryWorker(1, 3, tick, func() (bool, error) { - counter += 1 + counter++ if counter <= 1 { return false, nil - } else { - return true, nil } + + return true, nil }) if result != nil { diff --git a/pkg/util/users/users.go b/pkg/util/users/users.go index 8d6acd9f8..9d435fd53 100644 --- a/pkg/util/users/users.go +++ b/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