Feature: update launchplan --archive to --deactivate (flyteorg#449)

* Rename --archive to --deactivate in update launchplan Signed-off-by: asoundarya96 <[email protected]> * Rename --archive to --deactivate in update launchplan Signed-off-by: asoundarya96 <[email protected]> * Keep --archive as deprecated flag Signed-off-by: asoundarya96 <[email protected]> * make generate Signed-off-by: Eduardo Apolinario <[email protected]> --------- Signed-off-by: asoundarya96 <[email protected]> Signed-off-by: Eduardo Apolinario <[email protected]> Co-authored-by: Eduardo Apolinario <[email protected]>
eapolinario · Dec 26, 2023 · 2eb0674 · 2eb0674
1 parent 0b49071
commit 2eb0674
Show file tree

Hide file tree

Showing 7 changed files with 72 additions and 23 deletions.
diff --git a/clierrors/errors.go b/clierrors/errors.go
@@ -1,7 +1,8 @@
 package clierrors
 
 var (
-	ErrInvalidStateUpdate = "invalid state passed. Specify either activate or archive\n"
+	ErrInvalidStateUpdate     = "invalid state passed. Specify either activate or archive\n"
+	ErrInvalidBothStateUpdate = "invalid state passed. Specify either activate or deactivate\n"
 
 	ErrProjectNotPassed     = "project id wasn't passed\n" // #nosec
 	ErrProjectIDBothPassed  = "both project and id are passed\n"

diff --git a/cmd/config/subcommand/launchplan/updateconfig.go b/cmd/config/subcommand/launchplan/updateconfig.go
@@ -7,9 +7,10 @@ var (
 
 // Config
 type UpdateConfig struct {
-	Archive  bool   `json:"archive" pflag:",disable the launch plan schedule (if it has an active schedule associated with it)."`
-	Activate bool   `json:"activate" pflag:",activate launchplan."`
-	DryRun   bool   `json:"dryRun" pflag:",execute command without making any modifications."`
-	Force    bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
-	Version  string `json:"version" pflag:",version of the launchplan to be fetched."`
+	Activate   bool   `json:"activate" pflag:",activate launchplan."`
+	Archive    bool   `json:"archive" pflag:",(Deprecated) disable the launch plan schedule (if it has an active schedule associated with it)."`
+	Deactivate bool   `json:"deactivate" pflag:",disable the launch plan schedule (if it has an active schedule associated with it)."`
+	DryRun     bool   `json:"dryRun" pflag:",execute command without making any modifications."`
+	Force      bool   `json:"force" pflag:",do not ask for an acknowledgement during updates."`
+	Version    string `json:"version" pflag:",version of the launchplan to be fetched."`
 }
diff --git a/cmd/config/subcommand/launchplan/updateconfig_flags.go b/cmd/config/subcommand/launchplan/updateconfig_flags.go
diff --git a/cmd/config/subcommand/launchplan/updateconfig_flags_test.go b/cmd/config/subcommand/launchplan/updateconfig_flags_test.go
diff --git a/cmd/update/launch_plan.go b/cmd/update/launch_plan.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/admin"
 	"github.com/flyteorg/flyte/flyteidl/gen/pb-go/flyteidl/core"
+	"github.com/flyteorg/flyte/flytestdlib/logger"
 	"github.com/flyteorg/flytectl/clierrors"
 	"github.com/flyteorg/flytectl/cmd/config"
 	"github.com/flyteorg/flytectl/cmd/config/subcommand/launchplan"
@@ -22,10 +23,10 @@ Activates a ` + "`launch plan <https://docs.flyte.org/projects/cookbook/en/lates
 
  flytectl update launchplan -p flytesnacks -d development core.control_flow.merge_sort.merge_sort --version v1 --activate
 
-Archives ` + "`(deactivates) <https://docs.flyte.org/projects/cookbook/en/latest/auto/core/scheduled_workflows/lp_schedules.html#deactivating-a-schedule>`__" + ` a launch plan which deschedules any scheduled job associated with it:
+Deactivates a ` + "`launch plan <https://docs.flyte.org/projects/cookbook/en/latest/auto/core/scheduled_workflows/lp_schedules.html#deactivating-a-schedule>`__" + ` which deschedules any scheduled job associated with it:
 ::
 
- flytectl update launchplan -p flytesnacks -d development core.control_flow.merge_sort.merge_sort --version v1 --archive
+ flytectl update launchplan -p flytesnacks -d development core.control_flow.merge_sort.merge_sort --version v1 --deactivate
 
 Usage
 `
@@ -45,14 +46,22 @@ func updateLPFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandCont
 
 	activate := launchplan.UConfig.Activate
 	archive := launchplan.UConfig.Archive
-	if activate == archive && archive {
-		return fmt.Errorf(clierrors.ErrInvalidStateUpdate)
+
+	var deactivate bool
+	if archive {
+		deprecatedCommandWarning(ctx, "archive", "deactivate")
+		deactivate = true
+	} else {
+		deactivate = launchplan.UConfig.Deactivate
+	}
+	if activate == deactivate && deactivate {
+		return fmt.Errorf(clierrors.ErrInvalidBothStateUpdate)
 	}
 
 	var newState admin.LaunchPlanState
 	if activate {
 		newState = admin.LaunchPlanState_ACTIVE
-	} else if archive {
+	} else if deactivate {
 		newState = admin.LaunchPlanState_INACTIVE
 	}
 
@@ -106,3 +115,7 @@ func updateLPFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandCont
 
 	return nil
 }
+
+func deprecatedCommandWarning(ctx context.Context, oldCommand string, newCommand string) {
+	logger.Warningf(ctx, "--%v is deprecated, Please use --%v", oldCommand, newCommand)
+}
diff --git a/cmd/update/launch_plan_test.go b/cmd/update/launch_plan_test.go
@@ -53,14 +53,32 @@ func TestLaunchPlanCanBeArchived(t *testing.T) {
 		})
 }
 
-func TestLaunchPlanCannotBeActivatedAndArchivedAtTheSameTime(t *testing.T) {
+func TestLaunchPlanCanBeDeactivated(t *testing.T) {
+	testLaunchPlanUpdate(
+		/* setup */ func(s *testutils.TestStruct, config *launchplan.UpdateConfig, launchplan *admin.LaunchPlan) {
+			launchplan.Closure.State = admin.LaunchPlanState_ACTIVE
+			config.Deactivate = true
+			config.Force = true
+		},
+		/* assert */ func(s *testutils.TestStruct, err error) {
+			assert.Nil(t, err)
+			s.MockAdminClient.AssertCalled(
+				t, "UpdateLaunchPlan", s.Ctx,
+				mock.MatchedBy(
+					func(r *admin.LaunchPlanUpdateRequest) bool {
+						return r.State == admin.LaunchPlanState_INACTIVE
+					}))
+		})
+}
+
+func TestLaunchPlanCannotBeActivatedAndDeactivatedAtTheSameTime(t *testing.T) {
 	testLaunchPlanUpdate(
 		/* setup */ func(s *testutils.TestStruct, config *launchplan.UpdateConfig, launchplan *admin.LaunchPlan) {
 			config.Activate = true
-			config.Archive = true
+			config.Deactivate = true
 		},
 		/* assert */ func(s *testutils.TestStruct, err error) {
-			assert.ErrorContains(t, err, "Specify either activate or archive")
+			assert.ErrorContains(t, err, "Specify either activate or deactivate")
 			s.MockAdminClient.AssertNotCalled(t, "UpdateLaunchPlan", mock.Anything, mock.Anything)
 		})
 }

diff --git a/docs/source/gen/flytectl_update_launchplan.rst b/docs/source/gen/flytectl_update_launchplan.rst