diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2732baaa1dda..76b377637b12 100644 --- a/CONTRIBUTING.md +++ b/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 ``` diff --git a/Dockerfile b/Dockerfile index 34adc2f90cdf..8aa6b550b492 100644 --- a/Dockerfile +++ b/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- && \ diff --git a/Makefile b/Makefile index d1d3142a4117..0c8694658c06 100644 --- a/Makefile +++ b/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 diff --git a/cmd/argo/commands/logs.go b/cmd/argo/commands/logs.go index 026ec7c8b221..0ba0dfa1d979 100644 --- a/cmd/argo/commands/logs.go +++ b/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 { diff --git a/golangci.yml b/golangci.yml new file mode 100644 index 000000000000..297cf81d4b0e --- /dev/null +++ b/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 diff --git a/pkg/apis/workflow/v1alpha1/types.go b/pkg/apis/workflow/v1alpha1/types.go index 855b3ad51473..4529f3bf6e80 100644 --- a/pkg/apis/workflow/v1alpha1/types.go +++ b/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 diff --git a/workflow/artifacts/raw/raw.go b/workflow/artifacts/raw/raw.go index f40d5e985732..3742701d014a 100644 --- a/workflow/artifacts/raw/raw.go +++ b/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 { diff --git a/workflow/common/util.go b/workflow/common/util.go index 4c1dd2514a50..73c56b0b52cd 100644 --- a/workflow/common/util.go +++ b/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) { diff --git a/workflow/controller/dag.go b/workflow/controller/dag.go index 6ebf1d3f2c76..4e686557ebed 100644 --- a/workflow/controller/dag.go +++ b/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 diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 3e14d1b7611b..82a017b52c43 100644 --- a/workflow/controller/operator.go +++ b/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 diff --git a/workflow/controller/steps.go b/workflow/controller/steps.go index 9e55c88d3911..f282c6ed7496 100644 --- a/workflow/controller/steps.go +++ b/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 } diff --git a/workflow/controller/workflowpod.go b/workflow/controller/workflowpod.go index 7a4d6113a7ec..226f3331e242 100644 --- a/workflow/controller/workflowpod.go +++ b/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) } diff --git a/workflow/executor/executor.go b/workflow/executor/executor.go index ad15c5439a86..500dc93c9ba0 100644 --- a/workflow/executor/executor.go +++ b/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 diff --git a/workflow/executor/k8sapi/client.go b/workflow/executor/k8sapi/client.go index 1da5433be3e6..3ceba2d2bf45 100644 --- a/workflow/executor/k8sapi/client.go +++ b/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) -} diff --git a/workflow/executor/kubelet/client.go b/workflow/executor/kubelet/client.go index 6fc7e5df69ab..7d0bdda1594b 100644 --- a/workflow/executor/kubelet/client.go +++ b/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) diff --git a/workflow/ttlcontroller/ttlcontroller.go b/workflow/ttlcontroller/ttlcontroller.go index d5862127a321..0e3249f31772 100644 --- a/workflow/ttlcontroller/ttlcontroller.go +++ b/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 } diff --git a/workflow/util/util.go b/workflow/util/util.go index 842939aa3265..015d626fdeb9 100644 --- a/workflow/util/util.go +++ b/workflow/util/util.go @@ -257,9 +257,9 @@ func SuspendWorkflow(wfIf v1alpha1.WorkflowInterface, workflowName string) error if IsWorkflowCompleted(wf) { return false, errSuspendedCompletedWorkflow } - if wf.Spec.Suspend == nil || *wf.Spec.Suspend != true { + if wf.Spec.Suspend == nil || !*wf.Spec.Suspend { wf.Spec.Suspend = pointer.BoolPtr(true) - wf, err = wfIf.Update(wf) + _, err = wfIf.Update(wf) if err != nil { if apierr.IsConflict(err) { return false, nil @@ -298,7 +298,7 @@ func ResumeWorkflow(wfIf v1alpha1.WorkflowInterface, workflowName string) error } } if updated { - wf, err = wfIf.Update(wf) + _, err = wfIf.Update(wf) if err != nil { if apierr.IsConflict(err) { return false, nil