Pull request template for Flyteplugins (#66)

* Pull request template for Flyteplugins
* Linter fixes

flyteplugins/.golangci.yml

@@ -0,0 +1,30 @@
+# WARNING: THIS FILE IS MANAGED IN THE 'BOILERPLATE' REPO AND COPIED TO OTHER REPOSITORIES.
+# ONLY EDIT THIS FILE FROM WITHIN THE 'LYFT/BOILERPLATE' REPOSITORY:
+# 
+# TO OPT OUT OF UPDATES, SEE https://github.com/lyft/boilerplate/blob/master/Readme.rst
+
+run:
+  skip-dirs:
+    - pkg/client
+
+linters:
+  disable-all: true
+  enable:
+    - deadcode
+    - errcheck
+    - gas
+    - goconst
+    - goimports
+    - golint
+    - gosimple
+    - govet
+    - ineffassign
+    - misspell
+    - nakedret
+    - staticcheck
+    - structcheck
+    - typecheck
+    - unconvert
+    - unparam
+    - unused
+    - varcheck

flyteplugins/boilerplate/lyft/golang_support_tools/go.sum

@@ -39,6 +39,7 @@ github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc/go.mod h1:LOuy
 github.com/alecthomas/template v0.0.0-20190718012654-fb15b899a751/go.mod h1:LOuyumcjzFXgccqObfd/Ljyb9UuFJ6TxHnclSeseNhc=
 github.com/alecthomas/units v0.0.0-20151022065526-2efee857e7cf/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
 github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRFXyAe+OPACYpWeL0wqObRcbAqCMya13uyzqw0=
+github.com/alvaroloes/enumer v1.1.2 h1:5khqHB33TZy1GWCO/lZwcroBFh7u+0j40T83VUbfAMY=
 github.com/alvaroloes/enumer v1.1.2/go.mod h1:FxrjvuXoDAx9isTJrv4c+T410zFi0DtXIT0m65DJ+Wo=
 github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
 github.com/aws/aws-sdk-go v1.25.16 h1:k7Fy6T/uNuLX6zuayU/TJoP7yMgGcJSkZpF7QVjwYpA=
@@ -263,6 +264,7 @@ github.com/onsi/ginkgo v1.10.1 h1:q/mM8GF/n0shIN8SaAZ0V+jnLPzen6WIVZdiwrRlMlo=
 github.com/onsi/ginkgo v1.10.1/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
 github.com/onsi/gomega v1.7.0 h1:XPnZz8VVBHjVsy1vzJmRwIcSwiUO+JFfrv/xGiigmME=
 github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY=
+github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1 h1:/I3lTljEEDNYLho3/FUB7iD/oc2cEFgVmbHzV+O0PtU=
 github.com/pascaldekloe/name v0.0.0-20180628100202-0fd16699aae1/go.mod h1:eD5JxqMiuNYyFNmyY9rkJ/slN8y59oEu4Ei7F8OoKWQ=
 github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k=
 github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic=

flyteplugins/boilerplate/lyft/golang_test_targets/Makefile

@@ -26,7 +26,8 @@ mod_download: #download dependencies (including test deps) for the package
 install: download_tooling mod_download
 
 .PHONY: show
-show: go list -m all
+show:
+	go list -m all
 
 .PHONY: test_unit
 test_unit:

flyteplugins/boilerplate/lyft/pull_request_template/Readme.rst

@@ -0,0 +1,8 @@
+Pull Request Template
+~~~~~~~~~~~~~~~~~~~~~
+
+Provides a Pull Request template.
+
+**To Enable:**
+
+Add ``lyft/golang_test_targets`` to your ``boilerplate/update.cfg`` file.

flyteplugins/boilerplate/lyft/pull_request_template/pull_request_template.md

@@ -0,0 +1,26 @@
+# TL;DR
+_Please replace this text with a description of what this PR accomplishes._
+
+## Type
+ - [ ] Bug Fix
+ - [ ] Feature
+ - [ ] Plugin
+
+## Are all requirements met?
+
+ - [ ] Code completed
+ - [ ] Smoke tested
+ - [ ] Unit tests added
+ - [ ] Code documentation added
+ - [ ] Any pending items have an associated Issue
+
+## Complete description
+ _How did you fix the bug, make the feature etc. Link to any design docs etc_
+
+## Tracking Issue
+https://github.com/lyft/flyte/issues/<number>
+
+## Follow-up issue
+_NA_
+OR
+_https://github.com/lyft/flyte/issues/<number>_

flyteplugins/boilerplate/lyft/pull_request_template/update.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+
+# WARNING: THIS FILE IS MANAGED IN THE 'BOILERPLATE' REPO AND COPIED TO OTHER REPOSITORIES.
+# ONLY EDIT THIS FILE FROM WITHIN THE 'LYFT/BOILERPLATE' REPOSITORY:
+# 
+# TO OPT OUT OF UPDATES, SEE https://github.com/lyft/boilerplate/blob/master/Readme.rst
+
+set -e
+
+DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
+
+cp ${DIR}/pull_request_template.md ${DIR}/../../../pull_request_template.md

flyteplugins/boilerplate/update.cfg

@@ -1,3 +1,4 @@
 lyft/golang_test_targets
 lyft/golangci_file
 lyft/golang_support_tools
+lyft/pull_request_template

flyteplugins/go/tasks/aws/config.go

@@ -29,7 +29,7 @@ var (
 // Config section for AWS Package
 type Config struct {
 	Region               string          `json:"region" pflag:",AWS Region to connect to."`
-	AccountID            string          `json:"accountId" pflag:",AWS Account Id."`
+	AccountID            string          `json:"accountId" pflag:",AWS Account Identifier."`
 	Retries              int             `json:"retries" pflag:",Number of retries."`
 	MaxErrorStringLength int             `json:"maxErrorLength" pflag:",Maximum size of error messages."`
 	CatalogCacheTimeout  config.Duration `json:"catalog-timeout" pflag:"\"5s\",Timeout duration for checking catalog for all batch tasks"`

flyteplugins/go/tasks/config_load_test.go

@@ -67,7 +67,7 @@ func TestLoadConfig(t *testing.T) {
 
 		assert.Equal(t, []v1.Toleration{tolGPU}, k8sConfig.ResourceTolerations[v1.ResourceName("nvidia.com/gpu")])
 		assert.Equal(t, []v1.Toleration{tolStorage}, k8sConfig.ResourceTolerations[v1.ResourceStorage])
-		assert.Equal(t, "1000m", k8sConfig.DefaultCpuRequest)
+		assert.Equal(t, "1000m", k8sConfig.DefaultCPURequest)
 		assert.Equal(t, "1024Mi", k8sConfig.DefaultMemoryRequest)
 	})
 

flyteplugins/go/tasks/logs/logging_utils.go

@@ -6,7 +6,7 @@ import (
 	logUtils "github.com/lyft/flyteidl/clients/go/coreutils/logs"
 	"github.com/lyft/flyteidl/gen/pb-go/flyteidl/core"
 	"github.com/lyft/flytestdlib/logger"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 )
 
 func GetLogsForContainerInPod(ctx context.Context, pod *v1.Pod, index uint32, nameSuffix string) ([]*core.TaskLog, error) {

flyteplugins/go/tasks/logs/logging_utils_test.go

@@ -73,8 +73,7 @@ func TestGetLogsForContainerInPod_MissingStatus(t *testing.T) {
 				},
 			},
 		},
-		Status: v1.PodStatus{
-		},
+		Status: v1.PodStatus{},
 	}
 	pod.Name = podName
 

