Skip to content

Commit edf9604

Browse files
t0mmylamt0mmylam
authored
fix: cleanup cli code (#133)
1 parent 4a3e388 commit edf9604

File tree

17 files changed

+139
-179
lines changed

17 files changed

+139
-179
lines changed

‎operator/cmd/cli/app/cli_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ var _ = Describe("Skyhook CLI Tests", func() {
247247

248248
Describe("defaultNamespace constant", func() {
249249
It("should be set to skyhook", func() {
250-
Expect(defaultNamespace).To(Equal("skyhook"))
250+
Expect(context.DefaultNamespace).To(Equal("skyhook"))
251251
})
252252
})
253253
})

‎operator/cmd/cli/app/lifecycle.go‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
package app
2020

2121
import (
22+
"bufio"
2223
"fmt"
24+
"strings"
2325

2426
"github.com/spf13/cobra"
2527

@@ -62,11 +64,23 @@ func newLifecycleCmd(ctx *cliContext.CLIContext, cfg lifecycleConfig) *cobra.Com
6264
if cfg.needsConfirm && !opts.confirm {
6365
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "This will %s Skyhook %q. Continue? [y/N]: ",
6466
cfg.confirmVerb, skyhookName)
65-
var response string
66-
if _, err := fmt.Scanln(&response); err != nil || (response != "y" && response != "Y") {
67+
reader := bufio.NewReader(cmd.InOrStdin())
68+
response, err := reader.ReadString('\n')
69+
if err != nil {
6770
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "Aborted.")
6871
return nil
6972
}
73+
response = strings.TrimSpace(response)
74+
if response != "y" && response != "Y" {
75+
_, _ = fmt.Fprintln(cmd.OutOrStdout(), "Aborted.")
76+
return nil
77+
}
78+
}
79+
80+
// Check dry-run before making changes
81+
if ctx.GlobalFlags.DryRun {
82+
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "[dry-run] Would %s Skyhook %q\n", cfg.confirmVerb, skyhookName)
83+
return nil
7084
}
7185

7286
clientFactory := client.NewFactory(ctx.GlobalFlags.ConfigFlags)
@@ -131,6 +145,7 @@ The operator will resume processing nodes after this command.`,
131145
annotation: utils.PauseAnnotation,
132146
action: "remove",
133147
verb: "resumed",
148+
confirmVerb: "resume",
134149
needsConfirm: false,
135150
})
136151
}
@@ -170,6 +185,7 @@ The operator will resume normal processing after this command.`,
170185
annotation: utils.DisableAnnotation,
171186
action: "remove",
172187
verb: "enabled",
188+
confirmVerb: "enable",
173189
needsConfirm: false,
174190
})
175191
}

