fix: create analysisrun cmd using template generated name (#1471)

Signed-off-by: Hui Kang <[email protected]>

pkg/kubectl-argo-rollouts/cmd/create/create.go

@@ -274,11 +274,11 @@ func NewCmdCreateAnalysisRun(o *options.ArgoRolloutsOptions) *cobra.Command {
 				}
 			}
 
-			objName, notFound, err := unstructured.NestedString(obj.Object, "metadata", "name")
+			objName, found, err := unstructured.NestedString(obj.Object, "metadata", "name")
 			if err != nil {
 				return err
 			}
-			if !notFound {
+			if found {
 				templateName = objName
 			}
 
@@ -292,6 +292,10 @@ func NewCmdCreateAnalysisRun(o *options.ArgoRolloutsOptions) *cobra.Command {
 			}
 			ns := o.Namespace()
 
+			if name == "" && generateName == "-" {
+				return fmt.Errorf("name is invalid")
+			}
+
 			obj, err = analysisutil.NewAnalysisRunFromUnstructured(obj, templateArgs, name, generateName, ns)
 			if err != nil {
 				return err

pkg/kubectl-argo-rollouts/cmd/create/create_test.go

@@ -155,6 +155,21 @@ func TestCreateAnalysisRunName(t *testing.T) {
 	assert.Empty(t, stderr)
 }
 
+func TestCreateAnalysisRunNameNotSpecified(t *testing.T) {
+	tf, o := options.NewFakeArgoRolloutsOptions()
+	defer tf.Cleanup()
+
+	cmd := NewCmdCreateAnalysisRun(o)
+	cmd.PersistentPreRunE = o.PersistentPreRunE
+	cmd.SetArgs([]string{"--from-file", "testdata/analysis-template-no-name.yaml", "-a", "foo=bar"})
+	err := cmd.Execute()
+	assert.EqualError(t, err, "name is invalid")
+	stdout := o.Out.(*bytes.Buffer).String()
+	stderr := o.ErrOut.(*bytes.Buffer).String()
+	assert.Empty(t, stdout)
+	assert.Equal(t, "Error: name is invalid\n", stderr)
+}
+
 func TestCreateAnalysisRunWithInstanceID(t *testing.T) {
 	tf, o := options.NewFakeArgoRolloutsOptions()
 	defer tf.Cleanup()

pkg/kubectl-argo-rollouts/cmd/create/testdata/analysis-template-no-name.yaml

@@ -0,0 +1,22 @@
+kind: AnalysisTemplate
+apiVersion: argoproj.io/v1alpha1
+metadata:
+spec:
+  args:
+  - name: foo
+  metrics:
+  - name: pass
+    interval: 5s
+    failureLimit: 1
+    provider:
+      job:
+        spec:
+          template:
+            spec:
+              containers:
+              - name: sleep
+                image: alpine:3.8
+                command: [sh, -c]
+                args: [exit 0]
+              restartPolicy: Never
+          backoffLimit: 0

utils/analysis/helpers.go

@@ -13,7 +13,6 @@ import (
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	patchtypes "k8s.io/apimachinery/pkg/types"
-	"k8s.io/apimachinery/pkg/util/validation/field"
 
 	argoprojclient "github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/typed/rollouts/v1alpha1"
 )
@@ -373,7 +372,17 @@ func NewAnalysisRunFromUnstructured(obj *unstructured.Unstructured, templateArgs
 		return nil, err
 	}
 
+	// Remove resourceVersion if exists
+	_, found, err := unstructured.NestedString(obj.Object, "metadata", "resourceVersion")
+	if err != nil {
+		return nil, err
+	}
+	if found {
+		unstructured.RemoveNestedField(obj.Object, "metadata", "resourceVersion")
+	}
+
 	// Set args
+	newArgVals := []interface{}{}
 	for i := 0; i < len(newArgs); i++ {
 		var newArgInterface map[string]interface{}
 		newArgBytes, err := json.Marshal(newArgs[i])
@@ -384,7 +393,10 @@ func NewAnalysisRunFromUnstructured(obj *unstructured.Unstructured, templateArgs
 		if err != nil {
 			return nil, err
 		}
-		err = unstructured.SetNestedMap(obj.Object, newArgInterface, field.NewPath("spec", "args").Index(i).String())
+		newArgVals = append(newArgVals, newArgInterface)
+	}
+	if len(newArgVals) > 0 {
+		err = unstructured.SetNestedSlice(obj.Object, newArgVals, "spec", "args")
 		if err != nil {
 			return nil, err
 		}

utils/analysis/helpers_test.go

@@ -1,20 +1,23 @@
 package analysis
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"testing"
 
 	log "github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kunstructured "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	kubetesting "k8s.io/client-go/testing"
 	"k8s.io/utils/pointer"
 
 	"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
 	"github.com/argoproj/argo-rollouts/pkg/client/clientset/versioned/fake"
+	"github.com/argoproj/argo-rollouts/utils/unstructured"
 )
 
 func TestIsWorst(t *testing.T) {
@@ -630,6 +633,60 @@ func TestMergeArgs(t *testing.T) {
 	}
 }
 
+func TestNewAnalysisRunFromUnstructured(t *testing.T) {
+	template := v1alpha1.AnalysisTemplate{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:            "foo",
+			Namespace:       metav1.NamespaceDefault,
+			ResourceVersion: "12345",
+		},
+		Spec: v1alpha1.AnalysisTemplateSpec{
+			Metrics: []v1alpha1.Metric{
+				{
+					Name: "success-rate",
+				},
+			},
+			Args: []v1alpha1.Argument{
+				{
+					Name: "my-arg-1",
+				},
+				{
+					Name: "my-arg-2",
+				},
+			},
+		},
+	}
+	args := []v1alpha1.Argument{
+		{
+			Name:  "my-arg-1",
+			Value: pointer.StringPtr("my-val-1"),
+		},
+		{
+			Name:  "my-arg-2",
+			Value: pointer.StringPtr("my-val-2"),
+		},
+	}
+
+	jsonStr, err := json.Marshal(template)
+	assert.NoError(t, err)
+	obj, err := unstructured.StrToUnstructured(string(jsonStr))
+	assert.NoError(t, err)
+
+	obj, err = NewAnalysisRunFromUnstructured(obj, args, "foo-run", "foo-run-generate-", "my-ns")
+	assert.NoError(t, err)
+	_, found, err := kunstructured.NestedString(obj.Object, "metadata", "resourceVersion")
+	assert.NoError(t, err)
+	assert.False(t, found)
+	arArgs, _, err := kunstructured.NestedSlice(obj.Object, "spec", "args")
+	assert.NoError(t, err)
+	assert.Equal(t, len(args), len(arArgs))
+
+	for i, arg := range arArgs {
+		argnv := arg.(map[string]interface{})
+		assert.Equal(t, *args[i].Value, argnv["value"])
+	}
+}
+
 //TODO(dthomson) remove this test in v0.9.0
 func TestNewAnalysisRunFromTemplate(t *testing.T) {
 	template := v1alpha1.AnalysisTemplate{