flyteplugins/go/tasks/pluginmachinery/core/exec_metadata.go

@@ -2,7 +2,7 @@ package core
 
 import (
 	"github.com/lyft/flyteidl/gen/pb-go/flyteidl/core"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 )
@@ -33,4 +33,3 @@ type TaskExecutionMetadata interface {
 	GetAnnotations() map[string]string
 	GetK8sServiceAccount() string
 }
-

flyteplugins/go/tasks/pluginmachinery/core/phase.go

@@ -118,7 +118,7 @@ func (p PhaseInfo) String() string {
 	return fmt.Sprintf("Phase<%s:%d %s Reason:%s>", p.phase, p.version, p.info, p.reason)
 }
 
-// Undefined entity, associated with an error
+// PhaseInfoUndefined should be used when the Phase is unknown usually associated with an error
 var PhaseInfoUndefined = PhaseInfo{phase: PhaseUndefined}
 
 func phaseInfo(p Phase, v uint32, err *core.ExecutionError, info *TaskInfo) PhaseInfo {

flyteplugins/go/tasks/pluginmachinery/core/resource_manager.go

@@ -83,7 +83,6 @@ type ResourceManager interface {
 	ReleaseResource(ctx context.Context, namespace ResourceNamespace, allocationToken string) error
 }
 
-
 type ResourceConstraint struct {
 	Value int64
 }
@@ -95,7 +94,6 @@ type ResourceConstraint struct {
 // For example, a ResourceConstraintsSpec with nil ProjectScopeResourceConstraint and a non-nil NamespaceScopeResourceConstraint means
 // that it only poses a cap at the namespace level. A zero-value ResourceConstraintsSpec means there's no constraints posed at any level.
 type ResourceConstraintsSpec struct {
-	ProjectScopeResourceConstraint *ResourceConstraint
+	ProjectScopeResourceConstraint   *ResourceConstraint
 	NamespaceScopeResourceConstraint *ResourceConstraint
 }
-

flyteplugins/go/tasks/pluginmachinery/core/transition.go

@@ -39,7 +39,7 @@ func (t Transition) String() string {
 	return fmt.Sprintf("%s,%s", t.ttype, t.info)
 }
 
-// Unknown/Undefined transition. To be returned when an error is observed
+// UnknownTransition is synonymous to UndefinedTransition. To be returned when an error is observed
 var UnknownTransition = Transition{TransitionTypeEphemeral, PhaseInfoUndefined}
 
 // Creates and returns a new Transition based on the PhaseInfo.Phase

flyteplugins/go/tasks/pluginmachinery/flytek8s/config/config.go

@@ -9,7 +9,7 @@ import (
 //go:generate pflags K8sPluginConfig
 
 const k8sPluginConfigSectionKey = "k8s"
-const defaultCpuRequest = "1000m"
+const defaultCPURequest = "1000m"
 const defaultMemoryRequest = "1024Mi"
 
 var (
@@ -19,8 +19,8 @@ var (
 		},
 	}
 
-	// Top level k8s plugin config section. If you are a plugin developer writing a k8s plugin,
-	// register your config section as a subsection to this.
+	// K8sPluginConfigSection provides a singular top level config section for all plugins.
+	// If you are a plugin developer writing a k8s plugin, register your config section as a subsection to this.
 	K8sPluginConfigSection = config.MustRegisterSubSection(k8sPluginConfigSectionKey, &defaultK8sConfig)
 )
 
@@ -40,7 +40,7 @@ type K8sPluginConfig struct {
 	// Currently we support simple resource based tolerations only
 	ResourceTolerations map[v1.ResourceName][]v1.Toleration `json:"resource-tolerations"  pflag:"-,Default tolerations to be applied for resource of type 'key'"`
 	// default cpu requests for a container
-	DefaultCpuRequest string `json:"default-cpus" pflag:",Defines a default value for cpu for containers if not specified."`
+	DefaultCPURequest string `json:"default-cpus" pflag:",Defines a default value for cpu for containers if not specified."`
 	// default memory requests for a container
 	DefaultMemoryRequest string `json:"default-memory" pflag:",Defines a default value for memory for containers if not specified."`
 }
@@ -51,8 +51,8 @@ func GetK8sPluginConfig() *K8sPluginConfig {
 	if pluginsConfig.DefaultMemoryRequest == "" {
 		pluginsConfig.DefaultMemoryRequest = defaultMemoryRequest
 	}
-	if pluginsConfig.DefaultCpuRequest == "" {
-		pluginsConfig.DefaultCpuRequest = defaultCpuRequest
+	if pluginsConfig.DefaultCPURequest == "" {
+		pluginsConfig.DefaultCPURequest = defaultCPURequest
 	}
 	return pluginsConfig
 }

flyteplugins/go/tasks/pluginmachinery/flytek8s/container_helper.go

@@ -40,7 +40,7 @@ func ApplyResourceOverrides(ctx context.Context, resources v1.ResourceRequiremen
 		if _, limitSet := resources.Limits[v1.ResourceCPU]; limitSet {
 			resources.Requests[v1.ResourceCPU] = resources.Limits[v1.ResourceCPU]
 		} else {
-			resources.Requests[v1.ResourceCPU] = resource.MustParse(config.GetK8sPluginConfig().DefaultCpuRequest)
+			resources.Requests[v1.ResourceCPU] = resource.MustParse(config.GetK8sPluginConfig().DefaultCPURequest)
 		}
 	}
 

flyteplugins/go/tasks/pluginmachinery/flytek8s/k8s_resource_adds.go

@@ -40,19 +40,19 @@ func GetExecutionEnvVars(id pluginsCore.TaskExecutionID) []v1.EnvVar {
 	}
 
 	// Execution level env variables.
-	nodeExecutionId := id.GetID().NodeExecutionId.ExecutionId
+	nodeExecutionID := id.GetID().NodeExecutionId.ExecutionId
 	envVars := []v1.EnvVar{
 		{
 			Name:  "FLYTE_INTERNAL_EXECUTION_ID",
-			Value: nodeExecutionId.Name,
+			Value: nodeExecutionID.Name,
 		},
 		{
 			Name:  "FLYTE_INTERNAL_EXECUTION_PROJECT",
-			Value: nodeExecutionId.Project,
+			Value: nodeExecutionID.Project,
 		},
 		{
 			Name:  "FLYTE_INTERNAL_EXECUTION_DOMAIN",
-			Value: nodeExecutionId.Domain,
+			Value: nodeExecutionID.Domain,
 		},
 		// TODO: Fill in these
 		// {
@@ -67,42 +67,42 @@ func GetExecutionEnvVars(id pluginsCore.TaskExecutionID) []v1.EnvVar {
 
 	// Task definition Level env variables.
 	if id.GetID().TaskId != nil {
-		taskId := id.GetID().TaskId
+		taskID := id.GetID().TaskId
 
 		envVars = append(envVars,
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_TASK_PROJECT",
-				Value: taskId.Project,
+				Value: taskID.Project,
 			},
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_TASK_DOMAIN",
-				Value: taskId.Domain,
+				Value: taskID.Domain,
 			},
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_TASK_NAME",
-				Value: taskId.Name,
+				Value: taskID.Name,
 			},
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_TASK_VERSION",
-				Value: taskId.Version,
+				Value: taskID.Version,
 			},
 			// Historic Task Definition Level env variables.
 			// Remove these once SDK is migrated to use the new ones.
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_PROJECT",
-				Value: taskId.Project,
+				Value: taskID.Project,
 			},
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_DOMAIN",
-				Value: taskId.Domain,
+				Value: taskID.Domain,
 			},
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_NAME",
-				Value: taskId.Name,
+				Value: taskID.Name,
 			},
 			v1.EnvVar{
 				Name:  "FLYTE_INTERNAL_VERSION",
-				Value: taskId.Version,
+				Value: taskID.Version,
 			})
 
 	}

