review: judge lenses over the cli

cmd/image/build.go

@@ -33,7 +33,6 @@ func (o *buildImageOptions) run(ctx context.Context) error {
 
 	interactive := term.IsTerminal(int(os.Stdout.Fd()))
 	progress := map[string]int{}
-	p := 0
 	for {
 		msg, err := resp.Recv()
 		if errors.Is(err, io.EOF) {
@@ -50,19 +49,17 @@ func (o *buildImageOptions) run(ctx context.Context) error {
 			fmt.Print(msg.Stream)
 			if msg.Status == "finished" {
 				clear(progress)
-				p = 0
 			}
 		case msg.Status != "":
 			if msg.Id == "" {
 				fmt.Println(msg.Status)
 			} else {
 				data := fmt.Sprintf("%s: %s %s", msg.Id, msg.Status, msg.Progress)
 				if pos, ok := progress[msg.Id]; !ok {
-					progress[msg.Id] = p
+					progress[msg.Id] = len(progress)
 					fmt.Println(data)
-					p++
 				} else if interactive {
-					fmt.Printf(progressRewrite, p-pos, data)
+					fmt.Printf(progressRewrite, len(progress)-pos, data)
 				} else {
 					fmt.Println(data)
 				}

cmd/image/cache.go

@@ -13,20 +13,13 @@ import (
 )
 
 type cacheImageOptions struct {
-	client    corepb.CoreRPCClient
-	images    []string
-	podname   string
-	nodenames []string
+	client corepb.CoreRPCClient
+	opts   *corepb.CacheImageOptions
 }
 
 func (o *cacheImageOptions) run(ctx context.Context) error {
 	logger := log.WithFunc("image.cacheImageOptions.run")
-	opts := &corepb.CacheImageOptions{
-		Images:    o.images,
-		Podname:   o.podname,
-		Nodenames: o.nodenames,
-	}
-	resp, err := o.client.CacheImage(ctx, opts)
+	resp, err := o.client.CacheImage(ctx, o.opts)
 	if err != nil {
 		return err
 	}
@@ -52,10 +45,12 @@ func cmdImageCache(ctx context.Context, cmd *cli.Command) error {
 	}
 
 	o := &cacheImageOptions{
-		client:    client,
-		images:    images,
-		podname:   cmd.String(flagPod),
-		nodenames: cmd.StringSlice(flagNode),
+		client: client,
+		opts: &corepb.CacheImageOptions{
+			Images:    images,
+			Podname:   cmd.String(flagPod),
+			Nodenames: cmd.StringSlice(flagNode),
+		},
 	}
 	return o.run(ctx)
 }

cmd/image/remove.go

@@ -13,22 +13,13 @@ import (
 )
 
 type removeImageOptions struct {
-	client    corepb.CoreRPCClient
-	images    []string
-	podname   string
-	nodenames []string
-	prune     bool
+	client corepb.CoreRPCClient
+	opts   *corepb.RemoveImageOptions
 }
 
 func (o *removeImageOptions) run(ctx context.Context) error {
 	logger := log.WithFunc("image.removeImageOptions.run")
-	opts := &corepb.RemoveImageOptions{
-		Images:    o.images,
-		Podname:   o.podname,
-		Nodenames: o.nodenames,
-		Prune:     o.prune,
-	}
-	resp, err := o.client.RemoveImage(ctx, opts)
+	resp, err := o.client.RemoveImage(ctx, o.opts)
 	if err != nil {
 		return err
 	}
@@ -57,11 +48,13 @@ func cmdImageRemove(ctx context.Context, cmd *cli.Command) error {
 	}
 
 	o := &removeImageOptions{
-		client:    client,
-		images:    images,
-		podname:   cmd.String(flagPod),
-		nodenames: cmd.StringSlice(flagNode),
-		prune:     cmd.Bool("prune"),
+		client: client,
+		opts: &corepb.RemoveImageOptions{
+			Images:    images,
+			Podname:   cmd.String(flagPod),
+			Nodenames: cmd.StringSlice(flagNode),
+			Prune:     cmd.Bool("prune"),
+		},
 	}
 	return o.run(ctx)
 }

cmd/image/stream_test.go

@@ -114,7 +114,7 @@ func TestCacheImageReportsFailureReason(t *testing.T) {
 		client: &fakeImageClient{cache: &fakeStream[corepb.CacheImageMessage]{msgs: []*corepb.CacheImageMessage{
 			{Image: "app:v1", Nodename: "node1", Success: false, Message: "no such image"},
 		}}},
-		images: []string{"app:v1"},
+		opts: &corepb.CacheImageOptions{Images: []string{"app:v1"}},
 	}
 
 	err := o.run(t.Context())
@@ -128,7 +128,7 @@ func TestRemoveImageReportsFailure(t *testing.T) {
 		client: &fakeImageClient{remove: &fakeStream[corepb.RemoveImageMessage]{msgs: []*corepb.RemoveImageMessage{
 			{Image: "app:v1", Success: false},
 		}}},
-		images: []string{"app:v1"},
+		opts: &corepb.RemoveImageOptions{Images: []string{"app:v1"}},
 	}
 
 	err := o.run(t.Context())
@@ -168,14 +168,14 @@ func TestImageCommandsPassANodeOnlyRequestThrough(t *testing.T) {
 		{
 			name: "cache",
 			run: func(ctx context.Context, c *fakeImageClient) error {
-				o := &cacheImageOptions{client: c, images: []string{"app:v1"}, nodenames: []string{"node1"}}
+				o := &cacheImageOptions{client: c, opts: &corepb.CacheImageOptions{Images: []string{"app:v1"}, Nodenames: []string{"node1"}}}
 				return o.run(ctx)
 			},
 		},
 		{
 			name: "remove",
 			run: func(ctx context.Context, c *fakeImageClient) error {
-				o := &removeImageOptions{client: c, images: []string{"app:v1"}, nodenames: []string{"node1"}}
+				o := &removeImageOptions{client: c, opts: &corepb.RemoveImageOptions{Images: []string{"app:v1"}, Nodenames: []string{"node1"}}}
 				return o.run(ctx)
 			},
 		},

cmd/lambda/run.go

@@ -19,7 +19,6 @@ var newline = []byte{'\n'}
 type runLambdaOptions struct {
 	client          corepb.CoreRPCClient
 	opts            *corepb.RunAndWaitOptions
-	stdin           bool
 	printWorkloadID bool
 }
 
@@ -52,7 +51,7 @@ func (o *runLambdaOptions) lambda(ctx context.Context) (int, error) {
 		_ = iStream.Send(newline)
 	}()
 
-	exitCount, stdin := int(o.opts.GetDeployOptions().GetCount()), o.stdin
+	exitCount, stdin := int(o.opts.GetDeployOptions().GetCount()), o.opts.GetDeployOptions().GetOpenStdin()
 	if o.opts.Async {
 		exitCount, stdin = 0, false
 	}
@@ -73,7 +72,6 @@ func cmdLambdaRun(ctx context.Context, cmd *cli.Command) error {
 	o := &runLambdaOptions{
 		client:          client,
 		opts:            opts,
-		stdin:           cmd.Bool("stdin"),
 		printWorkloadID: cmd.Bool("workload-id"),
 	}
 	return o.run(ctx)
@@ -86,11 +84,11 @@ func generateLambdaOptions(cmd *cli.Command) (*corepb.RunAndWaitOptions, error)
 
 	network := cmd.String("network")
 
-	memoryRequest, err := utils.ParseRAMInHuman(cmd.String("memory-request"))
+	memoryRequest, err := resourcetypes.ParseRAMInHuman(cmd.String("memory-request"))
 	if err != nil {
 		return nil, fmt.Errorf("parse memory-request: %w", err)
 	}
-	memoryLimit, err := utils.ParseRAMInHuman(cmd.String("memory"))
+	memoryLimit, err := resourcetypes.ParseRAMInHuman(cmd.String("memory"))
 	if err != nil {
 		return nil, fmt.Errorf("parse memory: %w", err)
 	}
@@ -111,11 +109,11 @@ func generateLambdaOptions(cmd *cli.Command) (*corepb.RunAndWaitOptions, error)
 		"memory-request": memoryRequest,
 		"memory-limit":   memoryLimit,
 	}
-	storageRequest, err := utils.ParseRAMInHuman(cmd.String("storage-request"))
+	storageRequest, err := resourcetypes.ParseRAMInHuman(cmd.String("storage-request"))
 	if err != nil {
 		return nil, fmt.Errorf("parse storage-request: %w", err)
 	}
-	storageLimit, err := utils.ParseRAMInHuman(cmd.String("storage"))
+	storageLimit, err := resourcetypes.ParseRAMInHuman(cmd.String("storage"))
 	if err != nil {
 		return nil, fmt.Errorf("parse storage: %w", err)
 	}

cmd/node/status.go

@@ -21,11 +21,13 @@ type setNodeStatusOptions struct {
 }
 
 func (o *setNodeStatusOptions) run(ctx context.Context) error {
+	err := o.heartbeat(ctx)
 	if o.interval == 0 {
-		return o.heartbeat(ctx)
+		return err
 	}
 
 	logger := log.WithFunc("node.setNodeStatusOptions.run")
+	logger.Error(ctx, err, "heartbeat")
 	ticker := time.NewTicker(time.Duration(o.interval) * time.Second)
 	defer ticker.Stop()
 

cmd/pod/capacity.go

@@ -70,12 +70,12 @@ func cmdPodCapacity(ctx context.Context, cmd *cli.Command) error {
 }
 
 func capacityResources(cmd *cli.Command) (map[string][]byte, error) {
-	memory, err := utils.ParseRAMInHuman(cmd.String(flagMemory))
+	memory, err := resourcetypes.ParseRAMInHuman(cmd.String(flagMemory))
 	if err != nil {
 		return nil, fmt.Errorf("parse memory: %w", err)
 	}
 
-	storage, err := utils.ParseRAMInHuman(cmd.String(flagStorage))
+	storage, err := resourcetypes.ParseRAMInHuman(cmd.String(flagStorage))
 	if err != nil {
 		return nil, fmt.Errorf("parse storage: %w", err)
 	}

cmd/pod/cmd.go

@@ -38,7 +38,6 @@ func Command() *cli.Command {
 					&cli.StringFlag{
 						Name:  "desc",
 						Usage: "description of pod",
-						Value: "",
 					},
 				},
 			},
@@ -92,13 +91,11 @@ func Command() *cli.Command {
 					&cli.BoolFlag{
 						Name:  "cpu-bind",
 						Usage: "bind cpu or not",
-						Value: false,
 					},
 					&cli.StringSliceFlag{
-						Name:     "node",
-						Aliases:  []string{"n"},
-						Usage:    "Specified the node(s) should join into the calculation. Could be specified multiple times with different names",
-						Required: false,
+						Name:    "node",
+						Aliases: []string{"n"},
+						Usage:   "Specified the node(s) should join into the calculation. Could be specified multiple times with different names",
 					},
 					utils.ExtraResourcesFlag(),
 				},

cmd/pod/nodes.go

@@ -48,14 +48,19 @@ func cmdPodListNodes(ctx context.Context, cmd *cli.Command) error {
 		return err
 	}
 
+	name := cmd.Args().First()
+	if name == "" {
+		return errors.New("pod name must be given")
+	}
+
 	filter := strings.ToLower(cmd.String("filter"))
 	if filter != up && filter != down && filter != all {
 		return errors.New("filter should be one of up/down/all")
 	}
 
 	o := &listPodNodesOptions{
 		client:          client,
-		name:            cmd.Args().First(),
+		name:            name,
 		filter:          filter,
 		labels:          utils.SplitEquality(cmd.StringSlice("label")),
 		timeoutInSecond: int32(cmd.Int("timeout")), //nolint:gosec