‎operator/cmd/cli/app/node/node_list.go‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,11 @@ import (
3838
// nodeListOptions holds the options for the node list command
3939
type nodeListOptions struct {
4040
skyhookName string
41-
output string
4241
}
4342

4443
// BindToCmd binds the options to the command flags
4544
func (o *nodeListOptions) BindToCmd(cmd *cobra.Command) {
4645
cmd.Flags().StringVar(&o.skyhookName, "skyhook", "", "Name of the Skyhook CR (required)")
47-
cmd.Flags().StringVarP(&o.output, "output", "o", "table", "Output format: table, json, yaml, wide")
4846

4947
_ = cmd.MarkFlagRequired("skyhook")
5048
}
@@ -76,7 +74,7 @@ specified Skyhook CR, along with a summary of package completion status.`,
7674
return fmt.Errorf("initializing kubernetes client: %w", err)
7775
}
7876

79-
return runNodeList(cmd.Context(), cmd.OutOrStdout(), kubeClient, opts)
77+
return runNodeList(cmd.Context(), kubeClient, opts, ctx)
8078
},
8179
}
8280

@@ -94,7 +92,8 @@ type nodeListEntry struct {
9492
Restarts int32 `json:"restarts"`
9593
}
9694

97-
func runNodeList(ctx context.Context, out io.Writer, kubeClient *client.Client, opts *nodeListOptions) error {
95+
func runNodeList(ctx context.Context, kubeClient *client.Client, opts *nodeListOptions, cliCtx *cliContext.CLIContext) error {
96+
out := cliCtx.Config().OutputWriter
9897
// Get all nodes
9998
nodeList, err := kubeClient.Kubernetes().CoreV1().Nodes().List(ctx, metav1.ListOptions{})
10099
if err != nil {
@@ -112,6 +111,9 @@ func runNodeList(ctx context.Context, out io.Writer, kubeClient *client.Client,
112111

113112
var nodeState v1alpha1.NodeState
114113
if err := json.Unmarshal([]byte(annotation), &nodeState); err != nil {
114+
if cliCtx.GlobalFlags.Verbose {
115+
_, _ = fmt.Fprintf(cliCtx.Config().ErrorWriter, "Warning: skipping node %q - invalid annotation: %v\n", node.Name, err)
116+
}
115117
continue
116118
}
117119

@@ -163,12 +165,12 @@ func runNodeList(ctx context.Context, out io.Writer, kubeClient *client.Client,
163165

164166
// Output based on format
165167
output := nodeListOutput{SkyhookName: opts.skyhookName, Nodes: entries}
166-
switch opts.output {
167-
case "json":
168+
switch cliCtx.GlobalFlags.OutputFormat {
169+
case utils.OutputFormatJSON:
168170
return utils.OutputJSON(out, output)
169-
case "yaml":
171+
case utils.OutputFormatYAML:
170172
return utils.OutputYAML(out, output)
171-
case "wide":
173+
case utils.OutputFormatWide:
172174
return outputNodeListTableOrWide(out, opts.skyhookName, entries, true)
173175
default:
174176
return outputNodeListTableOrWide(out, opts.skyhookName, entries, false)

‎operator/cmd/cli/app/node/node_list_test.go‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/NVIDIA/skyhook/operator/api/v1alpha1"
3333
"github.com/NVIDIA/skyhook/operator/internal/cli/client"
3434
"github.com/NVIDIA/skyhook/operator/internal/cli/context"
35+
"github.com/NVIDIA/skyhook/operator/internal/cli/utils"
3536
)
3637

3738
var _ = Describe("Node List Command", func() {
@@ -58,14 +59,6 @@ var _ = Describe("Node List Command", func() {
5859
Expect(err.Error()).To(ContainSubstring("skyhook"))
5960
})
6061

61-
It("should have output flag", func() {
62-
ctx := context.NewCLIContext(nil)
63-
cmd := NewListCmd(ctx)
64-
65-
outputFlag := cmd.Flags().Lookup("output")
66-
Expect(outputFlag).NotTo(BeNil())
67-
Expect(outputFlag.Shorthand).To(Equal("o"))
68-
})
6962
})
7063

7164
Describe("outputNodeListTableOrWide", func() {
@@ -123,12 +116,14 @@ var _ = Describe("Node List Command", func() {
123116
output *bytes.Buffer
124117
mockKube *fake.Clientset
125118
kubeClient *client.Client
119+
cliCtx *context.CLIContext
126120
)
127121

128122
BeforeEach(func() {
129123
output = &bytes.Buffer{}
130124
mockKube = fake.NewSimpleClientset()
131125
kubeClient = client.NewWithClientsAndConfig(mockKube, nil, nil)
126+
cliCtx = context.NewCLIContext(context.NewCLIConfig(context.WithOutputWriter(output)))
132127
})
133128

134129
It("should show no nodes when none have the skyhook", func() {
@@ -141,8 +136,8 @@ var _ = Describe("Node List Command", func() {
141136
_, err := mockKube.CoreV1().Nodes().Create(gocontext.Background(), node, metav1.CreateOptions{})
142137
Expect(err).NotTo(HaveOccurred())
143138

144-
opts := &nodeListOptions{skyhookName: "my-skyhook", output: "table"}
145-
err = runNodeList(gocontext.Background(), output, kubeClient, opts)
139+
opts := &nodeListOptions{skyhookName: "my-skyhook"}
140+
err = runNodeList(gocontext.Background(), kubeClient, opts, cliCtx)
146141
Expect(err).NotTo(HaveOccurred())
147142
Expect(output.String()).To(ContainSubstring("No nodes found"))
148143
})
@@ -166,8 +161,8 @@ var _ = Describe("Node List Command", func() {
166161
Expect(err).NotTo(HaveOccurred())
167162
}
168163

169-
opts := &nodeListOptions{skyhookName: "my-skyhook", output: "table"}
170-
err := runNodeList(gocontext.Background(), output, kubeClient, opts)
164+
opts := &nodeListOptions{skyhookName: "my-skyhook"}
165+
err := runNodeList(gocontext.Background(), kubeClient, opts, cliCtx)
171166
Expect(err).NotTo(HaveOccurred())
172167

173168
outputStr := output.String()
@@ -209,8 +204,8 @@ var _ = Describe("Node List Command", func() {
209204
_, err = mockKube.CoreV1().Nodes().Create(gocontext.Background(), node2, metav1.CreateOptions{})
210205
Expect(err).NotTo(HaveOccurred())
211206

212-
opts := &nodeListOptions{skyhookName: "skyhook-a", output: "table"}
213-
err = runNodeList(gocontext.Background(), output, kubeClient, opts)
207+
opts := &nodeListOptions{skyhookName: "skyhook-a"}
208+
err = runNodeList(gocontext.Background(), kubeClient, opts, cliCtx)
214209
Expect(err).NotTo(HaveOccurred())
215210

216211
outputStr := output.String()
@@ -235,8 +230,9 @@ var _ = Describe("Node List Command", func() {
235230
_, err := mockKube.CoreV1().Nodes().Create(gocontext.Background(), node, metav1.CreateOptions{})
236231
Expect(err).NotTo(HaveOccurred())
237232

238-
opts := &nodeListOptions{skyhookName: "my-skyhook", output: "json"}
239-
err = runNodeList(gocontext.Background(), output, kubeClient, opts)
233+
opts := &nodeListOptions{skyhookName: "my-skyhook"}
234+
cliCtx.GlobalFlags.OutputFormat = utils.OutputFormatJSON
235+
err = runNodeList(gocontext.Background(), kubeClient, opts, cliCtx)
240236
Expect(err).NotTo(HaveOccurred())
241237

242238
var result struct {
@@ -291,8 +287,9 @@ var _ = Describe("Node List Command", func() {
291287
Expect(err).NotTo(HaveOccurred())
292288
}
293289

294-
opts := &nodeListOptions{skyhookName: "my-skyhook", output: "json"}
295-
err := runNodeList(gocontext.Background(), output, kubeClient, opts)
290+
opts := &nodeListOptions{skyhookName: "my-skyhook"}
291+
cliCtx.GlobalFlags.OutputFormat = utils.OutputFormatJSON
292+
err := runNodeList(gocontext.Background(), kubeClient, opts, cliCtx)
296293
Expect(err).NotTo(HaveOccurred())
297294

298295
var result struct {

‎operator/cmd/cli/app/node/node_reset.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,9 @@ func runNodeReset(ctx context.Context, cmd *cobra.Command, kubeClient *client.Cl
138138

139139
var nodeState v1alpha1.NodeState
140140
if err := json.Unmarshal([]byte(annotation), &nodeState); err != nil {
141+
if cliCtx.GlobalFlags.Verbose {
142+
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Warning: skipping node %q - invalid annotation: %v\n", nodeName, err)
143+
}
141144
continue
142145
}
143146

‎operator/cmd/cli/app/node/node_status.go‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,11 @@ const nodeStateAnnotationPrefix = v1alpha1.METADATA_PREFIX + "/nodeState_"
4040
// nodeStatusOptions holds the options for the node status command
4141
type nodeStatusOptions struct {
4242
skyhookName string
43-
output string
4443
}
4544

4645
// BindToCmd binds the options to the command flags
4746
func (o *nodeStatusOptions) BindToCmd(cmd *cobra.Command) {
4847
cmd.Flags().StringVar(&o.skyhookName, "skyhook", "", "Filter by Skyhook name")
49-
cmd.Flags().StringVarP(&o.output, "output", "o", "table", "Output format: table, json, yaml, wide")
5048
}
5149

5250
// NewStatusCmd creates the node status command
@@ -90,7 +88,7 @@ Node names can be exact matches or regex patterns.`,
9088
return fmt.Errorf("initializing kubernetes client: %w", err)
9189
}
9290

