Use golangci-lint instead of deprecated gometalinter

CONTRIBUTING.md

@@ -25,7 +25,7 @@ Go to https://github.com/argoproj/
 * Docker
 * dep v0.5
    * Mac Install: `brew install dep`
-* gometalinter v2.0.12
+* golangci-lint v1.16.0
 
 ### Quickstart
 ```

Dockerfile

@@ -28,7 +28,13 @@ ENV DEP_VERSION=0.5.0
 RUN wget https://github.com/golang/dep/releases/download/v${DEP_VERSION}/dep-linux-amd64 -O /usr/local/bin/dep && \
     chmod +x /usr/local/bin/dep
 
+# Install golangci-lint
+ENV GOLANGCI_LINT_VERSION=1.16.0
+RUN curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/v$GOLANGCI_LINT_VERSION/install.sh| sh -s -- -b $(go env GOPATH)/bin v$GOLANGCI_LINT_VERSION
+
 # Install gometalinter
+# Keep gometalinter to avoid CI failures during the linter migration.
+# We can remove it after enough time has passed.
 ENV GOMETALINTER_VERSION=2.0.12
 RUN curl -sLo- https://github.com/alecthomas/gometalinter/releases/download/v${GOMETALINTER_VERSION}/gometalinter-${GOMETALINTER_VERSION}-linux-amd64.tar.gz | \
     tar -xzC "$GOPATH/bin" --exclude COPYING --exclude README.md --strip-components 1 -f- && \

Makefile

@@ -127,6 +127,8 @@ endif
 
 .PHONY: lint
 lint:
+	# Remove gometalinter after a migration time.
+	(command -v golangci-lint >/dev/null 2>&1 && golangci-lint run --config golangci.yml) || \
 	gometalinter --config gometalinter.json ./...
 
 .PHONY: test

cmd/argo/commands/logs.go

@@ -204,7 +204,7 @@ func (p *logPrinter) printLiveWorkflowLogs(workflowName string, wfClient workflo
 		}
 		for id := range wf.Status.Nodes {
 			node := wf.Status.Nodes[id]
-			if node.Type == v1alpha1.NodeTypePod && node.Phase != v1alpha1.NodeError && streamedPods[node.ID] == false {
+			if node.Type == v1alpha1.NodeTypePod && node.Phase != v1alpha1.NodeError && !streamedPods[node.ID] {
 				streamedPods[node.ID] = true
 				go func() {
 					var sinceTimePtr *metav1.Time
@@ -369,7 +369,7 @@ func mergeSorted(logs [][]logEntry) []logEntry {
 		left := logs[0]
 		right := logs[1]
 		size, i, j := len(left)+len(right), 0, 0
-		merged := make([]logEntry, size, size)
+		merged := make([]logEntry, size)
 
 		for k := 0; k < size; k++ {
 			if i > len(left)-1 && j <= len(right)-1 {

golangci.yml

@@ -0,0 +1,15 @@
+run:
+  tests: false
+  skip-files:
+    - "pkg/client"
+output:
+  format: colored-line-number
+  print-issued-lines: true
+  print-linter-name: true
+linters:
+  enable:
+  - gofmt
+  - goimports
+  - unconvert
+  - misspell
+  fast: false

pkg/apis/workflow/v1alpha1/types.go

@@ -1028,10 +1028,10 @@ func continues(c *ContinueOn, phase NodePhase) bool {
 	if c == nil {
 		return false
 	}
-	if c.Error == true && phase == NodeError {
+	if c.Error && phase == NodeError {
 		return true
 	}
-	if c.Failed == true && phase == NodeFailed {
+	if c.Failed && phase == NodeFailed {
 		return true
 	}
 	return false

workflow/artifacts/raw/raw.go

@@ -1,9 +1,10 @@
 package raw
 
 import (
+	"os"
+
 	"github.com/argoproj/argo/errors"
 	wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
-	"os"
 )
 
 type RawArtifactDriver struct {

workflow/common/util.go

@@ -474,7 +474,7 @@ func GetTaskAncestry(taskName string, tasks []wfv1.DAGTask) []string {
 	return ancestry
 }
 
-var yamlSeparator = regexp.MustCompile("\\n---")
+var yamlSeparator = regexp.MustCompile(`\n---`)
 
 // SplitYAMLFile is a helper to split a body into multiple workflow objects
 func SplitYAMLFile(body []byte, strict bool) ([]wfv1.Workflow, error) {

workflow/controller/dag.go

@@ -202,9 +202,7 @@ func (woc *wfOperationCtx) executeDAG(nodeName string, tmpl *wfv1.Template, boun
 			woc.log.Println(depName)
 		}
 		outboundNodeIDs := woc.getOutboundNodes(depNode.ID)
-		for _, outNodeID := range outboundNodeIDs {
-			outbound = append(outbound, outNodeID)
-		}
+		outbound = append(outbound, outboundNodeIDs...)
 	}
 	woc.log.Infof("Outbound nodes of %s set to %s", node.ID, outbound)
 	node.OutboundNodes = outbound

workflow/controller/operator.go

@@ -497,7 +497,6 @@ func (woc *wfOperationCtx) podReconciliation() error {
 				woc.completedPods[pod.ObjectMeta.Name] = true
 			}
 		}
-		return
 	}
 
 	parallelPodNum := make(chan string, 500)
@@ -657,7 +656,7 @@ func assessNodeStatus(pod *apiv1.Pod, node *wfv1.NodeStatus) *wfv1.NodeStatus {
 	}
 
 	if newDaemonStatus != nil {
-		if *newDaemonStatus == false {
+		if !*newDaemonStatus {
 			// if the daemon status switched to false, we prefer to just unset daemoned status field
 			// (as opposed to setting it to false)
 			newDaemonStatus = nil
@@ -1198,14 +1197,6 @@ func (woc *wfOperationCtx) markNodePhase(nodeName string, phase wfv1.NodePhase,
 	return node
 }
 
-// markNodeErrorClearOuput is a convenience method to mark a node with an error and clear the output
-func (woc *wfOperationCtx) markNodeErrorClearOuput(nodeName string, err error) *wfv1.NodeStatus {
-	nodeStatus := woc.markNodeError(nodeName, err)
-	nodeStatus.Outputs = nil
-	woc.wf.Status.Nodes[nodeStatus.ID] = *nodeStatus
-	return nodeStatus
-}
-
 // markNodeError is a convenience method to mark a node with an error and set the message from the error
 func (woc *wfOperationCtx) markNodeError(nodeName string, err error) *wfv1.NodeStatus {
 	return woc.markNodePhase(nodeName, wfv1.NodeError, err.Error())
@@ -1288,9 +1279,7 @@ func (woc *wfOperationCtx) getOutboundNodes(nodeID string) []string {
 			outbound = append(outbound, outboundNodeID)
 		} else {
 			subOutIDs := woc.getOutboundNodes(outboundNodeID)
-			for _, subOutID := range subOutIDs {
-				outbound = append(outbound, subOutID)
-			}
+			outbound = append(outbound, subOutIDs...)
 		}
 	}
 	return outbound

workflow/controller/steps.go

@@ -44,7 +44,7 @@ func (woc *wfOperationCtx) executeSteps(nodeName string, tmpl *wfv1.Template, bo
 		sgNodeName := fmt.Sprintf("%s[%d]", nodeName, i)
 		sgNode := woc.getNodeByName(sgNodeName)
 		if sgNode == nil {
-			sgNode = woc.initializeNode(sgNodeName, wfv1.NodeTypeStepGroup, "", stepsCtx.boundaryID, wfv1.NodeRunning)
+			_ = woc.initializeNode(sgNodeName, wfv1.NodeTypeStepGroup, "", stepsCtx.boundaryID, wfv1.NodeRunning)
 		}
 		// The following will connect the step group node to its parents.
 		if i == 0 {
@@ -137,9 +137,7 @@ func (woc *wfOperationCtx) updateOutboundNodes(nodeName string, tmpl *wfv1.Templ
 	for _, childID := range lastSGNode.Children {
 		outboundNodeIDs := woc.getOutboundNodes(childID)
 		woc.log.Infof("Outbound nodes of %s is %s", childID, outboundNodeIDs)
-		for _, outNodeID := range outboundNodeIDs {
-			outbound = append(outbound, outNodeID)
-		}
+		outbound = append(outbound, outboundNodeIDs...)
 	}
 	node := woc.getNodeByName(nodeName)
 	woc.log.Infof("Outbound nodes of %s is %s", node.ID, outbound)
@@ -332,9 +330,7 @@ func (woc *wfOperationCtx) expandStepGroup(stepGroup []wfv1.WorkflowStep) ([]wfv
 		if err != nil {
 			return nil, err
 		}
-		for _, newStep := range expandedStep {
-			newStepGroup = append(newStepGroup, newStep)
-		}
+		newStepGroup = append(newStepGroup, expandedStep...)
 	}
 	return newStepGroup, nil
 }

workflow/controller/workflowpod.go

@@ -598,7 +598,6 @@ func (woc *wfOperationCtx) addInputArtifactsVolumes(pod *apiv1.Pod, tmpl *wfv1.T
 		switch ctr.Name {
 		case common.MainContainerName:
 			mainCtrIndex = i
-			break
 		}
 	}
 	if mainCtrIndex == -1 {
@@ -907,7 +906,7 @@ func createSecretVal(volMap map[string]apiv1.Volume, secret *apiv1.SecretKeySele
 			Key:  secret.Key,
 			Path: secret.Key,
 		}
-		if val, _ := keyMap[secret.Name+"-"+secret.Key]; !val {
+		if val := keyMap[secret.Name+"-"+secret.Key]; !val {
 			keyMap[secret.Name+"-"+secret.Key] = true
 			vol.Secret.Items = append(vol.Secret.Items, key)
 		}

workflow/executor/executor.go

@@ -491,7 +491,7 @@ func (we *WorkflowExecutor) SaveLogs() (*wfv1.Artifact, error) {
 	return &art, nil
 }
 
-// GetSecretFromVolMount will retrive the Secrets from VolumeMount
+// GetSecretFromVolMount will retrieve the Secrets from VolumeMount
 func (we *WorkflowExecutor) GetSecretFromVolMount(accessKeyName string, accessKey string) ([]byte, error) {
 	return ioutil.ReadFile(filepath.Join(common.SecretVolMountPath, accessKeyName, accessKey))
 }
@@ -538,7 +538,7 @@ func (we *WorkflowExecutor) InitDriver(art wfv1.Artifact) (artifact.ArtifactDriv
 			Endpoint:  art.S3.Endpoint,
 			AccessKey: accessKey,
 			SecretKey: secretKey,
-			Secure:    art.S3.Insecure == nil || *art.S3.Insecure == false,
+			Secure:    art.S3.Insecure == nil || !*art.S3.Insecure,
 			Region:    art.S3.Region,
 		}
 		return &driver, nil

workflow/executor/k8sapi/client.go

@@ -4,17 +4,13 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"io/ioutil"
-	"os"
 	"syscall"
 	"time"
 
-	"github.com/argoproj/argo/util"
-
 	"github.com/argoproj/argo/errors"
 	"github.com/argoproj/argo/workflow/common"
 	execcommon "github.com/argoproj/argo/workflow/executor/common"
-	"k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	restclient "k8s.io/client-go/rest"
@@ -38,23 +34,6 @@ func newK8sAPIClient(clientset *kubernetes.Clientset, config *restclient.Config,
 	}, nil
 }
 
-func (c *k8sAPIClient) getFileContents(containerID, sourcePath string) (string, error) {
-	_, containerStatus, err := c.GetContainerStatus(containerID)
-	if err != nil {
-		return "", err
-	}
-	command := []string{"cat", sourcePath}
-	exec, err := common.ExecPodContainer(c.config, c.namespace, c.podName, containerStatus.Name, true, false, command...)
-	if err != nil {
-		return "", err
-	}
-	stdOut, _, err := common.GetExecutorOutput(exec)
-	if err != nil {
-		return "", err
-	}
-	return stdOut.String(), nil
-}
-
 func (c *k8sAPIClient) CreateArchive(containerID, sourcePath string) (*bytes.Buffer, error) {
 	_, containerStatus, err := c.GetContainerStatus(containerID)
 	if err != nil {
@@ -78,43 +57,14 @@ func (c *k8sAPIClient) getLogsAsStream(containerID string) (io.ReadCloser, error
 		return nil, err
 	}
 	return c.clientset.CoreV1().Pods(c.namespace).
-		GetLogs(c.podName, &v1.PodLogOptions{Container: containerStatus.Name, SinceTime: &metav1.Time{}}).Stream()
+		GetLogs(c.podName, &corev1.PodLogOptions{Container: containerStatus.Name, SinceTime: &metav1.Time{}}).Stream()
 }
 
-func (c *k8sAPIClient) getLogs(containerID string) (string, error) {
-	reader, err := c.getLogsAsStream(containerID)
-	if err != nil {
-		return "", err
-	}
-	bytes, err := ioutil.ReadAll(reader)
-	if err != nil {
-		return "", errors.InternalWrapError(err)
-	}
-	return string(bytes), nil
-}
-
-func (c *k8sAPIClient) saveLogs(containerID, path string) error {
-	reader, err := c.getLogsAsStream(containerID)
-	if err != nil {
-		return err
-	}
-	outFile, err := os.Create(path)
-	if err != nil {
-		return errors.InternalWrapError(err)
-	}
-	defer util.Close(outFile)
-	_, err = io.Copy(outFile, reader)
-	if err != nil {
-		return errors.InternalWrapError(err)
-	}
-	return nil
-}
-
-func (c *k8sAPIClient) getPod() (*v1.Pod, error) {
+func (c *k8sAPIClient) getPod() (*corev1.Pod, error) {
 	return c.clientset.CoreV1().Pods(c.namespace).Get(c.podName, metav1.GetOptions{})
 }
 
-func (c *k8sAPIClient) GetContainerStatus(containerID string) (*v1.Pod, *v1.ContainerStatus, error) {
+func (c *k8sAPIClient) GetContainerStatus(containerID string) (*corev1.Pod, *corev1.ContainerStatus, error) {
 	pod, err := c.getPod()
 	if err != nil {
 		return nil, nil, err
@@ -132,7 +82,7 @@ func (c *k8sAPIClient) waitForTermination(containerID string, timeout time.Durat
 	return execcommon.WaitForTermination(c, containerID, timeout)
 }
 
-func (c *k8sAPIClient) KillContainer(pod *v1.Pod, container *v1.ContainerStatus, sig syscall.Signal) error {
+func (c *k8sAPIClient) KillContainer(pod *corev1.Pod, container *corev1.ContainerStatus, sig syscall.Signal) error {
 	command := []string{"/bin/sh", "-c", fmt.Sprintf("kill -%d 1", sig)}
 	exec, err := common.ExecPodContainer(c.config, c.namespace, c.podName, container.Name, false, false, command...)
 	if err != nil {
@@ -145,7 +95,3 @@ func (c *k8sAPIClient) KillContainer(pod *v1.Pod, container *v1.ContainerStatus,
 func (c *k8sAPIClient) killGracefully(containerID string) error {
 	return execcommon.KillGracefully(c, containerID)
 }
-
-func (c *k8sAPIClient) copyArchive(containerID, sourcePath, destPath string) error {
-	return execcommon.CopyArchive(c, containerID, sourcePath, destPath)
-}

workflow/executor/kubelet/client.go

@@ -20,7 +20,7 @@ import (
 	execcommon "github.com/argoproj/argo/workflow/executor/common"
 	"github.com/gorilla/websocket"
 	log "github.com/sirupsen/logrus"
-	"k8s.io/api/core/v1"
+	corev1 "k8s.io/api/core/v1"
 )
 
 const (
@@ -98,7 +98,7 @@ func checkHTTPErr(resp *http.Response) error {
 	return nil
 }
 
-func (k *kubeletClient) getPodList() (*v1.PodList, error) {
+func (k *kubeletClient) getPodList() (*corev1.PodList, error) {
 	u, err := url.ParseRequestURI(fmt.Sprintf("https://%s/pods", k.kubeletEndpoint))
 	if err != nil {
 		return nil, errors.InternalWrapError(err)
@@ -116,7 +116,7 @@ func (k *kubeletClient) getPodList() (*v1.PodList, error) {
 		return nil, err
 	}
 	podListDecoder := json.NewDecoder(resp.Body)
-	podList := &v1.PodList{}
+	podList := &corev1.PodList{}
 	err = podListDecoder.Decode(podList)
 	if err != nil {
 		_ = resp.Body.Close()
@@ -165,7 +165,7 @@ func (k *kubeletClient) doRequestLogs(namespace, podName, containerName string)
 	return resp, nil
 }
 
-func (k *kubeletClient) GetContainerStatus(containerID string) (*v1.Pod, *v1.ContainerStatus, error) {
+func (k *kubeletClient) GetContainerStatus(containerID string) (*corev1.Pod, *corev1.ContainerStatus, error) {
 	podList, err := k.getPodList()
 	if err != nil {
 		return nil, nil, errors.InternalWrapError(err)
@@ -284,7 +284,7 @@ func (k *kubeletClient) WaitForTermination(containerID string, timeout time.Dura
 	return execcommon.WaitForTermination(k, containerID, timeout)
 }
 
-func (k *kubeletClient) KillContainer(pod *v1.Pod, container *v1.ContainerStatus, sig syscall.Signal) error {
+func (k *kubeletClient) KillContainer(pod *corev1.Pod, container *corev1.ContainerStatus, sig syscall.Signal) error {
 	u, err := url.ParseRequestURI(fmt.Sprintf("wss://%s/exec/%s/%s/%s?command=/bin/sh&&command=-c&command=kill+-%d+1&output=1&error=1", k.kubeletEndpoint, pod.Namespace, pod.Name, container.Name, sig))
 	if err != nil {
 		return errors.InternalWrapError(err)

workflow/ttlcontroller/ttlcontroller.go

@@ -188,7 +188,7 @@ func (c *Controller) deleteWorkflow(key string) error {
 	if c.ttlExpired(wf) {
 		log.Infof("Deleting TTL expired workflow %s/%s", wf.Namespace, wf.Name)
 		policy := metav1.DeletePropagationForeground
-		err = c.wfclientset.Argoproj().Workflows(wf.Namespace).Delete(wf.Name, &metav1.DeleteOptions{PropagationPolicy: &policy})
+		err = c.wfclientset.ArgoprojV1alpha1().Workflows(wf.Namespace).Delete(wf.Name, &metav1.DeleteOptions{PropagationPolicy: &policy})
 		if err != nil {
 			return err
 		}