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:https://%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
  }