Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --dry-run option to the submit command #1506

Merged
merged 16 commits into from
Aug 5, 2019
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/argo/commands/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ var (
restConfig *rest.Config
clientConfig clientcmd.ClientConfig
clientset *kubernetes.Clientset
wfClientset *wfclientset.Clientset // wfClientset is used for the server-dry-run submit option
wfClient v1alpha1.WorkflowInterface
jobStatusIconMap map[wfv1.NodePhase]string
noColor bool
Expand Down Expand Up @@ -94,8 +95,8 @@ func InitWorkflowClient(ns ...string) v1alpha1.WorkflowInterface {
log.Fatal(err)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not keep the origin code ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed confusing. I can revert the names if that is a problem

wfcs := wfclientset.NewForConfigOrDie(restConfig)
wfClient = wfcs.ArgoprojV1alpha1().Workflows(namespace)
wfClientset = wfclientset.NewForConfigOrDie(restConfig)
wfClient = wfClientset.ArgoprojV1alpha1().Workflows(namespace)
return wfClient
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/argo/commands/resubmit.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func NewResubmitCommand() *cobra.Command {
errors.CheckError(err)
newWF, err := util.FormulateResubmitWorkflow(wf, memoized)
errors.CheckError(err)
created, err := util.SubmitWorkflow(wfClient, newWF, nil)
created, err := util.SubmitWorkflow(wfClient, wfClientset, newWF, nil)
errors.CheckError(err)
printWorkflow(created, cliSubmitOpts.output, DefaultStatus)
waitOrWatch([]string{created.Name}, cliSubmitOpts)
Expand Down
74 changes: 73 additions & 1 deletion cmd/argo/commands/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ import (
"log"
"net/http"
"os"
"strconv"

"github.com/argoproj/pkg/json"
"github.com/spf13/cobra"

apimachineryversion "k8s.io/apimachinery/pkg/version"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
cmdutil "github.com/argoproj/argo/util/cmd"
"github.com/argoproj/argo/workflow/common"
Expand Down Expand Up @@ -52,6 +55,8 @@ func NewSubmitCommand() *cobra.Command {
command.Flags().StringArrayVarP(&submitOpts.Parameters, "parameter", "p", []string{}, "pass an input parameter")
command.Flags().StringVar(&submitOpts.ServiceAccount, "serviceaccount", "", "run all pods in the workflow using specified serviceaccount")
command.Flags().StringVar(&submitOpts.InstanceID, "instanceid", "", "submit with a specific controller's instance id label")
command.Flags().BoolVar(&submitOpts.DryRun, "dry-run", false, "modify the workflow on the client-side without creating it")
command.Flags().BoolVar(&submitOpts.ServerDryRun, "server-dry-run", false, "send request to server with dry-run flag which will modify the workflow without creating it")
command.Flags().StringVarP(&cliSubmitOpts.output, "output", "o", "", "Output format. One of: name|json|yaml|wide")
command.Flags().BoolVarP(&cliSubmitOpts.wait, "wait", "w", false, "wait for the workflow to complete")
command.Flags().BoolVar(&cliSubmitOpts.watch, "watch", false, "watch the workflow until it completes")
Expand Down Expand Up @@ -115,6 +120,47 @@ func SubmitWorkflows(filePaths []string, submitOpts *util.SubmitOpts, cliOpts *c
if cliOpts.wait {
log.Fatalf("--wait cannot be combined with --watch")
}
if submitOpts.DryRun {
log.Fatalf("--watch cannot be combined with --dry-run")
}
if submitOpts.ServerDryRun {
log.Fatalf("--watch cannot be combined with --server-dry-run")
}
}

if cliOpts.wait {
if submitOpts.DryRun {
log.Fatalf("--wait cannot be combined with --dry-run")
}
if submitOpts.ServerDryRun {
log.Fatalf("--wait cannot be combined with --server-dry-run")
}
}

if submitOpts.DryRun {
if cliOpts.output == "" {
log.Fatalf("--dry-run should have an output option")
}
if submitOpts.ServerDryRun {
log.Fatalf("--dry-run cannot be combined with --server-dry-run")
}
}

if submitOpts.ServerDryRun {
if cliOpts.output == "" {
log.Fatalf("--server-dry-run should have an output option")
}
serverVersion, err := wfClientset.Discovery().ServerVersion()
if err != nil {
log.Fatalf("Unexpected error while getting the server's api version")
}
isCompatible, err := checkServerVersionForDryRun(serverVersion)
if err != nil {
log.Fatalf("Unexpected error while checking the server's api version compatibility with --server-dry-run")
}
if !isCompatible {
log.Fatalf("--server-dry-run is not available for server api versions older than v1.12")
}
}

if len(workflows) == 0 {
Expand All @@ -123,13 +169,21 @@ func SubmitWorkflows(filePaths []string, submitOpts *util.SubmitOpts, cliOpts *c
}

var workflowNames []string

for _, wf := range workflows {
wf.Spec.Priority = cliOpts.priority
wfClient := defaultWFClient
if wf.Namespace != "" {
wfClient = InitWorkflowClient(wf.Namespace)
} else {
// This is here to avoid passing an empty namespace when using --server-dry-run
namespace, _, err := clientConfig.Namespace()
if err != nil {
log.Fatal(err)
}
wf.Namespace = namespace
}
created, err := util.SubmitWorkflow(wfClient, &wf, submitOpts)
created, err := util.SubmitWorkflow(wfClient, wfClientset, &wf, submitOpts)
if err != nil {
log.Fatalf("Failed to submit workflow: %v", err)
}
Expand All @@ -139,6 +193,24 @@ func SubmitWorkflows(filePaths []string, submitOpts *util.SubmitOpts, cliOpts *c
waitOrWatch(workflowNames, *cliOpts)
}

// Checks whether the server has support for the dry-run option
func checkServerVersionForDryRun(serverVersion *apimachineryversion.Info) (bool, error) {
majorVersion, err := strconv.Atoi(serverVersion.Major)
if err != nil {
return false, err
}
minorVersion, err := strconv.Atoi(serverVersion.Minor)
if err != nil {
return false, err
}
if majorVersion < 1 {
return false, nil
} else if majorVersion == 1 && minorVersion < 12 {
return false, nil
}
return true, nil
}

// unmarshalWorkflows unmarshals the input bytes as either json or yaml
func unmarshalWorkflows(wfBytes []byte, strict bool) []wfv1.Workflow {
var wf wfv1.Workflow
Expand Down
7 changes: 4 additions & 3 deletions workflow/artifacts/raw/raw_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package raw_test

import (
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/workflow/artifacts/raw"
"github.com/stretchr/testify/assert"
"io/ioutil"
"os"
"testing"
"time"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/workflow/artifacts/raw"
"github.com/stretchr/testify/assert"
)

const (
Expand Down
3 changes: 2 additions & 1 deletion workflow/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ package controller
import (
"bytes"
"encoding/json"
"github.com/argoproj/argo/workflow/config"
"io"
"io/ioutil"
"testing"

"github.com/argoproj/argo/workflow/config"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
fakewfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned/fake"
"github.com/ghodss/yaml"
Expand Down
3 changes: 2 additions & 1 deletion workflow/controller/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ package controller
import (
"encoding/json"
"fmt"
"github.com/argoproj/argo/workflow/config"
"strings"
"testing"

"github.com/argoproj/argo/workflow/config"

wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/argoproj/argo/test"
"github.com/argoproj/argo/workflow/common"
Expand Down
33 changes: 31 additions & 2 deletions workflow/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"github.com/argoproj/argo/errors"
"github.com/argoproj/argo/pkg/apis/workflow"
wfv1 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
wfclientset "github.com/argoproj/argo/pkg/client/clientset/versioned"
"github.com/argoproj/argo/pkg/client/clientset/versioned/typed/workflow/v1alpha1"
cmdutil "github.com/argoproj/argo/util/cmd"
"github.com/argoproj/argo/util/file"
Expand Down Expand Up @@ -141,11 +142,13 @@ type SubmitOpts struct {
Parameters []string // --parameter
ParameterFile string // --parameter-file
ServiceAccount string // --serviceaccount
DryRun bool // --dry-run
ServerDryRun bool // --server-dry-run
OwnerReference *metav1.OwnerReference // useful if your custom controller creates argo workflow resources
}

// SubmitWorkflow validates and submit a single workflow and override some of the fields of the workflow
func SubmitWorkflow(wfIf v1alpha1.WorkflowInterface, wf *wfv1.Workflow, opts *SubmitOpts) (*wfv1.Workflow, error) {
func SubmitWorkflow(wfIf v1alpha1.WorkflowInterface, wfClientset wfclientset.Interface, wf *wfv1.Workflow, opts *SubmitOpts) (*wfv1.Workflow, error) {
if opts == nil {
opts = &SubmitOpts{}
}
Expand Down Expand Up @@ -244,7 +247,33 @@ func SubmitWorkflow(wfIf v1alpha1.WorkflowInterface, wf *wfv1.Workflow, opts *Su
if err != nil {
return nil, err
}
return wfIf.Create(wf)

if opts.ServerDryRun {
wf, err := CreateServerDryRun(wf, wfClientset)
if err != nil {
return nil, err
}
return wf, err
} else if opts.DryRun {
return wf, nil
} else {
return wfIf.Create(wf)
}
}

// CreateServerDryRun fills the workflow struct with the server's representation without creating it and returns an error, if there is any
func CreateServerDryRun(wf *wfv1.Workflow, wfClientset wfclientset.Interface) (*wfv1.Workflow, error) {
// Keep the workflow metadata because it will be overwritten by the Post request
workflowTypeMeta := wf.TypeMeta
err := wfClientset.ArgoprojV1alpha1().RESTClient().Post().
Namespace(wf.Namespace).
Resource("workflows").
Body(wf).
Param("dryRun", "All").
Do().
Into(wf)
wf.TypeMeta = workflowTypeMeta
return wf, err
}

// SuspendWorkflow suspends a workflow by setting spec.suspend to true. Retries conflict errors
Expand Down