Skip to content

Commit

Permalink
feat(backend): upgrade argo library to 2.12.6. Part of #4553 (#5041)
Browse files Browse the repository at this point in the history
* tests

* added go mod file

* updated go.mod

* argo latest stable

* upgrade argo

* clean up

* go mod tidy to clean up

* fixed test after backend

* go mod tidy clean up

* more clean up

* added helper function and updated after feedback

* updated k8s.io/kubernetes to version 0.17.9

* updated go dependencies
  • Loading branch information
NikeNano authored Mar 3, 2021
1 parent fe6f073 commit df48073
Show file tree
Hide file tree
Showing 20 changed files with 366 additions and 224 deletions.
2 changes: 1 addition & 1 deletion backend/src/apiserver/resource/model_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func toModelParameters(apiParams []*api.Parameter) (string, error) {
for _, apiParam := range apiParams {
param := v1alpha1.Parameter{
Name: apiParam.Name,
Value: &apiParam.Value,
Value: v1alpha1.AnyStringPtr(apiParam.Value),
}
params = append(params, param)
}
Expand Down
10 changes: 5 additions & 5 deletions backend/src/apiserver/resource/resource_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func TestCreateRun_ThroughPipelineID(t *testing.T) {

expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("world")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("world")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = defaultPipelineRunnerServiceAccount
Expand Down Expand Up @@ -409,7 +409,7 @@ func TestCreateRun_ThroughWorkflowSpec(t *testing.T) {
expectedExperimentUUID := runDetail.ExperimentUUID
expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("world")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("world")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = defaultPipelineRunnerServiceAccount
Expand Down Expand Up @@ -458,7 +458,7 @@ func TestCreateRun_ThroughWorkflowSpecWithPatch(t *testing.T) {
expectedExperimentUUID := runDetail.ExperimentUUID
expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("test-default-bucket")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("test-default-bucket")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = defaultPipelineRunnerServiceAccount
Expand Down Expand Up @@ -544,7 +544,7 @@ func TestCreateRun_ThroughPipelineVersion(t *testing.T) {

expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("world")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("world")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = "sa1"
Expand Down Expand Up @@ -640,7 +640,7 @@ func TestCreateRun_ThroughPipelineIdAndPipelineVersion(t *testing.T) {

expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("world")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("world")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = "sa1"
Expand Down
8 changes: 4 additions & 4 deletions backend/src/apiserver/resource/resource_manager_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,22 +208,22 @@ func OverrideParameterWithSystemDefault(workflow util.Workflow, apiRun *api.Run)
patchedSlice := make([]wfv1.Parameter, 0)
for _, currentParam := range workflow.Spec.Arguments.Parameters {
if currentParam.Value != nil {
desiredValue, err := PatchPipelineDefaultParameter(*currentParam.Value)
desiredValue, err := PatchPipelineDefaultParameter(currentParam.Value.String())
if err != nil {
return fmt.Errorf("failed to patch default value to pipeline. Error: %v", err)
}
patchedSlice = append(patchedSlice, wfv1.Parameter{
Name: currentParam.Name,
Value: util.StringPointer(desiredValue),
Value: wfv1.AnyStringPtr(desiredValue),
})
} else if currentParam.Default != nil {
desiredValue, err := PatchPipelineDefaultParameter(*currentParam.Default)
desiredValue, err := PatchPipelineDefaultParameter(currentParam.Default.String())
if err != nil {
return fmt.Errorf("failed to patch default value to pipeline. Error: %v", err)
}
patchedSlice = append(patchedSlice, wfv1.Parameter{
Name: currentParam.Name,
Value: util.StringPointer(desiredValue),
Value: wfv1.AnyStringPtr(desiredValue),
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion backend/src/apiserver/server/api_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func toApiParameters(paramsString string) ([]*api.Parameter, error) {
for _, param := range params {
var value string
if param.Value != nil {
value = *param.Value
value = param.Value.String()
}
apiParam := api.Parameter{
Name: param.Name,
Expand Down
11 changes: 5 additions & 6 deletions backend/src/apiserver/server/run_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@ import (
"strings"
"testing"

"github.com/kubeflow/pipelines/backend/src/apiserver/resource"

"github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/golang/protobuf/ptypes/timestamp"
"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
api "github.com/kubeflow/pipelines/backend/api/go_client"
"github.com/kubeflow/pipelines/backend/src/apiserver/common"
"github.com/kubeflow/pipelines/backend/src/apiserver/resource"
"github.com/kubeflow/pipelines/backend/src/common/util"
"github.com/spf13/viper"
"github.com/stretchr/testify/assert"
Expand All @@ -38,7 +37,7 @@ func TestCreateRun(t *testing.T) {

expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("world")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("world")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = "pipeline-runner"
Expand Down Expand Up @@ -88,8 +87,8 @@ func TestCreateRunPatch(t *testing.T) {

expectedRuntimeWorkflow := testWorkflowPatch.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("test-default-bucket")},
{Name: "param2", Value: util.StringPointer("test-project-id")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("test-default-bucket")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("test-project-id")},
}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
Expand Down Expand Up @@ -183,7 +182,7 @@ func TestCreateRun_Multiuser(t *testing.T) {

expectedRuntimeWorkflow := testWorkflow.DeepCopy()
expectedRuntimeWorkflow.Spec.Arguments.Parameters = []v1alpha1.Parameter{
{Name: "param1", Value: util.StringPointer("world")}}
{Name: "param1", Value: v1alpha1.AnyStringPtr("world")}}
expectedRuntimeWorkflow.Labels = map[string]string{util.LabelKeyWorkflowRunId: "123e4567-e89b-12d3-a456-426655440000"}
expectedRuntimeWorkflow.Annotations = map[string]string{util.AnnotationKeyRunName: "run1"}
expectedRuntimeWorkflow.Spec.ServiceAccountName = "default-editor" // In multi-user mode, we use default service account.
Expand Down
4 changes: 2 additions & 2 deletions backend/src/common/util/formatter.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,14 @@ func (p *WorkflowFormatter) formatWorkflowParameters(workflow *v1alpha1.Workflow
}

func (p *WorkflowFormatter) formatParameter(param v1alpha1.Parameter) (*v1alpha1.Parameter, error) {
formatted, err := p.formatString(*param.Value)
formatted, err := p.formatString(param.Value.String())
if err != nil {
return nil, err
}

return &v1alpha1.Parameter{
Name: param.Name,
Value: &formatted,
Value: v1alpha1.AnyStringPtr(formatted),
}, nil
}

Expand Down
34 changes: 17 additions & 17 deletions backend/src/common/util/formatter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,12 @@ func TestFormatParameter(t *testing.T) {

param := v1alpha1.Parameter{
Name: "PARAM_NAME",
Value: StringPointer("PARAM_PREFIX_[[uuid]]_SUFFIX"),
Value: v1alpha1.AnyStringPtr("PARAM_PREFIX_[[uuid]]_SUFFIX"),
}

expected := v1alpha1.Parameter{
Name: "PARAM_NAME",
Value: StringPointer("PARAM_PREFIX_" + defaultUUID + "_SUFFIX"),
Value: v1alpha1.AnyStringPtr("PARAM_PREFIX_" + defaultUUID + "_SUFFIX"),
}

result, err := formatter.formatParameter(param)
Expand All @@ -141,7 +141,7 @@ func TestFormatParameterError(t *testing.T) {

param := v1alpha1.Parameter{
Name: "PARAM_NAME",
Value: StringPointer("PARAM_PREFIX_[[uuid]]_SUFFIX"),
Value: v1alpha1.AnyStringPtr("PARAM_PREFIX_[[uuid]]_SUFFIX"),
}

result, err := formatter.formatParameter(param)
Expand All @@ -160,8 +160,8 @@ func TestFormatNothingToDoExceptAddUUID(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1")},
{Name: "param2", Value: StringPointer("value2")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2")},
},
}}}

Expand All @@ -170,8 +170,8 @@ func TestFormatNothingToDoExceptAddUUID(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1")},
{Name: "param2", Value: StringPointer("value2")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2")},
},
}}}

Expand All @@ -191,8 +191,8 @@ func TestFormatEverytingToChange(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1-[[schedule]]")},
{Name: "param2", Value: StringPointer("value2-[[now]]-suffix")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1-[[schedule]]")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2-[[now]]-suffix")},
},
}}}

Expand All @@ -201,8 +201,8 @@ func TestFormatEverytingToChange(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1-20170706050403")},
{Name: "param2", Value: StringPointer("value2-20180807060504-suffix")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1-20170706050403")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2-20180807060504-suffix")},
},
}}}

Expand Down Expand Up @@ -290,8 +290,8 @@ func TestFormatOnlyWorkflowParameters(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1-[[schedule]]")},
{Name: "param2", Value: StringPointer("value2-[[now]]-suffix")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1-[[schedule]]")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2-[[now]]-suffix")},
},
}}}

