Skip to content

Commit b06186e

Browse files
alexeyklyukinalexeyklyukin
authored
Linter-induced code refactoring, run round 2. (zalando#360)
Run more linters in the gometalinter, i.e. deadcode, megacheck, nakedret, dup. More consistent code formatting, remove two dead functions, eliminate naked a bunch of naked returns, refactor a few functions to avoid code duplication.
1 parent 50f079c commit b06186e

File tree

15 files changed

+140
-188
lines changed

15 files changed

+140
-188
lines changed

‎pkg/apiserver/apiserver.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {
157157
)
158158

159159
if matches := util.FindNamedStringSubmatch(clusterStatusURL, req.URL.Path); matches != nil {
160-
namespace, _ := matches["namespace"]
160+
namespace := matches["namespace"]
161161
resp, err = s.controller.ClusterStatus(matches["team"], namespace, matches["cluster"])
162162
} else if matches := util.FindNamedStringSubmatch(teamURL, req.URL.Path); matches != nil {
163163
teamClusters := s.controller.TeamClusterList()
@@ -174,10 +174,10 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {
174174

175175
resp, err = clusterNames, nil
176176
} else if matches := util.FindNamedStringSubmatch(clusterLogsURL, req.URL.Path); matches != nil {
177-
namespace, _ := matches["namespace"]
177+
namespace := matches["namespace"]
178178
resp, err = s.controller.ClusterLogs(matches["team"], namespace, matches["cluster"])
179179
} else if matches := util.FindNamedStringSubmatch(clusterHistoryURL, req.URL.Path); matches != nil {
180-
namespace, _ := matches["namespace"]
180+
namespace := matches["namespace"]
181181
resp, err = s.controller.ClusterHistory(matches["team"], namespace, matches["cluster"])
182182
} else if req.URL.Path == clustersURL {
183183
clusterNamesPerTeam := make(map[string][]string)
@@ -194,8 +194,8 @@ func (s *Server) clusters(w http.ResponseWriter, req *http.Request) {
194194
s.respond(resp, err, w)
195195
}
196196

197-
func mustConvertToUint32(s string) uint32{
198-
result, err := strconv.Atoi(s);
197+
func mustConvertToUint32(s string) uint32 {
198+
result, err := strconv.Atoi(s)
199199
if err != nil {
200200
panic(fmt.Errorf("mustConvertToUint32 called for %s: %v", s, err))
201201
}
@@ -244,8 +244,6 @@ func (s *Server) databases(w http.ResponseWriter, req *http.Request) {
244244

245245
databaseNamesPerCluster := s.controller.ClusterDatabasesMap()
246246
s.respond(databaseNamesPerCluster, nil, w)
247-
return
248-
249247
}
250248

251249
func (s *Server) allQueues(w http.ResponseWriter, r *http.Request) {

‎pkg/cluster/cluster.go

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -450,16 +450,16 @@ func (c *Cluster) compareContainers(setA, setB *v1beta1.StatefulSet) (bool, []st
450450
return needsRollUpdate, reasons
451451
}
452452

453-
func compareResources(a *v1.ResourceRequirements, b *v1.ResourceRequirements) (equal bool) {
454-
equal = true
453+
func compareResources(a *v1.ResourceRequirements, b *v1.ResourceRequirements) bool {
454+
equal := true
455455
if a != nil {
456456
equal = compareResoucesAssumeFirstNotNil(a, b)
457457
}
458458
if equal && (b != nil) {
459459
equal = compareResoucesAssumeFirstNotNil(b, a)
460460
}
461461

462-
return
462+
return equal
463463
}
464464

465465
func compareResoucesAssumeFirstNotNil(a *v1.ResourceRequirements, b *v1.ResourceRequirements) bool {
@@ -786,15 +786,16 @@ func (c *Cluster) initInfrastructureRoles() error {
786786
}
787787

788788
// resolves naming conflicts between existing and new roles by chosing either of them.
789-
func (c *Cluster) resolveNameConflict(currentRole, newRole *spec.PgUser) (result spec.PgUser) {
789+
func (c *Cluster) resolveNameConflict(currentRole, newRole *spec.PgUser) spec.PgUser {
790+
var result spec.PgUser
790791
if newRole.Origin >= currentRole.Origin {
791792
result = *newRole
792793
} else {
793794
result = *currentRole
794795
}
795796
c.logger.Debugf("resolved a conflict of role %q between %s and %s to %s",
796797
newRole.Name, newRole.Origin, currentRole.Origin, result.Origin)
797-
return
798+
return result
798799
}
799800

800801
func (c *Cluster) shouldAvoidProtectedOrSystemRole(username, purpose string) bool {
@@ -838,8 +839,9 @@ func (c *Cluster) GetStatus() *spec.ClusterStatus {
838839
}
839840

840841
// Switchover does a switchover (via Patroni) to a candidate pod
841-
func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName) (err error) {
842+
func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName) error {
842843

844+
var err error
843845
c.logger.Debugf("failing over from %q to %q", curMaster.Name, candidate)
844846

845847
var wg sync.WaitGroup
@@ -858,8 +860,8 @@ func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName) (
858860

859861
select {
860862
case <-stopCh:
861-
case podLabelErr <- func() (err error) {
862-
_, err = c.waitForPodLabel(ch, stopCh, &role)
863+
case podLabelErr <- func() (err2 error) {
864+
_, err2 = c.waitForPodLabel(ch, stopCh, &role)
863865
return
864866
}():
865867
}
@@ -882,7 +884,7 @@ func (c *Cluster) Switchover(curMaster *v1.Pod, candidate spec.NamespacedName) (
882884
// close the label waiting channel no sooner than the waiting goroutine terminates.
883885
close(podLabelErr)
884886

885-
return
887+
return err
886888

887889
}
888890

‎pkg/cluster/k8sres.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,13 @@ import (
77

88
"github.com/Sirupsen/logrus"
99

10+
"k8s.io/api/apps/v1beta1"
11+
"k8s.io/api/core/v1"
12+
policybeta1 "k8s.io/api/policy/v1beta1"
1013
"k8s.io/apimachinery/pkg/api/resource"
1114
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1215
"k8s.io/apimachinery/pkg/types"
1316
"k8s.io/apimachinery/pkg/util/intstr"
14-
"k8s.io/api/core/v1"
15-
"k8s.io/api/apps/v1beta1"
16-
policybeta1 "k8s.io/api/policy/v1beta1"
1717

1818
"github.com/zalando-incubator/postgres-operator/pkg/spec"
1919
"github.com/zalando-incubator/postgres-operator/pkg/util"
@@ -90,7 +90,7 @@ func (c *Cluster) makeDefaultResources() spec.Resources {
9090
defaultRequests := spec.ResourceDescription{CPU: config.DefaultCPURequest, Memory: config.DefaultMemoryRequest}
9191
defaultLimits := spec.ResourceDescription{CPU: config.DefaultCPULimit, Memory: config.DefaultMemoryLimit}
9292

93-
return spec.Resources{ResourceRequest:defaultRequests, ResourceLimits:defaultLimits}
93+
return spec.Resources{ResourceRequest: defaultRequests, ResourceLimits: defaultLimits}
9494
}
9595

9696
func generateResourceRequirements(resources spec.Resources, defaultResources spec.Resources) (*v1.ResourceRequirements, error) {
@@ -366,7 +366,7 @@ func generateSidecarContainers(sidecars []spec.Sidecar,
366366
volumeMounts []v1.VolumeMount, defaultResources spec.Resources,
367367
superUserName string, credentialsSecretName string, logger *logrus.Entry) ([]v1.Container, error) {
368368

369-
if sidecars != nil && len(sidecars) > 0 {
369+
if len(sidecars) > 0 {
370370
result := make([]v1.Container, 0)
371371
for index, sidecar := range sidecars {
372372

@@ -699,7 +699,7 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu
699699
// generate sidecar containers
700700
if sidecarContainers, err = generateSidecarContainers(sideCars, volumeMounts, defaultResources,
701701
c.OpConfig.SuperUsername, c.credentialSecretName(c.OpConfig.SuperUsername), c.logger); err != nil {
702-
return nil, fmt.Errorf("could not generate sidecar containers: %v", err)
702+
return nil, fmt.Errorf("could not generate sidecar containers: %v", err)
703703
}
704704

705705
tolerationSpec := tolerations(&spec.Tolerations, c.OpConfig.PodToleration)
@@ -716,7 +716,7 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu
716716
int64(c.OpConfig.PodTerminateGracePeriod.Seconds()),
717717
c.OpConfig.PodServiceAccountName,
718718
c.OpConfig.KubeIAMRole,
719-
effectivePodPriorityClassName); err != nil{
719+
effectivePodPriorityClassName); err != nil {
720720
return nil, fmt.Errorf("could not generate pod template: %v", err)
721721
}
722722

@@ -726,7 +726,7 @@ func (c *Cluster) generateStatefulSet(spec *spec.PostgresSpec) (*v1beta1.Statefu
726726

727727
if volumeClaimTemplate, err = generatePersistentVolumeClaimTemplate(spec.Volume.Size,
728728
spec.Volume.StorageClass); err != nil {
729-
return nil, fmt.Errorf("could not generate volume claim template: %v", err)
729+
return nil, fmt.Errorf("could not generate volume claim template: %v", err)
730730
}
731731

732732
numberOfInstances := c.getNumberOfInstances(spec)
@@ -804,11 +804,11 @@ func (c *Cluster) mergeSidecars(sidecars []spec.Sidecar) []spec.Sidecar {
804804
return result
805805
}
806806

807-
func (c *Cluster) getNumberOfInstances(spec *spec.PostgresSpec) (newcur int32) {
807+
func (c *Cluster) getNumberOfInstances(spec *spec.PostgresSpec) int32 {
808808
min := c.OpConfig.MinInstances
809809
max := c.OpConfig.MaxInstances
810810
cur := spec.NumberOfInstances
811-
newcur = cur
811+
newcur := cur
812812

813813
if max >= 0 && newcur > max {
814814
newcur = max
@@ -820,7 +820,7 @@ func (c *Cluster) getNumberOfInstances(spec *spec.PostgresSpec) (newcur int32) {
820820
c.logger.Infof("adjusted number of instances from %d to %d (min: %d, max: %d)", cur, newcur, min, max)
821821
}
822822

823-
return
823+
return newcur
824824
}
825825

826826
func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string) (*v1.PersistentVolumeClaim, error) {
@@ -860,8 +860,8 @@ func generatePersistentVolumeClaimTemplate(volumeSize, volumeStorageClass string
860860
return volumeClaim, nil
861861
}
862862

863-
func (c *Cluster) generateUserSecrets() (secrets map[string]*v1.Secret) {
864-
secrets = make(map[string]*v1.Secret, len(c.pgUsers))
863+
func (c *Cluster) generateUserSecrets() map[string]*v1.Secret {
864+
secrets := make(map[string]*v1.Secret, len(c.pgUsers))
865865
namespace := c.Namespace
866866
for username, pgUser := range c.pgUsers {
867867
//Skip users with no password i.e. human users (they'll be authenticated using pam)
@@ -878,7 +878,7 @@ func (c *Cluster) generateUserSecrets() (secrets map[string]*v1.Secret) {
878878
}
879879
}
880880

881-
return
881+
return secrets
882882
}
883883

884884
func (c *Cluster) generateSingleUserSecret(namespace string, pgUser spec.PgUser) *v1.Secret {

‎pkg/cluster/pg.go

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -183,32 +183,30 @@ func (c *Cluster) getDatabases() (dbs map[string]string, err error) {
183183
dbs[datname] = owner
184184
}
185185

186-
return
186+
return dbs, err
187187
}
188188

189189
// executeCreateDatabase creates new database with the given owner.
190190
// The caller is responsible for openinging and closing the database connection.
191191
func (c *Cluster) executeCreateDatabase(datname, owner string) error {
192-
if !c.databaseNameOwnerValid(datname, owner) {
193-
return nil
194-
}
195-
c.logger.Infof("creating database %q with owner %q", datname, owner)
196-
197-
if _, err := c.pgDb.Exec(fmt.Sprintf(createDatabaseSQL, datname, owner)); err != nil {
198-
return fmt.Errorf("could not execute create database: %v", err)
199-
}
200-
return nil
192+
return c.execCreateOrAlterDatabase(datname, owner, createDatabaseSQL,
193+
"creating database", "create database")
201194
}
202195

203196
// executeCreateDatabase changes the owner of the given database.
204197
// The caller is responsible for openinging and closing the database connection.
205198
func (c *Cluster) executeAlterDatabaseOwner(datname string, owner string) error {
199+
return c.execCreateOrAlterDatabase(datname, owner, alterDatabaseOwnerSQL,
200+
"changing owner for database", "alter database owner")
201+
}
202+
203+
func (c *Cluster) execCreateOrAlterDatabase(datname, owner, statement, doing, operation string) error {
206204
if !c.databaseNameOwnerValid(datname, owner) {
207205
return nil
208206
}
209-
c.logger.Infof("changing database %q owner to %q", datname, owner)
210-
if _, err := c.pgDb.Exec(fmt.Sprintf(alterDatabaseOwnerSQL, datname, owner)); err != nil {
211-
return fmt.Errorf("could not execute alter database owner: %v", err)
207+
c.logger.Infof("%s %q owner %q", doing, datname, owner)
208+
if _, err := c.pgDb.Exec(fmt.Sprintf(statement, datname, owner)); err != nil {
209+
return fmt.Errorf("could not execute %s: %v", operation, err)
212210
}
213211
return nil
214212
}

‎pkg/cluster/pod.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ func (c *Cluster) masterCandidate(oldNodeName string) (*v1.Pod, error) {
183183
func (c *Cluster) MigrateMasterPod(podName spec.NamespacedName) error {
184184
var (
185185
masterCandidatePod *v1.Pod
186-
err error
187-
eol bool
186+
err error
187+
eol bool
188188
)
189189

190190
oldMaster, err := c.KubeClient.Pods(podName.Namespace).Get(podName.Name, metav1.GetOptions{})
@@ -212,7 +212,7 @@ func (c *Cluster) MigrateMasterPod(podName spec.NamespacedName) error {
212212
var sset *v1beta1.StatefulSet
213213
if sset, err = c.KubeClient.StatefulSets(c.Namespace).Get(c.statefulSetName(),
214214
metav1.GetOptions{}); err != nil {
215-
return fmt.Errorf("could not retrieve cluster statefulset: %v", err)
215+
return fmt.Errorf("could not retrieve cluster statefulset: %v", err)
216216
}
217217
c.Statefulset = sset
218218
}
@@ -225,7 +225,6 @@ func (c *Cluster) MigrateMasterPod(podName spec.NamespacedName) error {
225225
c.logger.Warningf("single master pod for cluster %q, migration will cause longer downtime of the master instance", c.clusterName())
226226
}
227227

228-
229228
// there are two cases for each postgres cluster that has its master pod on the node to migrate from:
230229
// - the cluster has some replicas - migrate one of those if necessary and failover to it
231230
// - there are no replicas - just terminate the master and wait until it respawns

‎pkg/cluster/resources.go

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -633,27 +633,3 @@ func (c *Cluster) GetStatefulSet() *v1beta1.StatefulSet {
633633
func (c *Cluster) GetPodDisruptionBudget() *policybeta1.PodDisruptionBudget {
634634
return c.PodDisruptionBudget
635635
}
636-
637-
func (c *Cluster) createDatabases() error {
638-
c.setProcessName("creating databases")
639-
640-
if len(c.Spec.Databases) == 0 {
641-
return nil
642-
}
643-
644-
if err := c.initDbConn(); err != nil {
645-
return fmt.Errorf("could not init database connection")
646-
}
647-
defer func() {
648-
if err := c.closeDbConn(); err != nil {
649-
c.logger.Errorf("could not close database connection: %v", err)
650-
}
651-
}()
652-
653-
for datname, owner := range c.Spec.Databases {
654-
if err := c.executeCreateDatabase(datname, owner); err != nil {
655-
return err
656-
}
657-
}
658-
return nil
659-
}

0 commit comments

Comments
 (0)