Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
47 changes: 47 additions & 0 deletions flyteadmin/pkg/manager/impl/launch_plan_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,51 @@ func (m *LaunchPlanManager) disableLaunchPlan(ctx context.Context, request *admi
return &admin.LaunchPlanUpdateResponse{}, nil
}

func (m *LaunchPlanManager) archiveLaunchPlan(ctx context.Context, request *admin.LaunchPlanUpdateRequest) (
*admin.LaunchPlanUpdateResponse, error) {
if err := validation.ValidateIdentifier(request.GetId(), common.LaunchPlan); err != nil {
logger.Debugf(ctx, "can't archive launch plan [%+v] with invalid identifier: %v", request.GetId(), err)
return nil, err
}
launchPlanModel, err := util.GetLaunchPlanModel(ctx, m.db, request.GetId())
if err != nil {
logger.Debugf(ctx, "couldn't find launch plan [%+v] to archive with err: %v", request.GetId(), err)
return nil, err
}

err = m.updateLaunchPlanModelState(&launchPlanModel, admin.LaunchPlanState_ARCHIVED)
if err != nil {
logger.Debugf(ctx, "failed to archive launch plan [%+v] with err: %v", request.GetId(), err)
return nil, err
}

var launchPlanSpec admin.LaunchPlanSpec
err = proto.Unmarshal(launchPlanModel.Spec, &launchPlanSpec)
if err != nil {
logger.Errorf(ctx, "failed to unmarshal launch plan spec when archiving %+v", request.GetId())
return nil, errors.NewFlyteAdminErrorf(codes.Internal,
"failed to unmarshal launch plan spec when archiving %+v", request.GetId())
}
if launchPlanSpec.GetEntityMetadata() != nil && launchPlanSpec.GetEntityMetadata().GetSchedule() != nil {
err = m.disableSchedule(ctx, &core.Identifier{
Project: launchPlanModel.Project,
Domain: launchPlanModel.Domain,
Name: launchPlanModel.Name,
Version: launchPlanModel.Version,
})
if err != nil {
return nil, err
}
}
err = m.db.LaunchPlanRepo().Update(ctx, launchPlanModel)
if err != nil {
logger.Debugf(ctx, "Failed to update launchPlanModel with ID [%+v] with err %v", request.GetId(), err)
return nil, err
}
logger.Debugf(ctx, "archived launch plan: [%+v]", request.GetId())
return &admin.LaunchPlanUpdateResponse{}, nil
}

func (m *LaunchPlanManager) enableLaunchPlan(ctx context.Context, request *admin.LaunchPlanUpdateRequest) (
*admin.LaunchPlanUpdateResponse, error) {
newlyActiveLaunchPlanModel, err := m.db.LaunchPlanRepo().Get(ctx, repoInterfaces.Identifier{
Expand Down Expand Up @@ -339,6 +384,8 @@ func (m *LaunchPlanManager) UpdateLaunchPlan(ctx context.Context, request *admin
return m.disableLaunchPlan(ctx, request)
case admin.LaunchPlanState_ACTIVE:
return m.enableLaunchPlan(ctx, request)
case admin.LaunchPlanState_ARCHIVED:
return m.archiveLaunchPlan(ctx, request)
default:
return nil, errors.NewFlyteAdminErrorf(
codes.InvalidArgument, "Unrecognized launch plan state %v for update for launch plan [%+v]",
Expand Down
2 changes: 1 addition & 1 deletion flytectl/cmd/config/subcommand/launchplan/updateconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ var (
// Config
type UpdateConfig struct {
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)."`
Archive bool `json:"archive" pflag:",archive the launch plan version and hide it from list queries (disabling any schedule)."`
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."`
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 19 additions & 11 deletions flytectl/cmd/update/launch_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
cmdUtil "github.com/flyteorg/flyte/flytectl/pkg/commandutils"
"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"
)

const (
Expand All @@ -29,6 +28,13 @@ Deactivates a ` + "`launch plan <https://docs.flyte.org/en/latest/user_guide/pro

flytectl update launchplan -p flytesnacks -d development core.control_flow.merge_sort.merge_sort --version v1 --deactivate

Archives a launch plan version, marking it as old/unused. Archived launch plans can be filtered out
of list queries using ne(state,2) and any active schedule is disabled. Archived launch plans can
still be used to launch executions:
::

flytectl update launchplan -p flytesnacks -d development core.control_flow.merge_sort.merge_sort --version v1 --archive

Usage
`
)
Expand All @@ -47,15 +53,19 @@ func updateLPFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandCont

activate := launchplan.UConfig.Activate
archive := launchplan.UConfig.Archive
deactivate := launchplan.UConfig.Deactivate

var deactivate bool
setCount := 0
if activate {
setCount++
}
if deactivate {
setCount++
}
if archive {
deprecatedCommandWarning(ctx, "archive", "deactivate")
deactivate = true
} else {
deactivate = launchplan.UConfig.Deactivate
setCount++
}
if activate == deactivate && deactivate {
if setCount > 1 {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if we do deactivate and archive? It shouldn't raise an error

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These are three distinct states in the enum (ACTIVE, INACTIVE, ARCHIVED), and each flag maps to exactly one LaunchPlanState value. If we allowed --deactivate --archive together it would be ambiguous — we'd have to silently pick one, which is more confusing than just asking the user to specify a single flag.

Archive already implies the launch plan won't be scheduled (the server-side behavior disables any active schedule), so there's no need to also pass --deactivate. If the intent is to archive, --archive alone is sufficient.

return errors.New(clierrors.ErrInvalidBothStateUpdate)
}

Expand All @@ -64,6 +74,8 @@ func updateLPFunc(ctx context.Context, args []string, cmdCtx cmdCore.CommandCont
newState = admin.LaunchPlanState_ACTIVE
} else if deactivate {
newState = admin.LaunchPlanState_INACTIVE
} else if archive {
newState = admin.LaunchPlanState_ARCHIVED
}

id := &core.Identifier{
Expand Down Expand Up @@ -116,7 +128,3 @@ 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)
}
2 changes: 1 addition & 1 deletion flytectl/cmd/update/launch_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestLaunchPlanCanBeArchived(t *testing.T) {
t, "UpdateLaunchPlan", s.Ctx,
mock.MatchedBy(
func(r *admin.LaunchPlanUpdateRequest) bool {
return r.GetState() == admin.LaunchPlanState_INACTIVE
return r.GetState() == admin.LaunchPlanState_ARCHIVED
}))
})
}
Expand Down
5 changes: 3 additions & 2 deletions flyteidl/clients/go/assets/admin.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions flyteidl/gen/pb-es/flyteidl/admin/launch_plan_pb.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

49 changes: 28 additions & 21 deletions flyteidl/gen/pb-go/flyteidl/admin/launch_plan.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion flyteidl/gen/pb-js/flyteidl.d.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions flyteidl/gen/pb-js/flyteidl.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading