Merge pull request #6497 from blackpiglet/6190_fix_part_2

Remove dependency of the legacy client code from pkg/cmd directory part 2

changelogs/unreleased/6497-blackpiglet

@@ -0,0 +1 @@
+Remove dependency of the legacy client code from pkg/cmd directory part 2

pkg/client/factory_test.go

@@ -141,6 +141,10 @@ func TestFactory(t *testing.T) {
 			kubebuilderClient, e := f.KubebuilderClient()
 			assert.Contains(t, e.Error(), fmt.Sprintf("Get \"%s/api?timeout=", test.expectedHost))
 			assert.Nil(t, kubebuilderClient)
+
+			kbClientWithWatch, e := f.KubebuilderWatchClient()
+			assert.Contains(t, e.Error(), fmt.Sprintf("Get \"%s/api?timeout=", test.expectedHost))
+			assert.Nil(t, kbClientWithWatch)
 		})
 	}
 }

pkg/cmd/cli/backup/download_test.go

@@ -27,22 +27,20 @@ import (
 	flag "github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 
 	"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/generated/clientset/versioned/scheme"
+	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 	veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec"
-
-	factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
 )
 
 func TestNewDownloadCommand(t *testing.T) {
 	// create a factory
 	f := &factorymocks.Factory{}
 
 	backupName := "backup-1"
-	kbclient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
+	kbclient := velerotest.NewFakeControllerRuntimeClient(t)
 	err := kbclient.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, backupName).Result())
 	require.NoError(t, err)
 	err = kbclient.Create(context.Background(), builder.ForBackup(cmdtest.VeleroNameSpace, "bk-to-be-download").Result())

pkg/cmd/cli/backup/get.go

@@ -21,6 +21,8 @@ import (
 
 	"github.com/spf13/cobra"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
+	kbclient "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,24 @@ 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 backups *api.BackupList
+			backups := new(api.BackupList)
 			if len(args) > 0 {
-				backups = new(api.BackupList)
 				for _, name := range args {
-					backup, err := veleroClient.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), name, metav1.GetOptions{})
+					backup := new(api.Backup)
+					err := kbClient.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: name}, backup)
 					cmd.CheckError(err)
 					backups.Items = append(backups.Items, *backup)
 				}
 			} else {
-				backups, err = veleroClient.VeleroV1().Backups(f.Namespace()).List(context.TODO(), listOptions)
+				parsedSelector, err := labels.Parse(listOptions.LabelSelector)
+				cmd.CheckError(err)
+				err = kbClient.List(context.TODO(), backups, &kbclient.ListOptions{
+					LabelSelector: parsedSelector,
+					Namespace:     f.Namespace(),
+				})
 				cmd.CheckError(err)
 			}
 

pkg/cmd/cli/backup/get_test.go

