diff --git a/csi/controller.go b/csi/controller.go index 1495d437a..880019ea0 100644 --- a/csi/controller.go +++ b/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 } diff --git a/csi/controller_test.go b/csi/controller_test.go index 7c3549868..3c8d95649 100644 --- a/csi/controller_test.go +++ b/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 }, diff --git a/csi/csi.go b/csi/csi.go index 467152223..ea139b7da 100644 --- a/csi/csi.go +++ b/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 .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 } diff --git a/csi/csi_test.go b/csi/csi_test.go index 6a1e7fc8a..08c92cd1f 100644 --- a/csi/csi_test.go +++ b/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)