Skip to content

Commit

Permalink
This commit incorporate review comments
Browse files Browse the repository at this point in the history
Signed-off-by: mittachaitu <sai.chaithanya@mayadata.io>
  • Loading branch information
mittachaitu committed Jun 3, 2020
1 parent d4d58a6 commit 17ec490
Show file tree
Hide file tree
Showing 9 changed files with 31 additions and 30 deletions.
2 changes: 1 addition & 1 deletion deploy/cstor-operator.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ spec:
protocol: TCP
targetPort: 5757
selector:
name: maya-apiserver
name: cvc-operator
sessionAffinity: None
---
apiVersion: apps/v1
Expand Down
2 changes: 1 addition & 1 deletion pkg/controllers/cstorvolumeconfig/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ func setupCVCServer(k8sclientset kubernetes.Interface, openebsClientset clientse
cvcServer := cvcserver.NewCVCServer(config, os.Stdout).
WithOpenebsClientSet(openebsClientset).
WithKubernetesClientSet(k8sclientset).
WithSnapshoter(&snapshot.SnapClient{})
WithSnapshotter(&snapshot.SnapClient{})

// Setup the HTTP server
http, err := cvcserver.NewHTTPServer(cvcServer)
Expand Down
9 changes: 5 additions & 4 deletions pkg/server/cstorvolumeconfig/backup_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ type backupAPIOps struct {
resp http.ResponseWriter
k8sclientset kubernetes.Interface
clientset clientset.Interface
snapshoter snapshot.Snapshoter
snapshotter snapshot.Snapshotter
}

var (
Expand All @@ -59,7 +59,7 @@ func (s *HTTPServer) backupV1alpha1SpecificRequest(resp http.ResponseWriter, req
resp: resp,
k8sclientset: s.cvcServer.kubeclientset,
clientset: s.cvcServer.clientset,
snapshoter: s.cvcServer.snapshoter,
snapshotter: s.cvcServer.snapshotter,
}

switch req.Method {
Expand Down Expand Up @@ -87,14 +87,15 @@ func (bOps *backupAPIOps) create() (interface{}, error) {
if err := backupCreateRequestValidations(backUp); err != nil {
return nil, err
}
klog.Infof("Requested to create backup for volume %s/%s remoteBackup: %t", backUp.Namespace, backUp.Spec.VolumeName, !backUp.Spec.LocalSnap)

// TODO: Move this to interface so that we can mock
// snapshot calls
snapshot := snapshot.Snapshot{
VolumeName: backUp.Spec.VolumeName,
SnapshotName: backUp.Spec.SnapName,
Namespace: getOpenEBSNamespace(),
SnapClient: bOps.snapshoter,
SnapClient: bOps.snapshotter,
}
snapResp, err := snapshot.CreateSnapshot(bOps.clientset)
if err != nil {
Expand Down Expand Up @@ -270,7 +271,7 @@ func (bOps *backupAPIOps) deleteBackup(backup, volname, ns, schedule string) err
VolumeName: volname,
SnapshotName: backup,
Namespace: ns,
SnapClient: bOps.snapshoter,
SnapClient: bOps.snapshotter,
}
// Snapshot Name and backup name are same
_, err = snapshot.DeleteSnapshot(bOps.clientset)
Expand Down
22 changes: 11 additions & 11 deletions pkg/server/cstorvolumeconfig/https_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,8 @@ func TestBackupPostEndPoint(t *testing.T) {
cspcName string
// cstorBackup used to query on backup endpoint
cstorBackup *openebsapis.CStorBackup
// snapshoter is used to mock snapshot operations on volumes
snapshoter *snapshot.FakeSnapshoter
// snapshotter is used to mock snapshot operations on volumes
snapshotter *snapshot.FakeSnapshotter
// cvrStatus creates CVR with provided phase
cvrStatus cstor.CStorVolumeReplicaPhase
isScheduledBackup bool
Expand All @@ -265,7 +265,7 @@ func TestBackupPostEndPoint(t *testing.T) {
SnapName: "snapshot1",
},
},
snapshoter: &snapshot.FakeSnapshoter{},
snapshotter: &snapshot.FakeSnapshotter{},
cvrStatus: cstor.CVRStatusOnline,
expectedResponseCode: http.StatusOK,
verifyBackUpStatus: verifyExistenceOfPendingBackup,
Expand All @@ -284,7 +284,7 @@ func TestBackupPostEndPoint(t *testing.T) {
SnapName: "snapshot",
},
},
snapshoter: &snapshot.FakeSnapshoter{
snapshotter: &snapshot.FakeSnapshotter{
ShouldReturnFakeError: true,
},
expectedResponseCode: http.StatusBadRequest,
Expand All @@ -302,7 +302,7 @@ func TestBackupPostEndPoint(t *testing.T) {
BackupDest: "172.102.29.12:3234",
},
},
snapshoter: &snapshot.FakeSnapshoter{
snapshotter: &snapshot.FakeSnapshotter{
ShouldReturnFakeError: true,
},
expectedResponseCode: http.StatusBadRequest,
Expand All @@ -322,7 +322,7 @@ func TestBackupPostEndPoint(t *testing.T) {
SnapName: "snapshot",
},
},
snapshoter: &snapshot.FakeSnapshoter{},
snapshotter: &snapshot.FakeSnapshotter{},
expectedResponseCode: http.StatusBadRequest,
verifyBackUpStatus: backupShouldNotExist,
cvrStatus: cstor.CVRStatusOffline,
Expand All @@ -340,7 +340,7 @@ func TestBackupPostEndPoint(t *testing.T) {
SnapName: "snapshot4",
},
},
snapshoter: &snapshot.FakeSnapshoter{},
snapshotter: &snapshot.FakeSnapshotter{},
expectedResponseCode: http.StatusOK,
verifyBackUpStatus: backupShouldNotExist,
cvrStatus: cstor.CVRStatusOnline,
Expand All @@ -359,7 +359,7 @@ func TestBackupPostEndPoint(t *testing.T) {
LocalSnap: true,
},
},
snapshoter: &snapshot.FakeSnapshoter{},
snapshotter: &snapshot.FakeSnapshotter{},
expectedResponseCode: http.StatusOK,
verifyBackUpStatus: backupShouldNotExist,
cvrStatus: cstor.CVRStatusOffline,
Expand All @@ -377,7 +377,7 @@ func TestBackupPostEndPoint(t *testing.T) {
LocalSnap: true,
},
},
snapshoter: &snapshot.FakeSnapshoter{
snapshotter: &snapshot.FakeSnapshotter{
ShouldReturnFakeError: true,
},
expectedResponseCode: http.StatusBadRequest,
Expand Down Expand Up @@ -405,7 +405,7 @@ func TestBackupPostEndPoint(t *testing.T) {
cvcServer: NewCVCServer(server.DefaultServerConfig(), os.Stdout).
WithOpenebsClientSet(f.openebsClient).
WithKubernetesClientSet(f.k8sClient).
WithSnapshoter(test.snapshoter),
WithSnapshotter(test.snapshotter),
logger: log.New(os.Stdout, "", log.LstdFlags|log.Lmicroseconds),
}
//Marshal serializes the value provided into a json document
Expand Down Expand Up @@ -532,7 +532,7 @@ func TestBackupGetEndPoint(t *testing.T) {
cvcServer: NewCVCServer(server.DefaultServerConfig(), os.Stdout).
WithOpenebsClientSet(f.openebsClient).
WithKubernetesClientSet(f.k8sClient).
WithSnapshoter(&snapshot.FakeSnapshoter{}),
WithSnapshotter(&snapshot.FakeSnapshotter{}),
logger: log.New(os.Stdout, "", log.LstdFlags|log.Lmicroseconds),
}
//Marshal serializes the value provided into a json document
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/cstorvolumeconfig/restore_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func (s *HTTPServer) restoreV1alpha1SpecificRequest(
klog.Infof("Got restore GET request")
return restoreOps.get()
}
klog.Infof("please add a support for %s method in restore", req.Method)
klog.Infof("restore endpoint doesn't support %s", req.Method)
return nil, CodedError(405, ErrInvalidMethod)
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/server/cstorvolumeconfig/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ type CVCServer struct {
// clientset is a openebs custom resource package generated for custom API group.
clientset clientset.Interface

// snapshoter is used to perform snapshot operations on Volumes
snapshoter snapshot.Snapshoter
// snapshotter is used to perform snapshot operations on Volumes
snapshotter snapshot.Snapshotter
}

// NewCVCServer is used to create a new CVC server
Expand All @@ -70,9 +70,9 @@ func (cs *CVCServer) WithOpenebsClientSet(openebsClientSet clientset.Interface)
return cs
}