Expand All @@ -300,8 +300,8 @@ func TestFormatOnlyWorkflowParameters(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1-20170706050403")},
{Name: "param2", Value: StringPointer("value2-20180807060504-suffix")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1-20170706050403")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2-20180807060504-suffix")},
},
}}}

Expand Down Expand Up @@ -337,8 +337,8 @@ func TestFormatError(t *testing.T) {
Spec: v1alpha1.WorkflowSpec{
Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{
{Name: "param1", Value: StringPointer("value1-[[schedule]]-[[uuid]]")},
{Name: "param2", Value: StringPointer("value2-[[now]]-suffix")},
{Name: "param1", Value: v1alpha1.AnyStringPtr("value1-[[schedule]]-[[uuid]]")},
{Name: "param2", Value: v1alpha1.AnyStringPtr("value2-[[now]]-suffix")},
},
}}}

Expand Down
16 changes: 16 additions & 0 deletions backend/src/common/util/pointer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"fmt"
"time"

workflowapi "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
"github.com/go-openapi/strfmt"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)
Expand Down Expand Up @@ -95,3 +96,18 @@ func ToInt64Pointer(t *metav1.Time) *int64 {
return Int64Pointer(t.Unix())
}
}

func ToAnyStringPointer(s *string) *workflowapi.AnyString {
if s != nil {
return workflowapi.AnyStringPtr(*s)
}
return nil
}

