Fix flytectl returning the oldest workflow when using --latest flag (f…

…lyteorg#5716) * Fix flytectl returning the oldest workflow when using --latest flag (flyteorg#5716) Signed-off-by: Rafael Raposo <[email protected]> Signed-off-by: Bugra Gedik <[email protected]>
bgedik · Sep 12, 2024 · f30613b · f30613b
1 parent d3a3fd1
commit f30613b
Show file tree

Hide file tree

Showing 10 changed files with 42 additions and 34 deletions.
diff --git a/flytectl/cmd/get/launch_plan.go b/flytectl/cmd/get/launch_plan.go
@@ -219,7 +219,7 @@ func FetchLPForName(ctx context.Context, fetcher ext.AdminFetcherExtInterface, n
 	var lp *admin.LaunchPlan
 	var err error
 	if launchplan.DefaultConfig.Latest {
-		if lp, err = fetcher.FetchLPLatestVersion(ctx, name, project, domain, launchplan.DefaultConfig.Filter); err != nil {
+		if lp, err = fetcher.FetchLPLatestVersion(ctx, name, project, domain); err != nil {
 			return nil, err
 		}
 		launchPlans = append(launchPlans, lp)

diff --git a/flytectl/cmd/get/launch_plan_test.go b/flytectl/cmd/get/launch_plan_test.go
@@ -293,11 +293,10 @@ func TestGetLaunchPlanFuncLatest(t *testing.T) {
 	defer s.TearDown()
 	getLaunchPlanSetup()
 	launchplan.DefaultConfig.Latest = true
-	launchplan.DefaultConfig.Filter = filters.Filters{}
-	s.FetcherExt.OnFetchLPLatestVersionMatch(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(launchPlan2, nil)
+	s.FetcherExt.OnFetchLPLatestVersionMatch(mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(launchPlan2, nil)
 	err := getLaunchPlanFunc(s.Ctx, argsLp, s.CmdCtx)
 	assert.Nil(t, err)
-	s.FetcherExt.AssertCalled(t, "FetchLPLatestVersion", s.Ctx, "launchplan1", projectValue, domainValue, launchplan.DefaultConfig.Filter)
+	s.FetcherExt.AssertCalled(t, "FetchLPLatestVersion", s.Ctx, "launchplan1", projectValue, domainValue)
 	s.TearDownAndVerify(t, `{"id": {"name": "launchplan1","version": "v2"},"spec": {"workflowId": {"name": "workflow2"},"defaultInputs": {"parameters": {"generic": {"var": {"type": {"simple": "STRUCT"},"description": "generic"},"default": {"scalar": {"generic": {"foo": "foo"}}}},"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "short desc"}},"numbers_count": {"var": {"type": {"simple": "INTEGER"},"description": "long description will be truncated in table"}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"},"description": "run_local_at_count"},"default": {"scalar": {"primitive": {"integer": "10"}}}}}}},"closure": {"expectedInputs": {"parameters": {"generic": {"var": {"type": {"simple": "STRUCT"},"description": "generic"},"default": {"scalar": {"generic": {"foo": "foo"}}}},"numbers": {"var": {"type": {"collectionType": {"simple": "INTEGER"}},"description": "short desc"}},"numbers_count": {"var": {"type": {"simple": "INTEGER"},"description": "long description will be truncated in table"}},"run_local_at_count": {"var": {"type": {"simple": "INTEGER"},"description": "run_local_at_count"},"default": {"scalar": {"primitive": {"integer": "10"}}}}}},"createdAt": "1970-01-01T00:00:01Z"}}`)
 }
 

diff --git a/flytectl/cmd/get/workflow.go b/flytectl/cmd/get/workflow.go
@@ -183,7 +183,7 @@ func FetchWorkflowForName(ctx context.Context, fetcher ext.AdminFetcherExtInterf
 	domain string) (workflows []*admin.Workflow, isList bool, err error) {
 	var workflow *admin.Workflow
 	if workflowconfig.DefaultConfig.Latest {
-		if workflow, err = fetcher.FetchWorkflowLatestVersion(ctx, name, project, domain, workflowconfig.DefaultConfig.Filter); err != nil {
+		if workflow, err = fetcher.FetchWorkflowLatestVersion(ctx, name, project, domain); err != nil {
 			return nil, false, err
 		}
 		workflows = append(workflows, workflow)

diff --git a/flytectl/cmd/get/workflow_test.go b/flytectl/cmd/get/workflow_test.go
@@ -174,9 +174,8 @@ func TestGetWorkflowFuncLatestWithTable(t *testing.T) {
 
 	getWorkflowSetup()
 	workflow.DefaultConfig.Latest = true
-	workflow.DefaultConfig.Filter = filters.Filters{}
 	config.GetConfig().Output = printer.OutputFormatTABLE.String()
-	s.FetcherExt.OnFetchWorkflowLatestVersionMatch(s.Ctx, "workflow1", projectValue, domainValue, filters.Filters{}).Return(workflow1, nil)
+	s.FetcherExt.OnFetchWorkflowLatestVersionMatch(s.Ctx, "workflow1", projectValue, domainValue).Return(workflow1, nil)
 	err := getWorkflowFunc(s.Ctx, argsWf, s.CmdCtx)
 	assert.Nil(t, err)
 	s.TearDownAndVerify(t, `

diff --git a/flytectl/pkg/ext/fetcher.go b/flytectl/pkg/ext/fetcher.go
@@ -34,7 +34,7 @@ type AdminFetcherExtInterface interface {
 	FetchAllVerOfLP(ctx context.Context, lpName, project, domain string, filter filters.Filters) ([]*admin.LaunchPlan, error)
 
 	// FetchLPLatestVersion fetches latest version of launch plan in a  project, domain
-	FetchLPLatestVersion(ctx context.Context, name, project, domain string, filter filters.Filters) (*admin.LaunchPlan, error)
+	FetchLPLatestVersion(ctx context.Context, name, project, domain string) (*admin.LaunchPlan, error)
 
 	// FetchLPVersion fetches particular version of launch plan in a  project, domain
 	FetchLPVersion(ctx context.Context, name, version, project, domain string) (*admin.LaunchPlan, error)
@@ -55,7 +55,7 @@ type AdminFetcherExtInterface interface {
 	FetchAllVerOfWorkflow(ctx context.Context, name, project, domain string, filter filters.Filters) ([]*admin.Workflow, error)
 
 	// FetchWorkflowLatestVersion fetches latest version of workflow in a  project, domain
-	FetchWorkflowLatestVersion(ctx context.Context, name, project, domain string, filter filters.Filters) (*admin.Workflow, error)
+	FetchWorkflowLatestVersion(ctx context.Context, name, project, domain string) (*admin.Workflow, error)
 
 	// FetchWorkflowVersion fetches particular version of workflow in a  project, domain
 	FetchWorkflowVersion(ctx context.Context, name, version, project, domain string) (*admin.Workflow, error)

diff --git a/flytectl/pkg/ext/launch_plan_fetcher.go b/flytectl/pkg/ext/launch_plan_fetcher.go
@@ -26,8 +26,13 @@ func (a *AdminFetcherExtClient) FetchAllVerOfLP(ctx context.Context, lpName, pro
 }
 
 // FetchLPLatestVersion fetches latest version for give launch plan name
-func (a *AdminFetcherExtClient) FetchLPLatestVersion(ctx context.Context, name, project, domain string, filter filters.Filters) (*admin.LaunchPlan, error) {
-	// Fetch the latest version of the task.
+func (a *AdminFetcherExtClient) FetchLPLatestVersion(ctx context.Context, name, project, domain string) (*admin.LaunchPlan, error) {
+	// Fetch the latest version of the LaunchPLan.
+	filter := filters.Filters{
+		SortBy: "created_at",
+		Limit:  1,
+		Asc:    false,
+	}
 	lpVersions, err := a.FetchAllVerOfLP(ctx, name, project, domain, filter)
 	if err != nil {
 		return nil, err

diff --git a/flytectl/pkg/ext/launch_plan_fetcher_test.go b/flytectl/pkg/ext/launch_plan_fetcher_test.go
@@ -156,14 +156,14 @@ func TestFetchAllVerOfLPEmptyResponse(t *testing.T) {
 func TestFetchLPLatestVersion(t *testing.T) {
 	getLaunchPlanFetcherSetup()
 	adminClient.OnListLaunchPlansMatch(mock.Anything, mock.Anything).Return(launchPlanListResponse, nil)
-	_, err := adminFetcherExt.FetchLPLatestVersion(ctx, "lpName", "project", "domain", lpFilters)
+	_, err := adminFetcherExt.FetchLPLatestVersion(ctx, "lpName", "project", "domain")
 	assert.Nil(t, err)
 }
 
 func TestFetchLPLatestVersionError(t *testing.T) {
 	launchPlanListResponse := &admin.LaunchPlanList{}
 	getLaunchPlanFetcherSetup()
 	adminClient.OnListLaunchPlansMatch(mock.Anything, mock.Anything).Return(launchPlanListResponse, nil)
-	_, err := adminFetcherExt.FetchLPLatestVersion(ctx, "lpName", "project", "domain", lpFilters)
+	_, err := adminFetcherExt.FetchLPLatestVersion(ctx, "lpName", "project", "domain")
 	assert.Equal(t, fmt.Errorf("no launchplans retrieved for lpName"), err)
 }
diff --git a/flytectl/pkg/ext/mocks/admin_fetcher_ext_interface.go b/flytectl/pkg/ext/mocks/admin_fetcher_ext_interface.go
diff --git a/flytectl/pkg/ext/workflow_fetcher.go b/flytectl/pkg/ext/workflow_fetcher.go
@@ -42,8 +42,13 @@ func (a *AdminFetcherExtClient) FetchAllWorkflows(ctx context.Context, project,
 }
 
 // FetchWorkflowLatestVersion fetches latest version for given workflow name
-func (a *AdminFetcherExtClient) FetchWorkflowLatestVersion(ctx context.Context, name, project, domain string, filter filters.Filters) (*admin.Workflow, error) {
+func (a *AdminFetcherExtClient) FetchWorkflowLatestVersion(ctx context.Context, name, project, domain string) (*admin.Workflow, error) {
 	// Fetch the latest version of the workflow.
+	filter := filters.Filters{
+		SortBy: "created_at",
+		Limit:  1,
+		Asc:    false,
+	}
 	wVersions, err := a.FetchAllVerOfWorkflow(ctx, name, project, domain, filter)
 	if err != nil {
 		return nil, err

diff --git a/flytectl/pkg/ext/workflow_fetcher_test.go b/flytectl/pkg/ext/workflow_fetcher_test.go
@@ -147,14 +147,14 @@ func TestFetchWorkflowLatestVersion(t *testing.T) {
 	getWorkflowFetcherSetup()
 	adminClient.OnGetWorkflowMatch(mock.Anything, mock.Anything).Return(workflowResponse, nil)
 	adminClient.OnListWorkflowsMatch(mock.Anything, mock.Anything).Return(workflowListResponse, nil)
-	_, err := adminFetcherExt.FetchWorkflowLatestVersion(ctx, "workflowName", "project", "domain", workflowFilter)
+	_, err := adminFetcherExt.FetchWorkflowLatestVersion(ctx, "workflowName", "project", "domain")
 	assert.Nil(t, err)
 }
 
 func TestFetchWorkflowLatestVersionError(t *testing.T) {
 	workflowListResponse := &admin.WorkflowList{}
 	getWorkflowFetcherSetup()
 	adminClient.OnListWorkflowsMatch(mock.Anything, mock.Anything).Return(workflowListResponse, nil)
-	_, err := adminFetcherExt.FetchWorkflowLatestVersion(ctx, "workflowName", "project", "domain", workflowFilter)
+	_, err := adminFetcherExt.FetchWorkflowLatestVersion(ctx, "workflowName", "project", "domain")
 	assert.Equal(t, fmt.Errorf("no workflow retrieved for workflowName"), err)
 }