// WithSnapshoter sets the snapshoter with provided argument
func (cs *CVCServer) WithSnapshoter(snapshoter snapshot.Snapshoter) *CVCServer {
cs.snapshoter = snapshoter
// WithSnapshotter sets the snapshotter with provided argument
func (cs *CVCServer) WithSnapshotter(snapshotter snapshot.Snapshotter) *CVCServer {
cs.snapshotter = snapshotter
return cs
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/snapshot/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ type CommandStatus struct {
Response string `json:"response"`
}

// Snapshoter is used to perform snapshot operations on given volume
type Snapshoter interface {
// Snapshotter is used to perform snapshot operations on given volume
type Snapshotter interface {
CreateSnapshot(ip, volumeName, namesapce string) (*v1proto.VolumeSnapCreateResponse, error)
DestroySnapshot(ip, volumeName, namesapce string) (*v1proto.VolumeSnapDeleteResponse, error)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Snapshot struct {
VolumeName string
SnapshotName string
Namespace string
SnapClient Snapshoter
SnapClient Snapshotter
}

// CreateSnapshot creates snapshot for provided CStor Volume
Expand Down
8 changes: 4 additions & 4 deletions pkg/snapshot/snapshottest/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,21 @@ import (
"github.com/pkg/errors"
)

// FakeSnapshoter is used to mock the snapshot operations
type FakeSnapshoter struct {
// FakeSnapshotter is used to mock the snapshot operations
type FakeSnapshotter struct {
ShouldReturnFakeError bool
}

// CreateSnapshot mocks snapshot create operation
func (fs *FakeSnapshoter) CreateSnapshot(ip, volName, snapName string) (*v1proto.VolumeSnapCreateResponse, error) {
func (fs *FakeSnapshotter) CreateSnapshot(ip, volName, snapName string) (*v1proto.VolumeSnapCreateResponse, error) {
if fs.ShouldReturnFakeError {
return nil, errors.Errorf("injected fake errors during snapshot create operation")
}
return &v1proto.VolumeSnapCreateResponse{}, nil
}

//DestroySnapshot mocks snapshot delete operation
func (fs *FakeSnapshoter) DestroySnapshot(ip, volName, snapName string) (*v1proto.VolumeSnapDeleteResponse, error) {
func (fs *FakeSnapshotter) DestroySnapshot(ip, volName, snapName string) (*v1proto.VolumeSnapDeleteResponse, error) {
if fs.ShouldReturnFakeError {
return nil, errors.Errorf("injected fake errors during snapshot delete operation")
}
Expand Down

0 comments on commit 17ec490

Please sign in to comment.