func ToStringPointer(a *workflowapi.AnyString) *string {
if a != nil {
v := a.String()
return &v
}
return nil
}
2 changes: 1 addition & 1 deletion backend/src/common/util/scheduled_workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (s *ScheduledWorkflow) ParametersAsString() (string, error) {
for _, param := range params {
workflowParam := workflowapi.Parameter{
Name: param.Name,
Value: &param.Value,
Value: workflowapi.AnyStringPtr(param.Value),
}
workflowParams = append(workflowParams, workflowParam)
}
Expand Down
4 changes: 2 additions & 2 deletions backend/src/common/util/scheduled_workflow_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ func TestScheduledWorkflow_ParametersAsString(t *testing.T) {
ServiceAccountName: "SERVICE_ACCOUNT",
Arguments: workflowapi.Arguments{
Parameters: []workflowapi.Parameter{
{Name: "PARAM3", Value: StringPointer("VALUE3")},
{Name: "PARAM4", Value: StringPointer("VALUE4")},
{Name: "PARAM3", Value: workflowapi.AnyStringPtr("VALUE3")},
{Name: "PARAM4", Value: workflowapi.AnyStringPtr("VALUE4")},
},
},
},
Expand Down
6 changes: 3 additions & 3 deletions backend/src/common/util/template_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ import (
"github.com/ghodss/yaml"
"github.com/stretchr/testify/assert"
"google.golang.org/grpc/codes"
"k8s.io/apimachinery/pkg/apis/meta/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestGetParameters(t *testing.T) {
template := v1alpha1.Workflow{
TypeMeta: v1.TypeMeta{APIVersion: "argoproj.io/v1alpha1", Kind: "Workflow"},
Spec: v1alpha1.WorkflowSpec{Arguments: v1alpha1.Arguments{
Parameters: []v1alpha1.Parameter{{Name: "name1", Value: StringPointer("value1")}}}}}
Parameters: []v1alpha1.Parameter{{Name: "name1", Value: v1alpha1.AnyStringPtr("value1")}}}}}
templateBytes, _ := yaml.Marshal(template)
paramString, err := GetParameters(templateBytes)
assert.Nil(t, err)
Expand All @@ -39,7 +39,7 @@ func TestGetParameters_ParametersTooLong(t *testing.T) {
var params []v1alpha1.Parameter
// Create a long enough parameter string so it exceed the length limit of parameter.
for i := 0; i < 10000; i++ {
params = append(params, v1alpha1.Parameter{Name: "name1", Value: StringPointer("value1")})
params = append(params, v1alpha1.Parameter{Name: "name1", Value: v1alpha1.AnyStringPtr("value1")})
}
template := v1alpha1.Workflow{Spec: v1alpha1.WorkflowSpec{Arguments: v1alpha1.Arguments{
Parameters: params}}}
Expand Down
8 changes: 5 additions & 3 deletions backend/src/common/util/workflow.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ func (w *Workflow) OverrideParameters(desiredParams map[string]string) {
if param, ok := desiredParams[currentParam.Name]; ok {
desiredValue = &param
} else {
desiredValue = currentParam.Value
desired := currentParam.Value.String()
desiredValue = &desired
}

desiredSlice = append(desiredSlice, workflowapi.Parameter{
Name: currentParam.Name,
Value: desiredValue,
Value: ToAnyStringPointer(desiredValue),
})
}
w.Spec.Arguments.Parameters = desiredSlice
Expand All @@ -64,7 +66,7 @@ func (w *Workflow) OverrideParameters(desiredParams map[string]string) {
func (w *Workflow) VerifyParameters(desiredParams map[string]string) error {
templateParamsMap := make(map[string]*string)
for _, param := range w.Spec.Arguments.Parameters {
templateParamsMap[param.Name] = param.Value
templateParamsMap[param.Name] = ToStringPointer(param.Value)
}
for k := range desiredParams {
_, ok := templateParamsMap[k]
Expand Down
Loading

0 comments on commit df48073

Please sign in to comment.