feat: Add more tests & fix issues found by them

internal/cli/kraft/cloud/instance/logs/logs.go

@@ -33,7 +33,6 @@ type LogOptions struct {
 	Follow   bool                  `local:"true" long:"follow" short:"f" usage:"Follow the logs of the instance every half second" default:"false"`
 	Metro    string                `noattribute:"true"`
 	NoPrefix bool                  `long:"no-prefix" usage:"When logging multiple machines, do not prefix each log line with the name"`
-	Prefix   string                `local:"true" long:"prefix" short:"p" usage:"Prefix the logs with a given string"`
 	Tail     int                   `local:"true" long:"tail" short:"n" usage:"Show the last given lines from the logs" default:"-1"`
 	Token    string                `noattribute:"true"`
 }

internal/cli/kraft/cloud/instance/start/start.go

@@ -34,7 +34,7 @@ func NewCmd() *cobra.Command {
 	cmd, err := cmdfactory.New(&StartOptions{}, cobra.Command{
 		Short:   "Start instances",
 		Use:     "start [FLAGS] [UUID|NAME [UUID|NAME]...]",
-		Args:    cobra.MinimumNArgs(1),
+		Args:    cobra.ArbitraryArgs,
 		Aliases: []string{"str"},
 		Example: heredoc.Doc(`
 			# Start an instance by UUID

internal/cli/kraft/cloud/instance/stop/stop.go

@@ -24,8 +24,8 @@ import (
 type StopOptions struct {
 	Auth         *config.AuthConfig    `noattribute:"true"`
 	Client       kraftcloud.KraftCloud `noattribute:"true"`
-	Wait         time.Duration         `local:"true" long:"wait" short:"w" usage:"Time to wait for the instance to drain all connections before it is stopped (ms/s/m/h)"`
-	DrainTimeout time.Duration         `local:"true" long:"drain-timeout" short:"d" usage:"Timeout for the instance to stop (ms/s/m/h)"`
+	Wait         time.Duration         `local:"true" long:"wait" short:"w" usage:"Timeout for the instance to stop (ms/s/m/h)"`
+	DrainTimeout time.Duration         `local:"true" long:"drain-timeout" short:"d" usage:"Time to wait for the instance to drain all connections before it is stopped (ms/s/m/h)"`
 	All          bool                  `long:"all" short:"a" usage:"Stop all instances"`
 	Force        bool                  `long:"force" short:"f" usage:"Force stop the instance(s)"`
 	Metro        string                `noattribute:"true"`

internal/cli/kraft/cloud/service/get/get.go

@@ -39,14 +39,17 @@ func NewCmd() *cobra.Command {
 	cmd, err := cmdfactory.New(&GetOptions{}, cobra.Command{
 		Short:   "Retrieve the state of services",
 		Use:     "get [FLAGS] UUID|NAME",
-		Args:    cobra.ExactArgs(1),
+		Args:    cobra.MinimumNArgs(1),
 		Aliases: []string{"gt"},
 		Example: heredoc.Doc(`
 			# Retrieve information about a kraftcloud service
 			$ kraft cloud service get fd1684ea-7970-4994-92d6-61dcc7905f2b
 
 			# Retrieve information about a kraftcloud service
 			$ kraft cloud service get my-service
+
+			# Retrieve information about two unikraft cloud services
+			$ kraft cloud service get my-service-1 my-service-2
 	`),
 		Annotations: map[string]string{
 			cmdfactory.AnnotationHelpGroup: "kraftcloud-svc",
@@ -82,9 +85,9 @@ func (opts *GetOptions) Run(ctx context.Context, args []string) error {
 		kraftcloud.WithToken(config.GetKraftCloudTokenAuthConfig(*auth)),
 	)
 
-	resp, err := client.WithMetro(opts.metro).Get(ctx, args[0])
+	resp, err := client.WithMetro(opts.metro).Get(ctx, args...)
 	if err != nil {
-		return fmt.Errorf("could not get service %s: %w", args[0], err)
+		return fmt.Errorf("could not get services: %w", err)
 	}
 
 	return utils.PrintServices(ctx, opts.Output, *resp)

internal/cli/kraft/cloud/service/list/list.go

@@ -21,7 +21,6 @@ import (
 
 type ListOptions struct {
 	Output string `long:"output" short:"o" usage:"Set output format. Options: table,yaml,json,list" default:"table"`
-	Watch  bool   `long:"watch" short:"w" usage:"After listing watch for changes."`
 
 	metro string
 	token string
@@ -39,9 +38,6 @@ func NewCmd() *cobra.Command {
 
 			# List all service in your account in full table format.
 			$ kraft cloud service list -o full
-
-			# List all service in your account and watch for changes.
-			$ kraft cloud service list -w
 		`),
 		Annotations: map[string]string{
 			cmdfactory.AnnotationHelpGroup: "kraftcloud-svc",

internal/cli/kraft/cloud/service/remove/remove.go

@@ -111,8 +111,17 @@ func Remove(ctx context.Context, opts *RemoveOptions, args ...string) error {
 		}
 
 		args = []string{}
+		attachedInstances := []string{}
 		for _, sgItem := range sgList {
-			args = append(args, sgItem.Name)
+			if sgItem.Instances != nil && len(sgItem.Instances) > 0 {
+				attachedInstances = append(attachedInstances, sgItem.Name)
+			} else {
+				args = append(args, sgItem.Name)
+			}
+		}
+
+		if len(attachedInstances) > 0 {
+			log.G(ctx).Warnf("ignoring %d service(s) as instances are attached: %v", len(attachedInstances), attachedInstances)
 		}
 	}
 

internal/cli/kraft/cloud/volume/get/get.go

@@ -39,7 +39,7 @@ func NewCmd() *cobra.Command {
 	cmd, err := cmdfactory.New(&GetOptions{}, cobra.Command{
 		Short:   "Retrieve the state of persistent volumes",
 		Use:     "get [FLAGS] UUID|NAME",
-		Args:    cobra.ExactArgs(1),
+		Args:    cobra.MinimumNArgs(1),
 		Aliases: []string{"gt"},
 		Example: heredoc.Doc(`
 			# Retrieve information about a kraftcloud volume by UUID
@@ -82,9 +82,9 @@ func (opts *GetOptions) Run(ctx context.Context, args []string) error {
 		kraftcloud.WithToken(config.GetKraftCloudTokenAuthConfig(*auth)),
 	)
 
-	resp, err := client.WithMetro(opts.metro).Get(ctx, args[0])
+	resp, err := client.WithMetro(opts.metro).Get(ctx, args...)
 	if err != nil {
-		return fmt.Errorf("could not get volume %s: %w", args[0], err)
+		return fmt.Errorf("could not get volume %v: %w", args, err)
 	}
 
 	return utils.PrintVolumes(ctx, opts.Output, *resp)

internal/cli/kraft/cloud/volume/import/import.go

@@ -13,18 +13,21 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"time"
 
 	"github.com/MakeNowJust/heredoc"
 	"github.com/dustin/go-humanize"
 	"github.com/spf13/cobra"
 
 	kraftcloud "sdk.kraft.cloud"
+	ukcinstances "sdk.kraft.cloud/instances"
 
 	"kraftkit.sh/cmdfactory"
 	"kraftkit.sh/config"
 	"kraftkit.sh/internal/cli/kraft/cloud/utils"
 	"kraftkit.sh/internal/fancymap"
 	"kraftkit.sh/iostreams"
+	"kraftkit.sh/log"
 	"kraftkit.sh/tui"
 	"kraftkit.sh/tui/paraprogress"
 	"kraftkit.sh/tui/processtree"
@@ -151,13 +154,14 @@ func importVolumeData(ctx context.Context, opts *ImportOptions) (retErr error) {
 
 	var authStr string
 	var instFQDN string
+	var instID string
 
 	paramodel, err = processTree(ctx, "Spawning temporary volume data import instance",
 		func(ctx context.Context) error {
 			if authStr, err = utils.GenRandAuth(); err != nil {
 				return fmt.Errorf("generating random authentication string: %w", err)
 			}
-			_, instFQDN, err = runVolimport(ctx, icli, opts.VolimportImage, volUUID, authStr, opts.Timeout)
+			instID, instFQDN, err = runVolimport(ctx, icli, opts.VolimportImage, volUUID, authStr, opts.Timeout)
 			return err
 		},
 	)
@@ -175,48 +179,94 @@ func importVolumeData(ctx context.Context, opts *ImportOptions) (retErr error) {
 	var freeSpace uint64
 	var totalSpace uint64
 
-	paraprogress, err := paraProgress(ctx, fmt.Sprintf("Importing data (%s)", humanize.IBytes(uint64(cpioSize))),
-		func(ctx context.Context, callback func(float64)) (retErr error) {
-			instAddr := instFQDN + ":" + strconv.FormatUint(uint64(volimportPort), 10)
-			conn, err := tls.Dial("tcp4", instAddr, nil)
-			if err != nil {
-				return fmt.Errorf("connecting to volume data import instance send port: %w", err)
-			}
-			defer conn.Close()
-
-			ctx, cancel := context.WithCancel(ctx)
-			defer cancel()
-			freeSpace, totalSpace, err = copyCPIO(ctx, conn, authStr, cpioPath, opts.Force, uint64(cpioSize), callback)
-			copyCPIOErr = err
+	if log.LoggerTypeFromString(config.G[config.KraftKit](ctx).Log.Type) == log.FANCY {
+		paraprogress, err := paraProgress(ctx, fmt.Sprintf("Importing data (%s)", humanize.IBytes(uint64(cpioSize))),
+			func(ctx context.Context, callback func(float64)) (retErr error) {
+				instAddr := instFQDN + ":" + strconv.FormatUint(uint64(volimportPort), 10)
+				conn, err := tls.Dial("tcp4", instAddr, nil)
+				if err != nil {
+					return fmt.Errorf("connecting to volume data import instance send port: %w", err)
+				}
+				defer conn.Close()
+
+				ctx, cancel := context.WithCancel(ctx)
+				defer cancel()
+				freeSpace, totalSpace, err = copyCPIO(ctx, conn, authStr, cpioPath, opts.Force, uint64(cpioSize), callback)
+				copyCPIOErr = err
+				return err
+			},
+		)
+		if err != nil {
 			return err
-		},
-	)
-	if err != nil {
-		return err
-	}
-	if err = paraprogress.Start(); err != nil {
+		}
+		if err = paraprogress.Start(); err != nil {
+			return err
+		}
+	} else {
+		instAddr := instFQDN + ":" + strconv.FormatUint(uint64(volimportPort), 10)
+		conn, err := tls.Dial("tcp4", instAddr, nil)
+		if err != nil {
+			return fmt.Errorf("connecting to volume data import instance send port: %w", err)
+		}
+		defer conn.Close()
+
+		ctx, cancel := context.WithCancel(ctx)
+		defer cancel()
+		freeSpace, totalSpace, err = copyCPIO(ctx, conn, authStr, cpioPath, opts.Force, uint64(cpioSize), func(progress float64) {})
+		copyCPIOErr = err
 		return err
 	}
 	if copyCPIOErr != nil {
 		return copyCPIOErr
 	}
 
-	fancymap.PrintFancyMap(iostreams.G(ctx).Out, tui.TextGreen, "Import complete",
-		fancymap.FancyMapEntry{
-			Key:   "volume",
-			Value: opts.VolID,
-		},
-		fancymap.FancyMapEntry{
-			Key:   "free",
-			Value: humanize.IBytes(freeSpace),
-		},
-		fancymap.FancyMapEntry{
-			Key:   "total",
-			Value: humanize.IBytes(totalSpace),
-		},
-	)
+	if log.LoggerTypeFromString(config.G[config.KraftKit](ctx).Log.Type) == log.FANCY {
+		fancymap.PrintFancyMap(iostreams.G(ctx).Out, tui.TextGreen, "Import complete",
+			fancymap.FancyMapEntry{
+				Key:   "volume",
+				Value: opts.VolID,
+			},
+			fancymap.FancyMapEntry{
+				Key:   "free",
+				Value: humanize.IBytes(freeSpace),
+			},
+			fancymap.FancyMapEntry{
+				Key:   "total",
+				Value: humanize.IBytes(totalSpace),
+			},
+		)
+	} else {
+		log.G(ctx).
+			WithField("volume", opts.VolID).
+			WithField("free", humanize.IBytes(freeSpace)).
+			WithField("total", humanize.IBytes(totalSpace)).
+			Info("Import complete")
+	}
 
-	return nil
+	// Stopping time can be anywhere between 1-1000ms, so we set a 1100ms timeout
+	waitResp, err := cli.Instances().WithMetro(opts.Metro).Wait(ctx, ukcinstances.StateStopped, 1100, instID)
+	if err != nil {
+		return fmt.Errorf("waiting for volume data import instance to stop: %w", err)
+	}
+
+	if w, err := waitResp.FirstOrErr(); err == nil {
+		if ukcinstances.State(w.State) == ukcinstances.StateRunning {
+			return fmt.Errorf("volume data import instance did not stop yet: %s", w.State)
+		} else {
+			// Wait a bit for the instance to be deleted
+			// NOTE(craciunoiuc): this should never be reached, but it's a safety net
+			time.Sleep(100 * time.Millisecond)
+			return nil
+		}
+	} else {
+		if strings.Contains(err.Error(), "No instance") {
+			return nil
+		} else if strings.Contains(err.Error(), "Operation timed out") {
+			return fmt.Errorf("timed out waiting for volume data import instance to stop: %w", err)
+		} else {
+			return fmt.Errorf("waiting for volume data import instance to stop: %w", err)
+		}
+	}
 }
 
 // processTree returns a TUI ProcessTree configured to run the given function

internal/cli/kraft/cloud/volume/list/list.go

@@ -21,7 +21,6 @@ import (
 
 type ListOptions struct {
 	Output string `long:"output" short:"o" usage:"Set output format. Options: table,yaml,json,list" default:"table"`
-	Watch  bool   `long:"watch" short:"w" usage:"After listing watch for changes."`
 
 	metro string
 	token string

test/e2e/cloud/cloud_suite_test.go

@@ -3,7 +3,7 @@
 // Licensed under the BSD-3-Clause License (the "License").
 // You may not use this file except in compliance with the License.
 
-package cli_test
+package cloud_test
 
 import (
 	"testing"

test/e2e/cloud/fixtures/import/Dockerfile

@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2024, Unikraft GmbH and The KraftKit Authors.
+# Licensed under the BSD-3-Clause License (the "License").
+# You may not use this file except in compliance with the License.
+
+FROM ubuntu:latest AS build
+
+RUN mkdir -p /tmp/build && \
+    echo "<p>Hello World!</p>" > /tmp/build/hello.txt
+
+FROM scratch AS import
+
+COPY --from=build /tmp/build/hello.txt /index.html

test/e2e/cloud/fixtures/import/cpio-large/index.html

@@ -0,0 +1 @@
+<p>Hello World!</p>

test/e2e/cloud/fixtures/import/cpio-large/soft-link-0

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-1

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-2

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-3

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-4

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-5

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-6

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-7

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-8

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio-large/soft-link-9

@@ -0,0 +1 @@
+index.html

test/e2e/cloud/fixtures/import/cpio/index.html

@@ -0,0 +1 @@
+<p>Hello World!</p>

test/e2e/cloud/img_list_test.go

@@ -3,7 +3,7 @@
 // Licensed under the BSD-3-Clause License (the "License").
 // You may not use this file except in compliance with the License.
 
-package cli_test
+package cloud_test
 
 import (
 	"fmt"

test/e2e/cloud/instance_create_test.go

@@ -3,7 +3,7 @@
 // Licensed under the BSD-3-Clause License (the "License").
 // You may not use this file except in compliance with the License.
 
-package cli_test
+package cloud_test
 
 import (
 	"crypto/rand"
@@ -756,6 +756,28 @@ var _ = Describe("kraft cloud instance create", func() {
 			)
 		})
 
+		AfterEach(func() {
+			// Remove the instance after the test
+			cleanCmd := fcmd.NewKraft(stdout, stderr, cfg.Path())
+			cleanCmd.Args = append(cleanCmd.Args,
+				"cloud", "instance", "delete",
+				"--log-level", "info",
+				"--log-type", "json",
+				instanceNameFull,
+			)
+
+			err := cleanCmd.Run()
+			time.Sleep(2 * time.Second)
+			if err != nil {
+				fmt.Print(cleanCmd.DumpError(stdout, stderr, err))
+			}
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(stderr.String()).To(BeEmpty())
+			Expect(stdout.String()).ToNot(BeEmpty())
+			Expect(stdout.String()).To(MatchRegexp(`removing 1 instance\(s\)`))
+		})
+
 		It("should not error out with an API error", func() {
 			err := cmd.Run()
 			time.Sleep(2 * time.Second)