Improve pkg/cmd/cli package UT coverage.

pkg/cmd/cli/backup/create_test.go

@@ -162,7 +162,7 @@ func TestCreateCommand(t *testing.T) {
 		excludeClusterScopedResources := "MutatingWebhookConfiguration,APIService"
 		includeNamespaceScopedResources := "Endpoints,Event,PodTemplate"
 		excludeNamespaceScopedResources := "Secret,MultiClusterIngress"
-		labels := "c=foo,b=woo"
+		labels := "c=foo"
 		storageLocation := "bsl-name-1"
 		snapshotLocations := "region=minio"
 		selector := "a=pod"

pkg/cmd/cli/backup/logs_test.go

@@ -105,6 +105,8 @@ func TestNewLogsCommand(t *testing.T) {
 		require.Error(t, err)
 
 		require.Equal(t, fmt.Sprintf("backup \"%s\" does not exist", backupName), err.Error())
+
+		c.Execute()
 	})
 
 	t.Run("Normal backup log test", func(t *testing.T) {
@@ -143,4 +145,24 @@ func TestNewLogsCommand(t *testing.T) {
 		case <-done:
 		}
 	})
+
+	t.Run("Invalid client test", func(t *testing.T) {
+		// create a factory
+		f := &factorymocks.Factory{}
+
+		kbClient := velerotest.NewFakeControllerRuntimeClient(t)
+
+		f.On("Namespace").Return(cmdtest.VeleroNameSpace)
+
+		c := NewLogsCommand(f)
+		assert.Equal(t, "Get backup logs", c.Short)
+
+		l := NewLogsOptions()
+		flags := new(flag.FlagSet)
+		l.BindFlags(flags)
+
+		f.On("KubebuilderClient").Return(kbClient, fmt.Errorf("test error"))
+		err := l.Complete([]string{""}, f)
+		require.Equal(t, "test error", err.Error())
+	})
 }

pkg/cmd/cli/restore/create_test.go