@@ -17,22 +17,22 @@ limitations under the License.
 package backup
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+	kbclient "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"
+	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 	veleroexec "github.com/vmware-tanzu/velero/pkg/util/exec"
-
-	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
-	factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
-	versionedmocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/mocks"
-	velerov1mocks "github.com/vmware-tanzu/velero/pkg/generated/clientset/versioned/typed/velero/v1/mocks"
 )
 
 func TestNewGetCommand(t *testing.T) {
@@ -41,18 +41,16 @@ func TestNewGetCommand(t *testing.T) {
 	// create a factory
 	f := &factorymocks.Factory{}
 
-	backups := &velerov1mocks.BackupInterface{}
-	veleroV1 := &velerov1mocks.VeleroV1Interface{}
-	client := &versionedmocks.Interface{}
-	bk := &velerov1api.Backup{}
-	bkList := &velerov1api.BackupList{}
+	client := velerotest.NewFakeControllerRuntimeClient(t)
 
-	backups.On("List", mock.Anything, mock.Anything).Return(bkList, nil)
-	backups.On("Get", mock.Anything, mock.Anything, mock.Anything).Return(bk, nil)
-	veleroV1.On("Backups", mock.Anything).Return(backups, nil)
-	client.On("VeleroV1").Return(veleroV1, nil)
-	f.On("Client").Return(client, nil)
-	f.On("Namespace").Return(mock.Anything)
+	for _, backupName := range args {
+		backup := builder.ForBackup(cmdtest.VeleroNameSpace, backupName).ObjectMeta(builder.WithLabels("abc", "abc")).Result()
+		err := client.Create(context.Background(), backup, &kbclient.CreateOptions{})
+		require.NoError(t, err)
+	}
+
+	f.On("KubebuilderClient").Return(client, nil)
+	f.On("Namespace").Return(cmdtest.VeleroNameSpace)
 
 	// create command
 	c := NewGetCommand(f, "velero backup get")
@@ -69,6 +67,28 @@ func TestNewGetCommand(t *testing.T) {
 	cmd := exec.Command(os.Args[0], []string{"-test.run=TestNewGetCommand"}...)
 	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag))
 	stdout, _, err := veleroexec.RunCommand(cmd)
+	require.NoError(t, err)
+
+	if err == nil {
+		output := strings.Split(stdout, "\n")
+		i := 0
+		for _, line := range output {
+			if strings.Contains(line, "New") {
+				i++
+			}
+		}
+		assert.Equal(t, len(args), i)
+	}
+
+	d := NewGetCommand(f, "velero backup get")
+	c.SetArgs([]string{"-l", "abc=abc"})
+	e = d.Execute()
+	assert.NoError(t, e)
+
+	cmd = exec.Command(os.Args[0], []string{"-test.run=TestNewGetCommand"}...)
+	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", cmdtest.CaptureFlag))
+	stdout, _, err = veleroexec.RunCommand(cmd)
+	require.NoError(t, err)
 
 	if err == nil {
 		output := strings.Split(stdout, "\n")
@@ -79,7 +99,5 @@ func TestNewGetCommand(t *testing.T) {
 			}
 		}
 		assert.Equal(t, len(args), i)
-		return
 	}
-	t.Fatalf("process ran with err %v, want backups by get()", err)
 }

pkg/cmd/cli/backup/logs.go

@@ -23,60 +23,94 @@ import (
 	"time"
 
 	"github.com/spf13/cobra"
+	"github.com/spf13/pflag"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	kbclient "sigs.k8s.io/controller-runtime/pkg/client"
 
 	velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	"github.com/vmware-tanzu/velero/pkg/client"
 	"github.com/vmware-tanzu/velero/pkg/cmd"
 	"github.com/vmware-tanzu/velero/pkg/cmd/util/downloadrequest"
 )
 
-func NewLogsCommand(f client.Factory) *cobra.Command {
+type LogsOptions struct {
+	Timeout               time.Duration
+	InsecureSkipTLSVerify bool
+	CaCertFile            string
+	Client                kbclient.Client
+	BackupName            string
+}
+
+func NewLogsOptions() LogsOptions {
 	config, err := client.LoadConfig()
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "WARNING: Error reading config file: %v\n", err)
 	}
 