93-
return runNodeStatus(cmd.Context(), cmd.OutOrStdout(), kubeClient, args, opts)
91+
return runNodeStatus(cmd.Context(), kubeClient, args, opts, ctx)
9492
},
9593
}
9694

@@ -119,7 +117,8 @@ type nodeSkyhookPkgStatus struct {
119117
Image string `json:"image,omitempty"`
120118
}
121119

122-
func runNodeStatus(ctx context.Context, out io.Writer, kubeClient *client.Client, nodePatterns []string, opts *nodeStatusOptions) error {
120+
func runNodeStatus(ctx context.Context, kubeClient *client.Client, nodePatterns []string, opts *nodeStatusOptions, cliCtx *cliContext.CLIContext) error {
121+
out := cliCtx.Config().OutputWriter
123122
// Get all nodes
124123
nodeList, err := kubeClient.Kubernetes().CoreV1().Nodes().List(ctx, metav1.ListOptions{})
125124
if err != nil {
@@ -175,6 +174,9 @@ func runNodeStatus(ctx context.Context, out io.Writer, kubeClient *client.Client
175174

176175
var nodeState v1alpha1.NodeState
177176
if err := json.Unmarshal([]byte(annotationValue), &nodeState); err != nil {
177+
if cliCtx.GlobalFlags.Verbose {
178+
_, _ = fmt.Fprintf(cliCtx.Config().ErrorWriter, "Warning: skipping node %q skyhook %q - invalid annotation: %v\n", node.Name, skyhookName, err)
179+
}
178180
continue // Skip invalid annotations
179181
}
180182

@@ -243,12 +245,12 @@ func runNodeStatus(ctx context.Context, out io.Writer, kubeClient *client.Client
243245
}
244246

245247
// Output based on format
246-
switch opts.output {
247-
case "json":
248+
switch cliCtx.GlobalFlags.OutputFormat {
249+
case utils.OutputFormatJSON:
248250
return utils.OutputJSON(out, summaries)
249-
case "yaml":
251+
case utils.OutputFormatYAML:
250252
return utils.OutputYAML(out, summaries)
251-
case "wide":
253+
case utils.OutputFormatWide:
252254
return outputNodeStatusWide(out, summaries)
253255
default:
254256
return outputNodeStatusTable(out, summaries)
@@ -267,13 +269,6 @@ func nodeStatusTableConfig() utils.TableConfig[nodeSkyhookSummary] {
267269
fmt.Sprintf("%d/%d", s.PackagesComplete, s.PackagesTotal),
268270
}
269271
},
270-
WideHeaders: []string{"COMPLETE", "TOTAL"},
271-
WideExtract: func(s nodeSkyhookSummary) []string {
272-
return []string{
273-
fmt.Sprintf("%d", s.PackagesComplete),
274-
fmt.Sprintf("%d", s.PackagesTotal),
275-
}
276-
},
277272
}
278273
}
279274

0 commit comments

Comments
 (0)