refactor(cli): restructure CLI to cmd/cli/app pattern and consolidate lifecycle commands

operator/internal/cli/cli.go → operator/cmd/cli/app/cli.go

@@ -16,16 +16,16 @@
  * limitations under the License.
  */
 
-package cli
+package app
 
 import (
 	"fmt"
 
 	"github.com/spf13/cobra"
 
+	"github.com/NVIDIA/skyhook/operator/cmd/cli/app/node"
+	pkg "github.com/NVIDIA/skyhook/operator/cmd/cli/app/package"
 	"github.com/NVIDIA/skyhook/operator/internal/cli/context"
-	"github.com/NVIDIA/skyhook/operator/internal/cli/node"
-	pkg "github.com/NVIDIA/skyhook/operator/internal/cli/package"
 	internalVersion "github.com/NVIDIA/skyhook/operator/internal/version"
 )
 

operator/internal/cli/cli_test.go → operator/cmd/cli/app/cli_test.go

@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-package cli
+package app
 
 import (
 	"bytes"

operator/cmd/cli/app/lifecycle.go

@@ -0,0 +1,175 @@
+/*
+ * SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+
+	"github.com/NVIDIA/skyhook/operator/internal/cli/client"
+	cliContext "github.com/NVIDIA/skyhook/operator/internal/cli/context"
+	"github.com/NVIDIA/skyhook/operator/internal/cli/utils"
+)
+
+// lifecycleConfig defines the configuration for a lifecycle command
+type lifecycleConfig struct {
+	use          string
+	short        string
+	long         string
+	example      string
+	annotation   string
+	action       string // "set" or "remove"
+	verb         string // past tense for output message (e.g., "paused", "resumed")
+	confirmVerb  string // verb for confirmation prompt (e.g., "pause", "disable")
+	needsConfirm bool
+}
+
+// lifecycleOptions holds the options for lifecycle commands that need confirmation
+type lifecycleOptions struct {
+	confirm bool
+}
+
+// newLifecycleCmd creates a lifecycle command based on the provided configuration
+func newLifecycleCmd(ctx *cliContext.CLIContext, cfg lifecycleConfig) *cobra.Command {
+	opts := &lifecycleOptions{}
+
+	cmd := &cobra.Command{
+		Use:     cfg.use,
+		Short:   cfg.short,
+		Long:    cfg.long,
+		Example: cfg.example,
+		Args:    cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			skyhookName := args[0]
+
+			if cfg.needsConfirm && !opts.confirm {
+				_, _ = fmt.Fprintf(cmd.OutOrStdout(), "This will %s Skyhook %q. Continue? [y/N]: ",
+					cfg.confirmVerb, skyhookName)
+				var response string
+				if _, err := fmt.Scanln(&response); err != nil || (response != "y" && response != "Y") {
+					_, _ = fmt.Fprintln(cmd.OutOrStdout(), "Aborted.")
+					return nil
+				}
+			}
+
+			clientFactory := client.NewFactory(ctx.GlobalFlags.ConfigFlags)
+			kubeClient, err := clientFactory.Client()
+			if err != nil {
+				return fmt.Errorf("initializing kubernetes client: %w", err)
+			}
+
+			if cfg.action == "set" {
+				err = utils.SetSkyhookAnnotation(cmd.Context(), kubeClient.Dynamic(), skyhookName, cfg.annotation, "true")
+			} else {
+				err = utils.RemoveSkyhookAnnotation(cmd.Context(), kubeClient.Dynamic(), skyhookName, cfg.annotation)
+			}
+			if err != nil {
+				return err
+			}
+
+			_, _ = fmt.Fprintf(cmd.OutOrStdout(), "Skyhook %q %s\n", skyhookName, cfg.verb)
+			return nil
+		},
+	}
+
+	if cfg.needsConfirm {
+		cmd.Flags().BoolVarP(&opts.confirm, "confirm", "y", false, "Skip confirmation prompt")
+	}
+
+	return cmd
+}
+
+// NewPauseCmd creates the pause command
+func NewPauseCmd(ctx *cliContext.CLIContext) *cobra.Command {
+	return newLifecycleCmd(ctx, lifecycleConfig{
+		use:   "pause <skyhook-name>",
+		short: "Pause a Skyhook from processing",
+		long: `Pause a Skyhook by setting the pause annotation.
+
+When a Skyhook is paused, the operator will stop processing new nodes
+but will not interrupt any currently running operations.`,
+		example: `  # Pause a Skyhook
+  kubectl skyhook pause gpu-init
+
+  # Pause without confirmation
+  kubectl skyhook pause gpu-init --confirm`,
+		annotation:   utils.PauseAnnotation,
+		action:       "set",
+		verb:         "paused",
+		confirmVerb:  "pause",
+		needsConfirm: true,
+	})
+}
+
+// NewResumeCmd creates the resume command
+func NewResumeCmd(ctx *cliContext.CLIContext) *cobra.Command {
+	return newLifecycleCmd(ctx, lifecycleConfig{
+		use:   "resume <skyhook-name>",
+		short: "Resume a paused Skyhook",
+		long: `Resume a paused Skyhook by removing the pause annotation.
+
+The operator will resume processing nodes after this command.`,
+		example: `  # Resume a paused Skyhook
+  kubectl skyhook resume gpu-init`,
+		annotation:   utils.PauseAnnotation,
+		action:       "remove",
+		verb:         "resumed",
+		needsConfirm: false,
+	})
+}
+
+// NewDisableCmd creates the disable command
+func NewDisableCmd(ctx *cliContext.CLIContext) *cobra.Command {
+	return newLifecycleCmd(ctx, lifecycleConfig{
+		use:   "disable <skyhook-name>",
+		short: "Disable a Skyhook completely",
+		long: `Disable a Skyhook by setting the disable annotation.
+
+When a Skyhook is disabled, the operator will completely stop processing
+and the Skyhook will be effectively inactive.`,
+		example: `  # Disable a Skyhook
+  kubectl skyhook disable gpu-init
+
+  # Disable without confirmation
+  kubectl skyhook disable gpu-init --confirm`,
+		annotation:   utils.DisableAnnotation,
+		action:       "set",
+		verb:         "disabled",
+		confirmVerb:  "disable",
+		needsConfirm: true,
+	})
+}
+
+// NewEnableCmd creates the enable command
+func NewEnableCmd(ctx *cliContext.CLIContext) *cobra.Command {
+	return newLifecycleCmd(ctx, lifecycleConfig{
+		use:   "enable <skyhook-name>",
+		short: "Enable a disabled Skyhook",
+		long: `Enable a disabled Skyhook by removing the disable annotation.
+
+The operator will resume normal processing after this command.`,
+		example: `  # Enable a disabled Skyhook
+  kubectl skyhook enable gpu-init`,
+		annotation:   utils.DisableAnnotation,
+		action:       "remove",
+		verb:         "enabled",
+		needsConfirm: false,
+	})
+}

operator/cmd/cli/app/lifecycle_test.go

@@ -0,0 +1,113 @@
+/*
+ * SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package app
+
+import (
+	"github.com/spf13/cobra"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"github.com/NVIDIA/skyhook/operator/internal/cli/context"
+)
+
+var _ = Describe("Lifecycle Commands", func() {
+	type lifecycleTestCase struct {
+		name           string
+		cmdFactory     func(*context.CLIContext) *cobra.Command
+		expectedUse    string
+		expectedVerb   string
+		hasConfirmFlag bool
+	}
+
+	testCases := []lifecycleTestCase{
+		{
+			name:           "Pause",
+			cmdFactory:     NewPauseCmd,
+			expectedUse:    "pause <skyhook-name>",
+			expectedVerb:   "Pause",
+			hasConfirmFlag: true,
+		},
+		{
+			name:           "Resume",
+			cmdFactory:     NewResumeCmd,
+			expectedUse:    "resume <skyhook-name>",
+			expectedVerb:   "Resume",
+			hasConfirmFlag: false,
+		},
+		{
+			name:           "Disable",
+			cmdFactory:     NewDisableCmd,
+			expectedUse:    "disable <skyhook-name>",
+			expectedVerb:   "Disable",
+			hasConfirmFlag: true,
+		},
+		{
+			name:           "Enable",
+			cmdFactory:     NewEnableCmd,
+			expectedUse:    "enable <skyhook-name>",
+			expectedVerb:   "Enable",
+			hasConfirmFlag: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		Describe(tc.name+" Command", func() {
+			It("should create command with correct properties", func() {
+				ctx := context.NewCLIContext(nil)
+				cmd := tc.cmdFactory(ctx)
+
+				Expect(cmd.Use).To(Equal(tc.expectedUse))
+				Expect(cmd.Short).To(ContainSubstring(tc.expectedVerb))
+			})
+
+			It("should handle confirm flag correctly", func() {
+				ctx := context.NewCLIContext(nil)
+				cmd := tc.cmdFactory(ctx)
+
+				confirmFlag := cmd.Flags().Lookup("confirm")
+				if tc.hasConfirmFlag {
+					Expect(confirmFlag).NotTo(BeNil())
+					Expect(confirmFlag.Shorthand).To(Equal("y"))
+					Expect(confirmFlag.DefValue).To(Equal("false"))
+				} else {
+					Expect(confirmFlag).To(BeNil())
+				}
+			})
+
+			It("should require exactly one argument", func() {
+				ctx := context.NewCLIContext(nil)
+				cmd := tc.cmdFactory(ctx)
+
+				err := cmd.Args(cmd, []string{})
+				Expect(err).To(HaveOccurred())
+
+				err = cmd.Args(cmd, []string{"skyhook1"})
+				Expect(err).NotTo(HaveOccurred())
+			})
+
+			It("should have examples in help", func() {
+				ctx := context.NewCLIContext(nil)
+				cmd := tc.cmdFactory(ctx)
+
+				Expect(cmd.Example).To(ContainSubstring("kubectl skyhook"))
+			})
+		})
+	}
+})

operator/internal/cli/package/package_rerun.go → operator/cmd/cli/app/package/package_rerun.go

@@ -280,7 +280,14 @@ func promptConfirmation(cmd *cobra.Command, opts *rerunOptions) (bool, error) {
 }
 
 // updateNodeAnnotations updates each node's annotation to trigger re-run
-func updateNodeAnnotations(ctx context.Context, kubeClient *client.Client, nodesToUpdate []string, nodeStates map[string]v1alpha1.NodeState, packageKey, annotationKey string, opts *rerunOptions) (int, []string) {
+func updateNodeAnnotations(
+	ctx context.Context,
+	kubeClient *client.Client,
+	nodesToUpdate []string,
+	nodeStates map[string]v1alpha1.NodeState,
+	packageKey, annotationKey string,
+	opts *rerunOptions,
+) (int, []string) {
 	var updateErrors []string
 	successCount := 0
 

operator/internal/cli/package/package_rerun_test.go → operator/cmd/cli/app/package/package_rerun_test.go

@@ -316,6 +316,7 @@ var _ = Describe("Package Rerun Command", func() {
 			Expect(output.String()).To(ContainSubstring("No nodes matched"))
 		})
 
+		//nolint:dupl // Test setup intentionally similar to other test cases for clarity
 		It("should show message when package not found on nodes", func() {
 			mockDynamic.On("Resource", skyhookGVR).Return(mockNSRes)
 			mockNSRes.On("Get", mock.Anything, testSkyhookNameRerun, mock.Anything).Return(createSkyhookUnstructured(), nil)
@@ -424,6 +425,7 @@ var _ = Describe("Package Rerun Command", func() {
 			Expect(output.String()).To(ContainSubstring("Aborted"))
 		})
 
+		//nolint:dupl // Test setup intentionally similar to other test cases for clarity
 		It("should update node annotations when confirmed", func() {
 			mockDynamic.On("Resource", skyhookGVR).Return(mockNSRes)
 			mockNSRes.On("Get", mock.Anything, testSkyhookNameRerun, mock.Anything).Return(createSkyhookUnstructured(), nil)

operator/internal/cli/version.go → operator/cmd/cli/app/version.go

@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-package cli
+package app
 
 import (
 	"context"

operator/internal/cli/version_test.go → operator/cmd/cli/app/version_test.go

@@ -16,7 +16,7 @@
  * limitations under the License.
  */
 
-package cli
+package app
 
 import (
 	"bytes"

operator/cmd/cli/main.go

@@ -21,7 +21,7 @@ package main
 import (
 	"os"
 
-	"github.com/NVIDIA/skyhook/operator/internal/cli"
+	cli "github.com/NVIDIA/skyhook/operator/cmd/cli/app"
 	"github.com/NVIDIA/skyhook/operator/internal/cli/context"
 )