@@ -18,7 +18,6 @@ package restore
 
 import (
 	"context"
-	"fmt"
 	"testing"
 	"time"
 
@@ -72,7 +71,7 @@ func TestCreateCommand(t *testing.T) {
 		scheduleName := "schedule1"
 		restoreVolumes := "true"
 		preserveNodePorts := "true"
-		labels := "c=foo,b=woo"
+		labels := "c=foo"
 		includeNamespaces := "app1,app2"
 		excludeNamespaces := "pod1,pod2,pod3"
 		existingResourcePolicy := "none"
@@ -146,7 +145,7 @@ func TestCreateCommand(t *testing.T) {
 
 	})
 
-	t.Run("create a restore create from schedule", func(t *testing.T) {
+	t.Run("create a restore from schedule", func(t *testing.T) {
 		f := &factorymocks.Factory{}
 		c := NewCreateCommand(f, "")
 		require.Equal(t, "Create a restore", c.Short)
@@ -157,18 +156,39 @@ func TestCreateCommand(t *testing.T) {
 		fromSchedule := "schedule-name-1"
 		flags.Parse([]string{"--from-schedule", fromSchedule})
 
-		fmt.Printf("debug, restore options: %+v\n", o)
-
 		kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch)
-
 		schedule := builder.ForSchedule(cmdtest.VeleroNameSpace, fromSchedule).Result()
-		kbclient.Create(context.Background(), schedule, &controllerclient.CreateOptions{})
+		require.NoError(t, kbclient.Create(context.Background(), schedule, &controllerclient.CreateOptions{}))
+		backup := builder.ForBackup(cmdtest.VeleroNameSpace, "test-backup").FromSchedule(schedule).Phase(velerov1api.BackupPhaseCompleted).Result()
+		require.NoError(t, kbclient.Create(context.Background(), backup, &controllerclient.CreateOptions{}))
 
 		f.On("Namespace").Return(cmdtest.VeleroNameSpace)
 		f.On("KubebuilderWatchClient").Return(kbclient, nil)
 
 		require.NoError(t, o.Complete(args, f))
-
+		require.NoError(t, o.Validate(c, []string{}, f))
 		require.NoError(t, o.Run(c, f))
 	})
+
+	t.Run("create a restore from not-existed backup", func(t *testing.T) {
+		f := &factorymocks.Factory{}
+		c := NewCreateCommand(f, "")
+		require.Equal(t, "Create a restore", c.Short)
+		flags := new(pflag.FlagSet)
+		o := NewCreateOptions()
+		o.BindFlags(flags)
+		nonExistedBackupName := "not-exist"
+
+		flags.Parse([]string{"--wait", "true"})
+		flags.Parse([]string{"--from-backup", nonExistedBackupName})
+
+		kbclient := velerotest.NewFakeControllerRuntimeClient(t).(kbclient.WithWatch)
+
+		f.On("Namespace").Return(cmdtest.VeleroNameSpace)
+		f.On("KubebuilderWatchClient").Return(kbclient, nil)
+
+		require.NoError(t, o.Complete(nil, f))
+		err := o.Validate(c, []string{}, f)
+		require.Equal(t, "backups.velero.io \"not-exist\" not found", err.Error())
+	})
 }

pkg/cmd/cli/restore/delete_test.go

@@ -0,0 +1,85 @@
+/*
+Copyright The Velero Contributors.
+
+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 restore
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"testing"
+
+	flag "github.com/spf13/pflag"
+	"github.com/stretchr/testify/require"
+	controllerclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/vmware-tanzu/velero/pkg/builder"
+	factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
+	"github.com/vmware-tanzu/velero/pkg/cmd/cli"
+	cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test"
+	velerotest "github.com/vmware-tanzu/velero/pkg/test"
+	veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec"
+)
+
+func TestDeleteCommand(t *testing.T) {
+	restore1 := "restore-name-1"
+	restore2 := "restore-name-2"
+
+	// create a factory
+	f := &factorymocks.Factory{}
+
+	client := velerotest.NewFakeControllerRuntimeClient(t)
+	client.Create(context.Background(), builder.ForRestore(cmdtest.VeleroNameSpace, restore1).Result(), &controllerclient.CreateOptions{})
+	client.Create(context.Background(), builder.ForRestore("default", restore2).Result(), &controllerclient.CreateOptions{})
+
+	f.On("KubebuilderClient").Return(client, nil)
+	f.On("Namespace").Return(cmdtest.VeleroNameSpace)
+
+	// create command
+	c := NewDeleteCommand(f, "velero restore delete")
+	c.SetArgs([]string{restore1, restore2})
+	require.Equal(t, "Delete restores", c.Short)
+
+	o := cli.NewDeleteOptions("restore")
+	flags := new(flag.FlagSet)
+	o.BindFlags(flags)
+	flags.Parse([]string{"--confirm"})
+
+	args := []string{restore1, restore2}
+
+	e := o.Complete(f, args)
+	require.Equal(t, nil, e)
+
+	e = o.Validate(c, f, args)
+	require.Equal(t, nil, e)
+
+	Run(o)
+
+	e = c.Execute()
+	require.Equal(t, nil, e)
+
+	if os.Getenv(cmdtest.CaptureFlag) == "1" {
+		return
+	}
+
+	cmd := exec.Command(os.Args[0], []string{"-test.run=TestDeleteCommand"}...)
+	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag))
+	stdout, _, err := veleroexec.RunCommand(cmd)
+	if err != nil {
+		require.Contains(t, stdout, fmt.Sprintf("restores.velero.io \"%s\" not found.", restore2))
+	}
+}

pkg/cmd/cli/restore/describe.go

@@ -23,6 +23,8 @@ import (
 
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	controllerclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/client"
@@ -48,34 +50,37 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
 		Use:   use + " [NAME1] [NAME2] [NAME...]",
 		Short: "Describe restores",
 		Run: func(c *cobra.Command, args []string) {
-			veleroClient, err := f.Client()
-			cmd.CheckError(err)
-
 			kbClient, err := f.KubebuilderClient()
 			cmd.CheckError(err)
 
-			var restores *velerov1api.RestoreList
+			restoreList := new(velerov1api.RestoreList)
 			if len(args) > 0 {
-				restores = new(velerov1api.RestoreList)
 				for _, name := range args {
-					restore, err := veleroClient.VeleroV1().Restores(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{})
+					restore := new(velerov1api.Restore)
+					err := kbClient.Get(context.TODO(), controllerclient.ObjectKey{Namespace: f.Namespace(), Name: name}, restore)
 					cmd.CheckError(err)
-					restores.Items = append(restores.Items, *restore)
+					restoreList.Items = append(restoreList.Items, *restore)
 				}
 			} else {
-				restores, err = veleroClient.VeleroV1().Restores(f.Namespace()).List(context.TODO(), listOptions)
+				parsedSelector, err := labels.Parse(listOptions.LabelSelector)
+				cmd.CheckError(err)
+
+				err = kbClient.List(context.TODO(), restoreList, &controllerclient.ListOptions{LabelSelector: parsedSelector, Namespace: f.Namespace()})
 				cmd.CheckError(err)
 			}
 
 			first := true
-			for i, restore := range restores.Items {
-				opts := newPodVolumeRestoreListOptions(restore.Name)
-				podvolumeRestoreList, err := veleroClient.VeleroV1().PodVolumeRestores(f.Namespace()).List(context.TODO(), opts)
+			for i, restore := range restoreList.Items {
+				podVolumeRestoreList := new(velerov1api.PodVolumeRestoreList)
+				err = kbClient.List(context.TODO(), podVolumeRestoreList, &controllerclient.ListOptions{
+					Namespace:     f.Namespace(),
+					LabelSelector: labels.SelectorFromSet(map[string]string{velerov1api.BackupNameLabel: label.GetValidName(restore.Name)}),
+				})
 				if err != nil {
 					fmt.Fprintf(os.Stderr, "error getting PodVolumeRestores for restore %s: %v\n", restore.Name, err)
 				}
 
-				s := output.DescribeRestore(context.Background(), kbClient, &restores.Items[i], podvolumeRestoreList.Items, details, insecureSkipTLSVerify, caCertFile)
+				s := output.DescribeRestore(context.Background(), kbClient, &restoreList.Items[i], podVolumeRestoreList.Items, details, insecureSkipTLSVerify, caCertFile)
 				if first {
 					first = false
 					fmt.Print(s)
@@ -94,11 +99,3 @@ func NewDescribeCommand(f client.Factory, use string) *cobra.Command {
 
 	return c
 }
-
-// newPodVolumeRestoreListOptions creates a ListOptions with a label selector configured to
-// find PodVolumeRestores for the restore identified by name.
-func newPodVolumeRestoreListOptions(name string) metav1.ListOptions {
-	return metav1.ListOptions{
-		LabelSelector: fmt.Sprintf("%s=%s", velerov1api.RestoreNameLabel, label.GetValidName(name)),
-	}
-}

pkg/cmd/cli/restore/describe_test.go

@@ -0,0 +1,75 @@
+/*
+Copyright The Velero Contributors.
+
+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 restore
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"k8s.io/client-go/rest"
+	controllerclient "sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/vmware-tanzu/velero/pkg/builder"
+	factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
+	cmdtest "github.com/vmware-tanzu/velero/pkg/cmd/test"
+	"github.com/vmware-tanzu/velero/pkg/features"
+	"github.com/vmware-tanzu/velero/pkg/test"
+	veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec"
+)
+
+func TestNewDescribeCommand(t *testing.T) {
+	// create a factory
+	f := &factorymocks.Factory{}
+	restoreName := "restore-describe-1"
+	testRestore := builder.ForRestore(cmdtest.VeleroNameSpace, restoreName).Result()
+
+	clientConfig := rest.Config{}
+	kbClient := test.NewFakeControllerRuntimeClient(t)
+	kbClient.Create(context.Background(), testRestore, &controllerclient.CreateOptions{})
+
+	f.On("ClientConfig").Return(&clientConfig, nil)
+	f.On("Namespace").Return(cmdtest.VeleroNameSpace)
+	f.On("KubebuilderClient").Return(kbClient, nil)
+
+	// create command
+	c := NewDescribeCommand(f, "velero restore describe")
+	assert.Equal(t, "Describe restores", c.Short)
+
+	features.NewFeatureFlagSet("EnableCSI")
+	defer features.NewFeatureFlagSet()
+
+	c.SetArgs([]string{restoreName})
+	e := c.Execute()
+	assert.NoError(t, e)
+
+	if os.Getenv(cmdtest.CaptureFlag) == "1" {
+		return
+	}
+	cmd := exec.Command(os.Args[0], []string{"-test.run=TestNewDescribeCommand"}...)
+	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag))
+	stdout, _, err := veleroexec.RunCommand(cmd)
+
+	if err == nil {
+		assert.Contains(t, stdout, fmt.Sprintf("Name:         %s", restoreName))
+		return
+	}
+	t.Fatalf("process ran with err %v, want backups by get()", err)
+}

pkg/cmd/cli/restore/get.go

@@ -21,6 +21,8 @@ import (
 
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	controllerclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/client"
@@ -38,19 +40,22 @@ func NewGetCommand(f client.Factory, use string) *cobra.Command {
 			err := output.ValidateFlags(c)
 			cmd.CheckError(err)
 
-			veleroClient, err := f.Client()
+			kbClient, err := f.KubebuilderClient()
 			cmd.CheckError(err)
 
-			var restores *api.RestoreList
+			restores := new(api.RestoreList)
 			if len(args) > 0 {
-				restores = new(api.RestoreList)
 				for _, name := range args {
-					restore, err := veleroClient.VeleroV1().Restores(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{})
+					restore := new(api.Restore)
+					err := kbClient.Get(context.TODO(), controllerclient.ObjectKey{Namespace: f.Namespace(), Name: name}, restore)
 					cmd.CheckError(err)
 					restores.Items = append(restores.Items, *restore)
 				}
 			} else {
-				restores, err = veleroClient.VeleroV1().Restores(f.Namespace()).List(context.TODO(), listOptions)
+				parsedSelector, err := labels.Parse(listOptions.LabelSelector)
+				cmd.CheckError(err)
+
+				err = kbClient.List(context.TODO(), restores, &controllerclient.ListOptions{LabelSelector: parsedSelector, Namespace: f.Namespace()})
 				cmd.CheckError(err)
 			}