fix: cloudbackup unit test

Signed-off-by: Shivanjan Chakravorty <[email protected]>

csi/controller.go

@@ -1034,7 +1034,8 @@ func (s *OsdCsiServer) createCloudBackup(
 	req *csi.CreateSnapshotRequest,
 ) (*csi.CreateSnapshotResponse, error) {
 	cloudBackupClient, err := s.getCloudBackupClient(ctx)
-	if err != nil {
+	cloudBackupDriverDisabled := sdk.IsErrorUnavailable(err)
+	if (err != nil && !cloudBackupDriverDisabled) || cloudBackupClient == nil {
 		return nil, err
 	}
 
@@ -1115,8 +1116,9 @@ func (s *OsdCsiServer) DeleteSnapshot(
 	req *csi.DeleteSnapshotRequest,
 ) (resp *csi.DeleteSnapshotResponse, err error) {
 	cloudBackupClient, err := s.getCloudBackupClient(ctx)
-	cloudBackupDriverUnavailable := sdk.IsErrorUnavailable(err)
-	if err != nil && !cloudBackupDriverUnavailable {
+	cloudBackupClientAvailable := cloudBackupClient != nil
+	cloudBackupDriverDisabled := sdk.IsErrorUnavailable(err)
+	if (err != nil && !cloudBackupDriverDisabled) || cloudBackupClient == nil {
 		return nil, err
 	}
 
@@ -1125,11 +1127,19 @@ func (s *OsdCsiServer) DeleteSnapshot(
 		return nil, status.Error(codes.InvalidArgument, "Snapshot id must be provided")
 	}
 
-	// Check if snapshot has been created but is in error state
-	backupStatus, err := cloudBackupClient.Status(ctx, &api.SdkCloudBackupStatusRequest{
-		TaskId: csiSnapshotID,
-	})
-	if sdk.IsErrorNotFound(err) || cloudBackupDriverUnavailable {
+	var backupStatus *api.SdkCloudBackupStatusResponse
+	if cloudBackupClientAvailable && !cloudBackupDriverDisabled {
+		// Check if snapshot has been created but is in error state
+		backupStatus, err = cloudBackupClient.Status(ctx, &api.SdkCloudBackupStatusRequest{
+			TaskId: csiSnapshotID,
+		})
+	}
+
+	isSnapshotIDPresentInCloud := true
+	if backupStatus != nil {
+		_, isSnapshotIDPresentInCloud = backupStatus.Statuses[csiSnapshotID]
+	}
+	if (sdk.IsErrorNotFound(err) && !cloudBackupDriverDisabled && cloudBackupClientAvailable) || !isSnapshotIDPresentInCloud {
 		resp, err = s.deleteLocalSnapshot(ctx, req)
 		return
 	}

csi/controller_test.go

@@ -3562,7 +3562,7 @@ func TestOsdCsiServer_CreateCloudSnapshot(t *testing.T) {
 			if ctx.Value("remote-client-error").(bool) {
 				err = mockErr
 			}
-			return nil, false, err
+			return nil, true, err
 		}).AnyTimes()
 
 	for _, tt := range tests {
@@ -3692,10 +3692,12 @@ func TestOsdCsiServer_DeleteCloudSnapshot(t *testing.T) {
 	mockRoundRobinBalancer.EXPECT().GetRemoteNodeConnection(gomock.Any()).DoAndReturn(
 		func(ctx context.Context) (*grpc.ClientConn, bool, error) {
 			var err error
+			var conn *grpc.ClientConn
 			if ctx.Value("remote-client-error").(bool) {
 				err = mockErr
+				conn = &grpc.ClientConn{}
 			}
-			return nil, false, err
+			return conn, true, err
 		}).AnyTimes()
 
 	for _, tt := range tests {
@@ -3710,6 +3712,7 @@ func TestOsdCsiServer_DeleteCloudSnapshot(t *testing.T) {
 			s := &OsdCsiServer{
 				specHandler: spec.NewSpecHandler(),
 				mu:          sync.Mutex{},
+
 				cloudBackupClient: func(cc grpc.ClientConnInterface) api.OpenStorageCloudBackupClient {
 					return mockCloudBackupClient
 				},

csi/csi.go

@@ -66,6 +66,8 @@ type OsdCsiServerConfig struct {
 	SdkPort            string
 	SchedulerName      string
 
+	CloudBackupClient func(cc grpc.ClientConnInterface) api.OpenStorageCloudBackupClient
+
 	// Name to be reported back to the CO. If not provided,
 	// the name will be in the format of <driver>.openstorage.org
 	CsiDriverName string
@@ -124,6 +126,14 @@ func NewOsdCsiServer(config *OsdCsiServerConfig) (grpcserver.Server, error) {
 		return nil, err
 	}
 
+	var cloudBackupClient func(cc grpc.ClientConnInterface) api.OpenStorageCloudBackupClient
+
+	if config.CloudBackupClient == nil {
+		cloudBackupClient = api.NewOpenStorageCloudBackupClient
+	} else {
+		cloudBackupClient = config.CloudBackupClient
+	}
+
 	return &OsdCsiServer{
 		specHandler:        spec.NewSpecHandler(),
 		GrpcServer:         gServer,
@@ -134,7 +144,7 @@ func NewOsdCsiServer(config *OsdCsiServerConfig) (grpcserver.Server, error) {
 		csiDriverName:      config.CsiDriverName,
 		allowInlineVolumes: config.EnableInlineVolumes,
 		roundRobinBalancer: config.RoundRobinBalancer,
-		cloudBackupClient:  api.NewOpenStorageCloudBackupClient,
+		cloudBackupClient:  cloudBackupClient,
 		config:             config,
 		autoRecoverStopCh:  make(chan struct{}),
 	}, nil
@@ -161,7 +171,16 @@ func (s *OsdCsiServer) getConn() (*grpc.ClientConn, error) {
 }
 
 func (s *OsdCsiServer) getRemoteConn(ctx context.Context) (*grpc.ClientConn, error) {
-	remoteConn, _, err := s.roundRobinBalancer.GetRemoteNodeConnection(ctx)
+	remoteConn, remote, err := s.roundRobinBalancer.GetRemoteNodeConnection(ctx)
+	if !remote {
+		clogger.WithContext(ctx).Infof("Remote connection not supported")
+		conn, err := s.getConn()
+		if nil != err {
+			clogger.WithContext(ctx).Infof("Remote connection not supported")
+			return nil, err
+		}
+		return conn, err
+	}
 	return remoteConn, err
 }
 

csi/csi_test.go

@@ -30,6 +30,7 @@ import (
 	"github.com/golang/mock/gomock"
 	"github.com/kubernetes-csi/csi-test/utils"
 	"github.com/libopenstorage/openstorage/api"
+	"github.com/libopenstorage/openstorage/api/mock"
 	"github.com/libopenstorage/openstorage/api/server/sdk"
 	"github.com/libopenstorage/openstorage/cluster"
 	clustermanager "github.com/libopenstorage/openstorage/cluster/manager"
@@ -49,6 +50,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 const (
@@ -74,6 +77,7 @@ type testServer struct {
 	server grpcserver.Server
 	m      *mockdriver.MockVolumeDriver
 	c      *mockcluster.MockCluster
+	cb     *mock.MockOpenStorageCloudBackupClient
 	mc     *gomock.Controller
 	sdk    *sdk.Server
 	port   string
@@ -160,9 +164,22 @@ func newTestServerWithConfig(t *testing.T, config *OsdCsiServerConfig) *testServ
 	tester.m = mockdriver.NewMockVolumeDriver(tester.mc)
 	tester.c = mockcluster.NewMockCluster(tester.mc)
 
+	// for CSI snapshot there happens to be a call to cloudbackups to check if the snapshot id requested
+	// is a cloud backup. the below code prevents it from crashing the osd-tests and pr-test
+	tester.cb = mock.NewMockOpenStorageCloudBackupClient(tester.mc)
+	tester.m.EXPECT().CloudBackupStatus(gomock.Any()).DoAndReturn(func(input *api.CloudBackupStatusRequest) (*api.CloudBackupStatusResponse, error) {
+		return nil, status.New(codes.NotFound, "MOCK ERR").Err()
+	}).AnyTimes()
+	tester.cb.EXPECT().Status(gomock.Any(), gomock.Any()).DoAndReturn(func(ctx context.Context, in *api.SdkCloudBackupStatusRequest, opts ...grpc.CallOption) (*api.SdkCloudBackupStatusResponse, error) {
+		return nil, status.New(codes.NotFound, "MOCK ERR").Err()
+	}).AnyTimes()
+
 	if config.Cluster == nil {
 		config.Cluster = tester.c
 	}
+	config.CloudBackupClient = func(cc grpc.ClientConnInterface) api.OpenStorageCloudBackupClient {
+		return tester.cb
+	}
 	config.RoundRobinBalancer = loadbalancer.NewNullBalancer()
 
 	setupMockDriver(tester, t)