-	timeout := time.Minute
-	insecureSkipTLSVerify := false
-	caCertFile := config.CACertFile()
+	return LogsOptions{
+		Timeout:               time.Minute,
+		InsecureSkipTLSVerify: false,
+		CaCertFile:            config.CACertFile(),
+	}
+}
+
+func (l *LogsOptions) BindFlags(flags *pflag.FlagSet) {
+	flags.DurationVar(&l.Timeout, "timeout", l.Timeout, "How long to wait to receive logs.")
+	flags.BoolVar(&l.InsecureSkipTLSVerify, "insecure-skip-tls-verify", l.InsecureSkipTLSVerify, "If true, the object store's TLS certificate will not be checked for validity. This is insecure and susceptible to man-in-the-middle attacks. Not recommended for production.")
+	flags.StringVar(&l.CaCertFile, "cacert", l.CaCertFile, "Path to a certificate bundle to use when verifying TLS connections.")
+}
+
+func (l *LogsOptions) Run(c *cobra.Command, f client.Factory) error {
+	backup := new(velerov1api.Backup)
+	err := l.Client.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: l.BackupName}, backup)
+	if apierrors.IsNotFound(err) {
+		return fmt.Errorf("backup %q does not exist", l.BackupName)
+	} else if err != nil {
+		return fmt.Errorf("error checking for backup %q: %v", l.BackupName, err)
+	}
+
+	switch backup.Status.Phase {
+	case velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed, velerov1api.BackupPhaseFailed, velerov1api.BackupPhaseWaitingForPluginOperations, velerov1api.BackupPhaseWaitingForPluginOperationsPartiallyFailed:
+		// terminal and waiting for plugin operations phases, do nothing.
+	default:
+		return fmt.Errorf("logs for backup %q are not available until it's finished processing, please wait "+
+			"until the backup has a phase of Completed or Failed and try again", l.BackupName)
+	}
+
+	err = downloadrequest.Stream(context.Background(), l.Client, f.Namespace(), l.BackupName, velerov1api.DownloadTargetKindBackupLog, os.Stdout, l.Timeout, l.InsecureSkipTLSVerify, l.CaCertFile)
+	return err
+}
+
+func (l *LogsOptions) Complete(args []string, f client.Factory) error {
+	if len(args) > 0 {
+		l.BackupName = args[0]
+	}
+
+	kbClient, err := f.KubebuilderClient()
+	if err != nil {
+		return err
+	}
+	l.Client = kbClient
+	return nil
+}
+
+func NewLogsCommand(f client.Factory) *cobra.Command {
+	l := NewLogsOptions()
 
 	c := &cobra.Command{
 		Use:   "logs BACKUP",
 		Short: "Get backup logs",
 		Args:  cobra.ExactArgs(1),
 		Run: func(c *cobra.Command, args []string) {
-			backupName := args[0]
-
-			veleroClient, err := f.Client()
+			err := l.Complete(args, f)
 			cmd.CheckError(err)
 
-			kbClient, err := f.KubebuilderClient()
-			cmd.CheckError(err)
-
-			backup, err := veleroClient.VeleroV1().Backups(f.Namespace()).Get(context.TODO(), backupName, metav1.GetOptions{})
-			if apierrors.IsNotFound(err) {
-				cmd.Exit("Backup %q does not exist.", backupName)
-			} else if err != nil {
-				cmd.Exit("Error checking for backup %q: %v", backupName, err)
-			}
-
-			switch backup.Status.Phase {
-			case velerov1api.BackupPhaseCompleted, velerov1api.BackupPhasePartiallyFailed, velerov1api.BackupPhaseFailed, velerov1api.BackupPhaseWaitingForPluginOperations, velerov1api.BackupPhaseWaitingForPluginOperationsPartiallyFailed:
-				// terminal and waiting for plugin operations phases, do nothing.
-			default:
-				cmd.Exit("Logs for backup %q are not available until it's finished processing. Please wait "+
-					"until the backup has a phase of Completed or Failed and try again.", backupName)
-			}
-
-			err = downloadrequest.Stream(context.Background(), kbClient, f.Namespace(), backupName, velerov1api.DownloadTargetKindBackupLog, os.Stdout, timeout, insecureSkipTLSVerify, caCertFile)
+			err = l.Run(c, f)
 			cmd.CheckError(err)
 		},
 	}
 
-	c.Flags().DurationVar(&timeout, "timeout", timeout, "How long to wait to receive logs.")
-	c.Flags().BoolVar(&insecureSkipTLSVerify, "insecure-skip-tls-verify", insecureSkipTLSVerify, "If true, the object store's TLS certificate will not be checked for validity. This is insecure and susceptible to man-in-the-middle attacks. Not recommended for production.")
-	c.Flags().StringVar(&caCertFile, "cacert", caCertFile, "Path to a certificate bundle to use when verifying TLS connections.")
+	l.BindFlags(c.Flags())
+
 	return c
 }