flyteplugins/go/tasks/pluginmachinery/flytek8s/pod_helper.go

@@ -180,9 +180,9 @@ func DemystifySuccess(status v1.PodStatus, info pluginsCore.TaskInfo) (pluginsCo
 	return pluginsCore.PhaseInfoSuccess(&info), nil
 }
 
-func ConvertPodFailureToError(status v1.PodStatus) (code, message string) {
-	code = "UnknownError"
-	message = "Container/Pod failed. No message received from kubernetes."
+func ConvertPodFailureToError(status v1.PodStatus) (string, string) {
+	code := "UnknownError"
+	message := "Container/Pod failed. No message received from kubernetes."
 	if len(status.Reason) > 0 {
 		code = status.Reason
 	}
@@ -210,7 +210,7 @@ func ConvertPodFailureToError(status v1.PodStatus) (code, message string) {
 				containerState.Terminated.Message)
 		}
 	}
-	return
+	return code, message
 }
 
 func GetLastTransitionOccurredAt(pod *v1.Pod) v12.Time {

flyteplugins/go/tasks/pluginmachinery/flytek8s/pod_helper_test.go

@@ -2,9 +2,10 @@ package flytek8s
 
 import (
 	"context"
+	"testing"
+
 	"github.com/lyft/flytestdlib/storage"
 	"github.com/stretchr/testify/mock"
-	"testing"
 
 	"github.com/lyft/flyteplugins/go/tasks/pluginmachinery/flytek8s/config"
 	"github.com/lyft/flyteplugins/go/tasks/pluginmachinery/io"

flyteplugins/go/tasks/pluginmachinery/flytek8s/utils.go

@@ -12,5 +12,3 @@ func ToK8sEnvVar(env []*core.KeyValuePair) []v1.EnvVar {
 	}
 	return envVars
 }
-
-

flyteplugins/go/tasks/pluginmachinery/flytek8s/utils_test.go

@@ -1,2 +1 @@
 package flytek8s
-