From b0cd1c22907cb83c5748d881687d1a93cb43dee6 Mon Sep 17 00:00:00 2001 From: ritho Date: Tue, 4 Jun 2019 01:25:43 -0500 Subject: [PATCH] Do some Kubectl optimizations suggested by the golangci linter The tool golangci-lint gives a bunch of warnings. This PR solves the easier/less controversial ones, so the code is simpler and a little bit more optimal, since it removes some if conditions. Kubernetes-commit: cd2adbe760641f844d84d411f9adcf17fb6982ff --- pkg/cmd/create/create_role_test.go | 2 +- pkg/cmd/delete/delete_test.go | 2 +- pkg/cmd/describe/describe_test.go | 6 +++--- pkg/cmd/plugin/plugin.go | 2 +- pkg/cmd/set/env/env_parse.go | 7 +++++-- pkg/cmd/util/sanity/cmd_sanity.go | 2 +- pkg/describe/versioned/describe.go | 17 ++++++++--------- pkg/drain/cordon.go | 6 ++---- pkg/util/i18n/i18n.go | 8 +++----- 9 files changed, 25 insertions(+), 27 deletions(-) diff --git a/pkg/cmd/create/create_role_test.go b/pkg/cmd/create/create_role_test.go index 7de49515..5136120f 100644 --- a/pkg/cmd/create/create_role_test.go +++ b/pkg/cmd/create/create_role_test.go @@ -140,7 +140,7 @@ func TestCreateRole(t *testing.T) { cmd.Run(cmd, []string{roleName}) actual := &rbac.Role{} if err := runtime.DecodeInto(scheme.Codecs.UniversalDecoder(), buf.Bytes(), actual); err != nil { - t.Log(string(buf.Bytes())) + t.Log(buf.String()) t.Fatal(err) } if !equality.Semantic.DeepEqual(test.expectedRole, actual) { diff --git a/pkg/cmd/delete/delete_test.go b/pkg/cmd/delete/delete_test.go index 9727245c..305e4e16 100644 --- a/pkg/cmd/delete/delete_test.go +++ b/pkg/cmd/delete/delete_test.go @@ -706,7 +706,7 @@ func TestResourceErrors(t *testing.T) { } if buf.Len() > 0 { - t.Errorf("buffer should be empty: %s", string(buf.Bytes())) + t.Errorf("buffer should be empty: %s", buf.String()) } }) } diff --git a/pkg/cmd/describe/describe_test.go b/pkg/cmd/describe/describe_test.go index 262913a6..7be073bc 100644 --- a/pkg/cmd/describe/describe_test.go +++ b/pkg/cmd/describe/describe_test.go @@ -59,7 +59,7 @@ func TestDescribeUnknownSchemaObject(t *testing.T) { t.Errorf("unexpected describer: %#v", d) } - if buf.String() != fmt.Sprintf("%s", d.Output) { + if buf.String() != d.Output { t.Errorf("unexpected output: %s", buf.String()) } } @@ -92,7 +92,7 @@ func TestDescribeUnknownNamespacedSchemaObject(t *testing.T) { t.Errorf("unexpected describer: %#v", d) } - if buf.String() != fmt.Sprintf("%s", d.Output) { + if buf.String() != d.Output { t.Errorf("unexpected output: %s", buf.String()) } } @@ -133,7 +133,7 @@ func TestDescribeObject(t *testing.T) { t.Errorf("unexpected describer: %#v", d) } - if buf.String() != fmt.Sprintf("%s", d.Output) { + if buf.String() != d.Output { t.Errorf("unexpected output: %s", buf.String()) } } diff --git a/pkg/cmd/plugin/plugin.go b/pkg/cmd/plugin/plugin.go index 486e0e39..d578ca08 100644 --- a/pkg/cmd/plugin/plugin.go +++ b/pkg/cmd/plugin/plugin.go @@ -99,7 +99,7 @@ func NewCmdPluginList(f cmdutil.Factory, streams genericclioptions.IOStreams) *c func (o *PluginListOptions) Complete(cmd *cobra.Command) error { o.Verifier = &CommandOverrideVerifier{ root: cmd.Root(), - seenPlugins: make(map[string]string, 0), + seenPlugins: make(map[string]string), } o.PluginPaths = filepath.SplitList(os.Getenv("PATH")) diff --git a/pkg/cmd/set/env/env_parse.go b/pkg/cmd/set/env/env_parse.go index 5e2e5290..d5b3ec70 100644 --- a/pkg/cmd/set/env/env_parse.go +++ b/pkg/cmd/set/env/env_parse.go @@ -81,7 +81,7 @@ func parseIntoEnvVar(spec []string, defaultReader io.Reader, envVarType string) return nil, nil, err } env = append(env, fileEnv...) - case strings.Index(envSpec, "=") != -1: + case strings.Contains(envSpec, "="): parts := strings.SplitN(envSpec, "=", 2) if len(parts) != 2 { return nil, nil, fmt.Errorf("invalid %s: %v", envVarType, envSpec) @@ -119,7 +119,8 @@ func readEnv(r io.Reader, envVarType string) ([]v1.EnvVar, error) { if pos := strings.Index(envSpec, "#"); pos != -1 { envSpec = envSpec[:pos] } - if strings.Index(envSpec, "=") != -1 { + + if strings.Contains(envSpec, "=") { parts := strings.SplitN(envSpec, "=", 2) if len(parts) != 2 { return nil, fmt.Errorf("invalid %s: %v", envVarType, envSpec) @@ -130,8 +131,10 @@ func readEnv(r io.Reader, envVarType string) ([]v1.EnvVar, error) { }) } } + if err := scanner.Err(); err != nil && err != io.EOF { return nil, err } + return env, nil } diff --git a/pkg/cmd/util/sanity/cmd_sanity.go b/pkg/cmd/util/sanity/cmd_sanity.go index df830587..c60bdc11 100644 --- a/pkg/cmd/util/sanity/cmd_sanity.go +++ b/pkg/cmd/util/sanity/cmd_sanity.go @@ -80,7 +80,7 @@ func RunCmdChecks(cmd *cobra.Command, cmdChecks []CmdCheck, skipCmd []string) [] fmt.Fprintf(os.Stdout, "---+ RUNNING COMMAND CHECKS on %q\n", cmdPath) for _, check := range cmdChecks { - if err := check(cmd); err != nil && len(err) > 0 { + if err := check(cmd); len(err) > 0 { errors = append(errors, err...) } } diff --git a/pkg/describe/versioned/describe.go b/pkg/describe/versioned/describe.go index c6f0e694..f9fae1df 100644 --- a/pkg/describe/versioned/describe.go +++ b/pkg/describe/versioned/describe.go @@ -777,10 +777,11 @@ func describePodIPs(pod *corev1.Pod, w PrefixWriter, space string) { } func describeVolumes(volumes []corev1.Volume, w PrefixWriter, space string) { - if volumes == nil || len(volumes) == 0 { + if len(volumes) == 0 { w.Write(LEVEL_0, "%sVolumes:\t\n", space) return } + w.Write(LEVEL_0, "%sVolumes:\n", space) for _, volume := range volumes { nameIndent := "" @@ -2426,7 +2427,6 @@ func describeIngressTLS(w PrefixWriter, ingTLS []networkingv1beta1.IngressTLS) { w.Write(LEVEL_1, "%v terminates %v\n", t.SecretName, strings.Join(t.Hosts, ",")) } } - return } // TODO: Move from annotations into Ingress status. @@ -2435,7 +2435,6 @@ func describeIngressAnnotations(w PrefixWriter, annotations map[string]string) { for k, v := range annotations { w.Write(LEVEL_1, "%v:\t%s\n", k, v) } - return } // ServiceDescriber generates information about a service. @@ -2742,8 +2741,8 @@ func (d *ServiceAccountDescriber) Describe(namespace, name string, describerSett for _, s := range secrets.Items { if s.Type == corev1.SecretTypeServiceAccountToken { - name, _ := s.Annotations[corev1.ServiceAccountNameKey] - uid, _ := s.Annotations[corev1.ServiceAccountUIDKey] + name := s.Annotations[corev1.ServiceAccountNameKey] + uid := s.Annotations[corev1.ServiceAccountUIDKey] if name == serviceAccount.Name && uid == string(serviceAccount.UID) { tokens = append(tokens, s) } @@ -4324,7 +4323,7 @@ func printLabelsMultiline(w PrefixWriter, title string, labels map[string]string func printLabelsMultilineWithIndent(w PrefixWriter, initialIndent, title, innerIndent string, labels map[string]string, skip sets.String) { w.Write(LEVEL_0, "%s%s:%s", initialIndent, title, innerIndent) - if labels == nil || len(labels) == 0 { + if len(labels) == 0 { w.WriteLine("") return } @@ -4362,7 +4361,7 @@ func printNodeTaintsMultiline(w PrefixWriter, title string, taints []corev1.Tain func printTaintsMultilineWithIndent(w PrefixWriter, initialIndent, title, innerIndent string, taints []corev1.Taint) { w.Write(LEVEL_0, "%s%s:%s", initialIndent, title, innerIndent) - if taints == nil || len(taints) == 0 { + if len(taints) == 0 { w.WriteLine("") return } @@ -4393,7 +4392,7 @@ func printPodsMultiline(w PrefixWriter, title string, pods []corev1.Pod) { func printPodsMultilineWithIndent(w PrefixWriter, initialIndent, title, innerIndent string, pods []corev1.Pod) { w.Write(LEVEL_0, "%s%s:%s", initialIndent, title, innerIndent) - if pods == nil || len(pods) == 0 { + if len(pods) == 0 { w.WriteLine("") return } @@ -4424,7 +4423,7 @@ func printPodTolerationsMultiline(w PrefixWriter, title string, tolerations []co func printTolerationsMultilineWithIndent(w PrefixWriter, initialIndent, title, innerIndent string, tolerations []corev1.Toleration) { w.Write(LEVEL_0, "%s%s:%s", initialIndent, title, innerIndent) - if tolerations == nil || len(tolerations) == 0 { + if len(tolerations) == 0 { w.WriteLine("") return } diff --git a/pkg/drain/cordon.go b/pkg/drain/cordon.go index fc339752..8f0f56d2 100644 --- a/pkg/drain/cordon.go +++ b/pkg/drain/cordon.go @@ -62,10 +62,8 @@ func NewCordonHelperFromRuntimeObject(nodeObject runtime.Object, scheme *runtime // or false when no change is needed func (c *CordonHelper) UpdateIfRequired(desired bool) bool { c.desired = desired - if c.node.Spec.Unschedulable == c.desired { - return false - } - return true + + return c.node.Spec.Unschedulable != c.desired } // PatchOrReplace uses given clientset to update the node status, either by patching or diff --git a/pkg/util/i18n/i18n.go b/pkg/util/i18n/i18n.go index 2e2bc2ff..31e830c6 100644 --- a/pkg/util/i18n/i18n.go +++ b/pkg/util/i18n/i18n.go @@ -76,11 +76,9 @@ func findLanguage(root string, getLanguageFn func() string) string { langStr := getLanguageFn() translations := knownTranslations[root] - if translations != nil { - for ix := range translations { - if translations[ix] == langStr { - return langStr - } + for ix := range translations { + if translations[ix] == langStr { + return langStr } } klog.V(3).Infof("Couldn't find translations for %s, using default", langStr)