stylecheck added to linting and issues resolved (#1639)

Signed-off-by: Ken Sipe <kensipe@gmail.com>

.golangci.yml

@@ -15,6 +15,7 @@ linters:
     - dupl
     - goconst
     - lll
+    - stylecheck
 run:
   build-tags:
     - integration

pkg/controller/instance/instance_controller.go

@@ -45,7 +45,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
-	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
 	kudov1beta1 "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
 	"github.com/kudobuilder/kudo/pkg/engine"
 	"github.com/kudobuilder/kudo/pkg/engine/renderer"
@@ -201,7 +200,7 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
 		return reconcile.Result{}, err
 	}
 
-	if planStatus.Status == v1beta1.ExecutionPending {
+	if planStatus.Status == kudov1beta1.ExecutionPending {
 		log.Printf("InstanceController: Going to start execution of plan '%s' on instance %s/%s", plan, instance.Namespace, instance.Name)
 		r.Recorder.Event(instance, "Normal", "PlanStarted", fmt.Sprintf("Execution of plan %s started", plan))
 	}
@@ -420,8 +419,8 @@ func ParamsMap(instance *kudov1beta1.Instance, operatorVersion *kudov1beta1.Oper
 }
 
 // PipesMap generates {{ Pipes.* }} map of keys and values which is later used during template rendering.
-func PipesMap(planName string, plan *v1beta1.Plan, tasks []v1beta1.Task, emeta *engine.Metadata) (map[string]string, error) {
-	taskByName := func(name string) (*v1beta1.Task, bool) {
+func PipesMap(planName string, plan *kudov1beta1.Plan, tasks []kudov1beta1.Task, emeta *engine.Metadata) (map[string]string, error) {
+	taskByName := func(name string) (*kudov1beta1.Task, bool) {
 		for _, t := range tasks {
 			if t.Name == name {
 				return &t, true
@@ -462,7 +461,7 @@ func PipesMap(planName string, plan *v1beta1.Plan, tasks []v1beta1.Task, emeta *
 // scheduled plan (UID has changed) and returns updated plan status. In this case Plan/phase/step statuses are set
 // to ExecutionPending meaning that the controller will restart plan execution. Otherwise (the plan is old),
 // nothing is changed and the existing plan status is returned.
-func resetPlanStatusIfPlanIsNew(i *v1beta1.Instance, plan string, uid types.UID) (*v1beta1.PlanStatus, error) {
+func resetPlanStatusIfPlanIsNew(i *kudov1beta1.Instance, plan string, uid types.UID) (*kudov1beta1.PlanStatus, error) {
 	ps := i.PlanStatus(plan)
 	if ps == nil {
 		return nil, fmt.Errorf("failed to find planStatus for plan '%s'", plan)
@@ -481,28 +480,28 @@ func resetPlanStatusIfPlanIsNew(i *v1beta1.Instance, plan string, uid types.UID)
 // ensurePlanStatusInitialized initializes plan status for all plans this instance supports  it does not trigger run
 // of any plan it either initializes everything for a fresh instance without any status or tries to adjust status
 // after OV was updated
-func ensurePlanStatusInitialized(i *v1beta1.Instance, ov *v1beta1.OperatorVersion) {
+func ensurePlanStatusInitialized(i *kudov1beta1.Instance, ov *kudov1beta1.OperatorVersion) {
 	if i.Status.PlanStatus == nil {
-		i.Status.PlanStatus = make(map[string]v1beta1.PlanStatus)
+		i.Status.PlanStatus = make(map[string]kudov1beta1.PlanStatus)
 	}
 
 	for planName, plan := range ov.Spec.Plans {
 		if _, ok := i.Status.PlanStatus[planName]; !ok {
-			planStatus := v1beta1.PlanStatus{
+			planStatus := kudov1beta1.PlanStatus{
 				Name:   planName,
-				Status: v1beta1.ExecutionNeverRun,
-				Phases: make([]v1beta1.PhaseStatus, 0),
+				Status: kudov1beta1.ExecutionNeverRun,
+				Phases: make([]kudov1beta1.PhaseStatus, 0),
 			}
 			for _, phase := range plan.Phases {
-				phaseStatus := v1beta1.PhaseStatus{
+				phaseStatus := kudov1beta1.PhaseStatus{
 					Name:   phase.Name,
-					Status: v1beta1.ExecutionNeverRun,
-					Steps:  make([]v1beta1.StepStatus, 0),
+					Status: kudov1beta1.ExecutionNeverRun,
+					Steps:  make([]kudov1beta1.StepStatus, 0),
 				}
 				for _, step := range phase.Steps {
-					stepStatus := v1beta1.StepStatus{
+					stepStatus := kudov1beta1.StepStatus{
 						Name:   step.Name,
-						Status: v1beta1.ExecutionNeverRun,
+						Status: kudov1beta1.ExecutionNeverRun,
 					}
 					phaseStatus.Steps = append(phaseStatus.Steps, stepStatus)
 				}
@@ -515,7 +514,7 @@ func ensurePlanStatusInitialized(i *v1beta1.Instance, ov *v1beta1.OperatorVersio
 
 // scheduledPlan method returns currently scheduled plan and its UID from Instance.Spec.PlanExecution field. However, due
 // to an edge case with instance deletion, this method also schedules the 'cleanup' plan if necessary (see the comments below)
-func scheduledPlan(i *v1beta1.Instance, ov *v1beta1.OperatorVersion) (string, types.UID) {
+func scheduledPlan(i *kudov1beta1.Instance, ov *kudov1beta1.OperatorVersion) (string, types.UID) {
 	// Instance deletion is an edge case where the admission webhook *can not* populate the Spec.PlanExecution.PlanName
 	// with the 'cleanup' plan. So we have to do it here ourselves. Only if:
 	// 1. Instance is being deleted
@@ -524,17 +523,17 @@ func scheduledPlan(i *v1beta1.Instance, ov *v1beta1.OperatorVersion) (string, ty
 	// we set the Spec.PlanExecution.PlanName = 'cleanup'
 	hasToScheduleCleanupAfterDeletion := func() bool {
 		shouldCleanup := i.IsDeleting() && kudov1beta1.CleanupPlanExists(ov)
-		cleanupNeverRun := i.PlanStatus(v1beta1.CleanupPlanName) == nil || i.PlanStatus(v1beta1.CleanupPlanName).Status == kudov1beta1.ExecutionNeverRun
-		cleanupNotScheduled := i.Spec.PlanExecution.PlanName != v1beta1.CleanupPlanName
+		cleanupNeverRun := i.PlanStatus(kudov1beta1.CleanupPlanName) == nil || i.PlanStatus(kudov1beta1.CleanupPlanName).Status == kudov1beta1.ExecutionNeverRun
+		cleanupNotScheduled := i.Spec.PlanExecution.PlanName != kudov1beta1.CleanupPlanName
 
 		return shouldCleanup && cleanupNeverRun && cleanupNotScheduled
 	}
 	if hasToScheduleCleanupAfterDeletion() {
-		log.Printf("InstanceController: Instance %s/%s is being deleted. Scheduling '%s' plan.", i.Namespace, i.Name, v1beta1.CleanupPlanName)
+		log.Printf("InstanceController: Instance %s/%s is being deleted. Scheduling '%s' plan.", i.Namespace, i.Name, kudov1beta1.CleanupPlanName)
 
-		i.Spec.PlanExecution.PlanName = v1beta1.CleanupPlanName
+		i.Spec.PlanExecution.PlanName = kudov1beta1.CleanupPlanName
 		i.Spec.PlanExecution.UID = uuid.NewUUID()
-		i.Spec.PlanExecution.Status = v1beta1.ExecutionNeverRun
+		i.Spec.PlanExecution.Status = kudov1beta1.ExecutionNeverRun
 	}
 
 	return i.Spec.PlanExecution.PlanName, i.Spec.PlanExecution.UID

pkg/engine/workflow/engine_test.go

@@ -6,7 +6,6 @@ import (
 	"time"
 
 	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/discovery/cached/memory"
@@ -48,15 +47,15 @@ func TestExecutePlan(t *testing.T) {
 			},
 		},
 			metadata:       meta,
-			expectedStatus: &v1beta1.PlanStatus{Status: v1beta1.ExecutionComplete, LastUpdatedTimestamp: &metav1.Time{Time: testTime}},
+			expectedStatus: &v1beta1.PlanStatus{Status: v1beta1.ExecutionComplete, LastUpdatedTimestamp: &v1.Time{Time: testTime}},
 			enhancer:       testEnhancer,
 		},
 		{name: "plan with a step to be executed is in progress when the step is not completed", activePlan: &ActivePlan{
 			Name: "test",
 			PlanStatus: &v1beta1.PlanStatus{
 				Name:                 "test",
 				Status:               v1beta1.ExecutionInProgress,
-				LastUpdatedTimestamp: &metav1.Time{Time: testTime},
+				LastUpdatedTimestamp: &v1.Time{Time: testTime},
 				Phases:               []v1beta1.PhaseStatus{{Name: "phase", Status: v1beta1.ExecutionInProgress, Steps: []v1beta1.StepStatus{{Status: v1beta1.ExecutionInProgress, Name: "step"}}}},
 			},
 			Spec: &v1beta1.Plan{
@@ -80,7 +79,7 @@ func TestExecutePlan(t *testing.T) {
 			expectedStatus: &v1beta1.PlanStatus{
 				Name:                 "test",
 				Status:               v1beta1.ExecutionInProgress,
-				LastUpdatedTimestamp: &metav1.Time{Time: testTime},
+				LastUpdatedTimestamp: &v1.Time{Time: testTime},
 				Phases:               []v1beta1.PhaseStatus{{Name: "phase", Status: v1beta1.ExecutionInProgress, Steps: []v1beta1.StepStatus{{Status: v1beta1.ExecutionInProgress, Name: "step"}}}}},
 			enhancer: testEnhancer,
 		},
@@ -89,7 +88,7 @@ func TestExecutePlan(t *testing.T) {
 			PlanStatus: &v1beta1.PlanStatus{
 				Name:                 "test",
 				Status:               v1beta1.ExecutionPending,
-				LastUpdatedTimestamp: &metav1.Time{Time: testTime},
+				LastUpdatedTimestamp: &v1.Time{Time: testTime},
 				Phases:               []v1beta1.PhaseStatus{{Name: "phase", Status: v1beta1.ExecutionPending, Steps: []v1beta1.StepStatus{{Status: v1beta1.ExecutionPending, Name: "step"}}}},
 			},
 			Spec: &v1beta1.Plan{
@@ -122,7 +121,7 @@ func TestExecutePlan(t *testing.T) {
 			Name: "test",
 			PlanStatus: &v1beta1.PlanStatus{
 				Name:                 "test",
-				LastUpdatedTimestamp: &metav1.Time{Time: testTime},
+				LastUpdatedTimestamp: &v1.Time{Time: testTime},
 				Phases:               []v1beta1.PhaseStatus{{Name: "phase", Status: v1beta1.ExecutionInProgress, Steps: []v1beta1.StepStatus{{Status: v1beta1.ErrorStatus, Name: "step"}}}},
 			},
 			Spec: &v1beta1.Plan{
@@ -478,7 +477,7 @@ func TestExecutePlan(t *testing.T) {
 			expectedStatus: &v1beta1.PlanStatus{
 				Name:                 "test",
 				Status:               v1beta1.ExecutionInProgress,
-				LastUpdatedTimestamp: &metav1.Time{Time: testTime},
+				LastUpdatedTimestamp: &v1.Time{Time: testTime},
 				Phases: []v1beta1.PhaseStatus{
 					{Name: "phaseOne", Status: v1beta1.ExecutionInProgress, Steps: []v1beta1.StepStatus{{Name: "step", Status: v1beta1.ErrorStatus, Message: "A transient error when executing task test.phaseOne.step.taskOne. Will retry. dummy error"}}},
 					{Name: "phaseTwo", Status: v1beta1.ExecutionInProgress, Steps: []v1beta1.StepStatus{{Name: "step", Status: v1beta1.ExecutionInProgress}}},
@@ -626,7 +625,7 @@ func TestExecutePlan(t *testing.T) {
 	fakeCachedDiscovery := memory.NewMemCacheClient(fakeDiscovery)
 	for _, tt := range tests {
 		newStatus, err := Execute(tt.activePlan, tt.metadata, testClient, fakeCachedDiscovery, nil, testScheme)
-		newStatus.LastUpdatedTimestamp = &metav1.Time{Time: testTime}
+		newStatus.LastUpdatedTimestamp = &v1.Time{Time: testTime}
 
 		if !tt.wantErr && err != nil {
 			t.Errorf("%s: Expecting no error but got one: %v", tt.name, err)
@@ -644,11 +643,11 @@ func TestExecutePlan(t *testing.T) {
 
 func instance() *v1beta1.Instance {
 	return &v1beta1.Instance{
-		TypeMeta: metav1.TypeMeta{
+		TypeMeta: v1.TypeMeta{
 			APIVersion: "kudo.dev/v1beta1",
 			Kind:       "Instance",
 		},
-		ObjectMeta: metav1.ObjectMeta{
+		ObjectMeta: v1.ObjectMeta{
 			Name:      "test-instance",
 			Namespace: "default",
 		},

pkg/kudoctl/cmd/init.go

@@ -169,7 +169,7 @@ func (initCmd *initCmd) run() error {
 			"IfNotPresent":
 			opts.ImagePullPolicy = initCmd.imagePullPolicy
 		default:
-			return fmt.Errorf("Unknown image pull policy %s, must be one of 'Always', 'IfNotPresent' or 'Never'", initCmd.imagePullPolicy)
+			return fmt.Errorf("unknown image pull policy %s, must be one of 'Always', 'IfNotPresent' or 'Never'", initCmd.imagePullPolicy)
 		}
 	}
 

pkg/kudoctl/cmd/plan/plan_status.go

@@ -55,7 +55,7 @@ func status(kc *kudo.Client, options *Options, ns string) error {
 			return err
 		}
 		if instance == nil {
-			return fmt.Errorf("Instance %s/%s does not exist", ns, options.Instance)
+			return fmt.Errorf("instance %s/%s does not exist", ns, options.Instance)
 		}
 
 		ov, err := kc.GetOperatorVersion(instance.Spec.OperatorVersion.Name, ns)

pkg/kudoctl/cmd/plan/plan_status_test.go

@@ -111,7 +111,7 @@ func TestStatus(t *testing.T) {
 		output          string
 		goldenFile      string
 	}{
-		{name: "nonexisting instance", instanceNameArg: "nonexisting", errorMessage: "Instance default/nonexisting does not exist"},
+		{name: "nonexisting instance", instanceNameArg: "nonexisting", errorMessage: "instance default/nonexisting does not exist"},
 		{name: "nonexisting ov", instance: instance, instanceNameArg: "test", errorMessage: "OperatorVersion test-1.0 from instance default/test does not exist"},
 		{name: "no plan run", instance: instance, ov: ov, instanceNameArg: "test", expectedOutput: "No plan ever run for instance - nothing to show for instance test\n"},
 		{name: "text output", instance: fatalErrInstance, ov: ov, instanceNameArg: "test", goldenFile: "planstatus.txt"},

pkg/kudoctl/cmd/prompt/operator.go

@@ -20,7 +20,7 @@ func ForOperator(fs afero.Fs, pathDefault string, overwrite bool, operatorDefaul
 
 	nameValid := func(input string) error {
 		if len(input) < 3 {
-			return errors.New("Operator name must have more than 3 characters")
+			return errors.New("operator name must have more than 3 characters")
 		}
 		return nil
 	}
@@ -32,7 +32,7 @@ func ForOperator(fs afero.Fs, pathDefault string, overwrite bool, operatorDefaul
 
 	pathValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Operator directory must have more than 1 character")
+			return errors.New("operator directory must have more than 1 character")
 		}
 		return generate.CanGenerateOperator(fs, input, overwrite)
 	}
@@ -44,7 +44,7 @@ func ForOperator(fs afero.Fs, pathDefault string, overwrite bool, operatorDefaul
 
 	versionValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Operator version is required in semver format")
+			return errors.New("operator version is required in semver format")
 		}
 		_, err := semver.NewVersion(input)
 		return err
@@ -98,7 +98,7 @@ func ForMaintainer() (*v1beta1.Maintainer, error) {
 
 	nameValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Maintainer name must be > than 1 character")
+			return errors.New("maintainer name must be > than 1 character")
 		}
 		return nil
 	}
@@ -132,10 +132,10 @@ func validEmail(email string) bool {
 func ForParameter(planNames []string, paramNameList []string) (*packages.Parameter, error) {
 	nameValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Parameter name must be > than 1 character")
+			return errors.New("parameter name must be > than 1 character")
 		}
 		if inArray(input, paramNameList) {
-			return errors.New("Parameter name must be unique")
+			return errors.New("parameter name must be unique")
 		}
 		return nil
 	}
@@ -204,10 +204,10 @@ func ForParameter(planNames []string, paramNameList []string) (*packages.Paramet
 func ForTaskName(existingTasks []v1beta1.Task) (string, error) {
 	nameValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Task name must be > than 1 character")
+			return errors.New("task name must be > than 1 character")
 		}
 		if taskExists(input, existingTasks) {
-			return errors.New("Task name must be unique")
+			return errors.New("task name must be unique")
 		}
 		return nil
 	}
@@ -319,10 +319,10 @@ func ForPlan(planNames []string, tasks []v1beta1.Task, fs afero.Fs, path string,
 
 	nameValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Plan name must be > than 1 character")
+			return errors.New("plan name must be > than 1 character")
 		}
 		if inArray(input, planNames) {
-			return errors.New("Plan name must be unique")
+			return errors.New("plan name must be unique")
 		}
 		return nil
 	}
@@ -449,10 +449,10 @@ func subtract(allTasksNames []string, currentStepTaskNames []string) (result []s
 func forStep(stepNames []string, stepIndex int, defaultStepName string) (*v1beta1.Step, error) {
 	stepNameValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Step name must be > than 1 character")
+			return errors.New("step name must be > than 1 character")
 		}
 		if inArray(input, stepNames) {
-			return errors.New("Step name must be unique in a Phase")
+			return errors.New("step name must be unique in a Phase")
 		}
 		return nil
 	}
@@ -467,10 +467,10 @@ func forStep(stepNames []string, stepIndex int, defaultStepName string) (*v1beta
 func forPhase(phaseNames []string, index int, defaultPhaseName string) (*v1beta1.Phase, error) {
 	pnameValid := func(input string) error {
 		if len(input) < 1 {
-			return errors.New("Phase name must be > than 1 character")
+			return errors.New("phase name must be > than 1 character")
 		}
 		if inArray(input, phaseNames) {
-			return errors.New("Phase name must be unique in plan")
+			return errors.New("phase name must be unique in plan")
 		}
 		return nil
 	}

pkg/kudoctl/cmd/uninstall.go

@@ -61,7 +61,7 @@ func newUninstallCmd() *cobra.Command {
 		Example: uninstallExample,
 		Args: func(cmd *cobra.Command, args []string) error {
 			if len(args) != 0 {
-				return fmt.Errorf("The command expects no arguments and --instance flag must be provided.\n %s", cmd.UsageString())
+				return fmt.Errorf("the command expects no arguments and --instance flag must be provided.\n %s", cmd.UsageString())
 			}
 			return nil
 		},

pkg/kudoctl/kudoinit/crd/crds.go

@@ -9,7 +9,6 @@ import (
 	apiextv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	"k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/typed/apiextensions/v1beta1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"sigs.k8s.io/yaml"
@@ -95,7 +94,7 @@ func (c Initializer) Install(client *kube.Client) error {
 }
 
 func (c Initializer) verifyIsNotInstalled(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error {
-	_, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, metav1.GetOptions{})
+	_, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, v1.GetOptions{})
 	if err != nil {
 		if kerrors.IsNotFound(err) {
 			return nil
@@ -107,7 +106,7 @@ func (c Initializer) verifyIsNotInstalled(client v1beta1.CustomResourceDefinitio
 }
 
 func (c Initializer) verifyInstallation(client v1beta1.CustomResourceDefinitionsGetter, crd *apiextv1beta1.CustomResourceDefinition, result *verifier.Result) error {
-	existingCrd, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, metav1.GetOptions{})
+	existingCrd, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, v1.GetOptions{})
 	if err != nil {
 		if os.IsTimeout(err) {
 			return err
@@ -133,14 +132,14 @@ func (c Initializer) apply(client v1beta1.CustomResourceDefinitionsGetter, crd *
 		// all installed custom resources. We must have a correct update!
 		clog.V(4).Printf("crd %v already exists, try to update", crd.Name)
 
-		oldCrd, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, metav1.GetOptions{})
+		oldCrd, err := client.CustomResourceDefinitions().Get(context.TODO(), crd.Name, v1.GetOptions{})
 		if err != nil {
 			return fmt.Errorf("failed to get crd for update %s: %v", crd.Name, err)
 		}
 
 		// As we call update, we need to take over the resourceVersion
 		crd.ResourceVersion = oldCrd.ResourceVersion
-		_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, metav1.UpdateOptions{})
+		_, err = client.CustomResourceDefinitions().Update(context.TODO(), crd, v1.UpdateOptions{})
 		if err != nil {
 			return fmt.Errorf("failed to update crd %s: %v", crd.Name, err)
 		}

pkg/kudoctl/kudoinit/manager/manager.go

@@ -6,7 +6,6 @@ import (
 
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -161,7 +160,7 @@ func generateDeployment(opts kudoinit.Options) *appsv1.StatefulSet {
 
 	secretDefaultMode := int32(420)
 	image := opts.Image
-	imagePullPolicy := v1.PullPolicy(opts.ImagePullPolicy)
+	imagePullPolicy := corev1.PullPolicy(opts.ImagePullPolicy)
 	s := &appsv1.StatefulSet{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "StatefulSet",