From 2df74bb58eca13c1e093149930364ba34e460c86 Mon Sep 17 00:00:00 2001 From: mittachaitu Date: Wed, 30 Oct 2019 00:09:52 +0530 Subject: [PATCH 1/4] feat(cstor-volume-mgmt): add replica scaledown support Signed-off-by: mittachaitu --- Gopkg.lock | 9 ++ .../volumereplica/volumereplica.go | 3 +- .../controller/volume-controller/handler.go | 117 ++++++++++++++++-- .../new_volume_controller.go | 2 - cmd/cstor-volume-mgmt/volume/volume.go | 38 +++--- pkg/cstor/volume/v1alpha1/cstorvolume.go | 59 ++++++++- pkg/util/fileoperator.go | 9 +- pkg/util/fileoperator_test.go | 53 ++++++-- 8 files changed, 244 insertions(+), 46 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index c28a2800bd..4a6cda8960 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -95,6 +95,14 @@ revision = "0ca988a254f991240804bf9821f3450d87ccbb1b" version = "v1.3.0" +[[projects]] + branch = "master" + digest = "1:1ba1d79f2810270045c328ae5d674321db34e3aae468eb4233883b473c5c0467" + name = "github.com/golang/glog" + packages = ["."] + pruneopts = "UT" + revision = "23def4e6c14b4da8ac2ed8007337bc5eb5007998" + [[projects]] branch = "master" digest = "1:b7cb6054d3dff43b38ad2e92492f220f57ae6087ee797dca298139776749ace8" @@ -1090,6 +1098,7 @@ "github.com/Masterminds/sprig", "github.com/docker/go-units", "github.com/ghodss/yaml", + "github.com/golang/glog", "github.com/golang/protobuf/proto", "github.com/google/gofuzz", "github.com/hashicorp/hcl", diff --git a/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go b/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go index 8e3922537e..28175f1afd 100644 --- a/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go +++ b/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go @@ -18,11 +18,12 @@ package volumereplica import ( "fmt" - "github.com/openebs/maya/pkg/alertlog" "os" "strings" "time" + "github.com/openebs/maya/pkg/alertlog" + "encoding/json" apis "github.com/openebs/maya/pkg/apis/openebs.io/v1alpha1" diff --git a/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go b/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go index 44e4aa7d83..f98ad9dec0 100644 --- a/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go +++ b/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go @@ -484,7 +484,16 @@ func (c *CStorVolumeController) addResizeConditions( // configuration on success updates istgt.conf file func (c *CStorVolumeController) updateCStorVolumeDRF( cStorVolume *apis.CStorVolume) { - err := volume.ExecuteDesiredReplicationFactorCommand(cStorVolume) + // make changes to copyCV instead of cStorVolume + copyCV := cStorVolume.DeepCopy() + var err error + if len(copyCV.Spec.ReplicaDetails.KnownReplicas) < + len(copyCV.Status.ReplicaDetails.KnownReplicas) { + copyCV, err = c.triggerScaleDownProcess(copyCV) + } else if len(copyCV.Spec.ReplicaDetails.KnownReplicas) == + len(copyCV.Status.ReplicaDetails.KnownReplicas) { + copyCV, err = c.triggerScaleUpProcess(copyCV) + } if err != nil { c.recorder.Event(cStorVolume, corev1.EventTypeWarning, @@ -493,6 +502,27 @@ func (c *CStorVolumeController) updateCStorVolumeDRF( " Error: %v", cStorVolume.Spec.DesiredReplicationFactor, err, ), ) + return + } + c.recorder.Event(cStorVolume, + corev1.EventTypeNormal, + string(common.SuccessUpdated), + fmt.Sprintf("Successfully updated the desiredReplicationFactor to %d", + cStorVolume.Spec.DesiredReplicationFactor), + ) + cStorVolume = copyCV +} + +// triggerScaleUpProcess returns error incase of any error during scaleup +// process else it will return cstorvolume object +func (c *CStorVolumeController) triggerScaleUpProcess( + cStorVolume *apis.CStorVolume) (*apis.CStorVolume, error) { + err := volume.ExecuteDesiredReplicationFactorCommand( + cStorVolume, + volume.GetScaleUpCommand, + ) + if err != nil { + return nil, err } fileOperator := util.RealFileOperator{} updatedConfig := fmt.Sprintf("%s %d", @@ -506,20 +536,81 @@ func (c *CStorVolumeController) updateCStorVolumeDRF( ) cstorvolume_v1alpha1.ConfFileMutex.Unlock() if err != nil { - c.recorder.Event(cStorVolume, - corev1.EventTypeWarning, - string(common.FailureUpdate), - fmt.Sprintf("failed to update confile with desired replication factor %d"+ - " Error: %v", cStorVolume.Spec.DesiredReplicationFactor, err, - ), - ) + return nil, pkg_errors.Wrapf(err, "failed to update conf file with "+ + "desiredReplicationFactor %d", cStorVolume.Spec.DesiredReplicationFactor) } - c.recorder.Event(cStorVolume, - corev1.EventTypeNormal, - string(common.SuccessUpdated), - fmt.Sprintf("Successfully updated the desiredReplicationFactor to %d", - cStorVolume.Spec.DesiredReplicationFactor), + return cStorVolume, nil +} + +// triggerScaleDownProcess returns error incase of any error during scaledown +// process else it will return cstorvolume object. Following steps are executed +// 1. Verify are all the replicas are healthy other than removing replica. +// 2. If step1 is passed then update the istgt.conf file with latest +// information. +// 3. Update cStorVolume CR(replicationFactor, ConsistencyFactor and known +// replica list. +// 4. Trigger istgtcontrol command (istgtcontrol drf +// +// In triggerScaleDownProcess cStorVolumeAPI is used to access fields in CV +// object and cStorvolume is used to access methods of cStorVolume +func (c *CStorVolumeController) triggerScaleDownProcess( + cStorVolumeAPI *apis.CStorVolume) (*apis.CStorVolume, error) { + volumeStatus, err := volume.GetVolumeStatus(cStorVolumeAPI) + if err != nil { + return nil, err + } + if common.CStorVolumeStatus(volumeStatus.Status) != common.CVStatusHealthy { + return nil, pkg_errors.Errorf("cStorvolume is not in healthy to trigger scaledown") + } + cStorVolume := cstorvolume_v1alpha1.NewForAPIObject(cStorVolumeAPI) + if !cStorVolume.AreSpecReplicasHealthy(volumeStatus) { + return nil, pkg_errors.Errorf("spec replicas are not in healthy state") + } + replicaID := cStorVolume.GetRemovingReplicaID() + if replicaID == "" { + return nil, pkg_errors.Errorf("removing replica is not present in volume status") + } + if cStorVolumeAPI.Spec.DesiredReplicationFactor < cStorVolumeAPI.Spec.ReplicationFactor { + configData := cStorVolume.BuildScaleDownConfigData(replicaID) + fileOperator := util.RealFileOperator{} + cstorvolume_v1alpha1.ConfFileMutex.Lock() + err := fileOperator.UpdateOrAppendMultipleLines( + cstorvolume_v1alpha1.IstgtConfPath, + configData, + 0644) + cstorvolume_v1alpha1.ConfFileMutex.Unlock() + if err != nil { + return nil, pkg_errors.Wrapf(err, "failed to update istgt conf file %v", configData) + } + cStorVolumeAPI.Spec.ReplicationFactor = cStorVolumeAPI.Spec.DesiredReplicationFactor + cStorVolumeAPI.Spec.ConsistencyFactor = (cStorVolumeAPI.Spec.ReplicationFactor)/2 + 1 + cStorVolumeAPI, err = c.clientset. + OpenebsV1alpha1(). + CStorVolumes(cStorVolumeAPI.Namespace). + Update(cStorVolumeAPI) + if err != nil { + return nil, pkg_errors.Wrapf(err, "failed to update cstorvolume") + } + } + err = volume.ExecuteDesiredReplicationFactorCommand( + cStorVolumeAPI, + volume.GetScaleDownCommand, ) + if err != nil { + return nil, err + } + cStorVolumeAPI.Status.ReplicaDetails.KnownReplicas = + cStorVolumeAPI.Spec.ReplicaDetails.KnownReplicas + cStorVolumeAPI, err = c.clientset. + OpenebsV1alpha1(). + CStorVolumes(cStorVolumeAPI.Namespace). + Update(cStorVolumeAPI) + if err != nil { + return nil, pkg_errors.Wrapf(err, + "failed to update cstorvolume status with scaledown replica information", + ) + } + return cStorVolumeAPI, nil } // resizeCStorVolume resize the cstorvolume and if any error occurs updates the diff --git a/cmd/cstor-volume-mgmt/controller/volume-controller/new_volume_controller.go b/cmd/cstor-volume-mgmt/controller/volume-controller/new_volume_controller.go index c856e237cc..55d44493c6 100644 --- a/cmd/cstor-volume-mgmt/controller/volume-controller/new_volume_controller.go +++ b/cmd/cstor-volume-mgmt/controller/volume-controller/new_volume_controller.go @@ -126,11 +126,9 @@ func NewCStorVolumeController( } else if IsDestroyEvent(newCStorVolume) { q.Operation = common.QOpDestroy klog.Infof("cStorVolume Destroy event : %v, %v", newCStorVolume.ObjectMeta.Name, string(newCStorVolume.ObjectMeta.UID)) - controller.recorder.Event(newCStorVolume, corev1.EventTypeNormal, string(common.SuccessSynced), string(common.MessageDestroySynced)) } else { q.Operation = common.QOpModify klog.Infof("cStorVolume Modify event : %v, %v", newCStorVolume.ObjectMeta.Name, string(newCStorVolume.ObjectMeta.UID)) - controller.recorder.Event(newCStorVolume, corev1.EventTypeNormal, string(common.SuccessSynced), string(common.MessageModifySynced)) } controller.enqueueCStorVolume(newCStorVolume, q) }, diff --git a/cmd/cstor-volume-mgmt/volume/volume.go b/cmd/cstor-volume-mgmt/volume/volume.go index 61dd8ccef0..e8d3bdbef9 100644 --- a/cmd/cstor-volume-mgmt/volume/volume.go +++ b/cmd/cstor-volume-mgmt/volume/volume.go @@ -281,11 +281,13 @@ func ResizeTargetVolume(cStorVolume *apis.CStorVolume) error { return nil } -//ExecuteDesiredReplicationFactorCommand executes istgtcontrol command to update -//desired replication factor -func ExecuteDesiredReplicationFactorCommand(cStorVolume *apis.CStorVolume) error { +// ExecuteDesiredReplicationFactorCommand executes istgtcontrol command to update +// desired replication factor +func ExecuteDesiredReplicationFactorCommand( + cStorVolume *apis.CStorVolume, + getDRFCmd func(*apis.CStorVolume) string) error { // send desiredReplicationFactor command to istgt and read the response - drfCmd := getDRFCommand(cStorVolume) + drfCmd := getDRFCmd(cStorVolume) sockResp, err := UnixSockVar.SendCommand(drfCmd) if err != nil { return errors.Wrapf( @@ -307,16 +309,29 @@ func ExecuteDesiredReplicationFactorCommand(cStorVolume *apis.CStorVolume) error return nil } -//getResizeCommand will return data required to execute istgtcontrol drf -//command -//Ex command: drf -func getDRFCommand(cstorVolume *apis.CStorVolume) string { +// GetScaleUpCommand will return data required to execute istgtcontrol drf +// command +// Ex command: drf +func GetScaleUpCommand(cstorVolume *apis.CStorVolume) string { return fmt.Sprintf("%s %s %d", util.IstgtDRFCmd, cstorVolume.Name, cstorVolume.Spec.DesiredReplicationFactor, ) } +// GetScaleDownCommand return replica scale down command +// Ex command: drf +func GetScaleDownCommand(cStorVolume *apis.CStorVolume) string { + cmd := fmt.Sprintf("%s %s %d ", util.IstgtDRFCmd, + cStorVolume.Name, + cStorVolume.Spec.DesiredReplicationFactor, + ) + for repID := range cStorVolume.Spec.ReplicaDetails.KnownReplicas { + cmd = cmd + fmt.Sprintf("%s ", repID) + } + return cmd +} + // getResizeCommand returns resize used to resize volumes // Ex command for resize: Resize volname 10G 10 30 func getResizeCommand(cstorVolume *apis.CStorVolume) string { @@ -349,13 +364,6 @@ func CheckValidVolume(cStorVolume *apis.CStorVolume) error { if cStorVolume.Spec.DesiredReplicationFactor == 0 { return fmt.Errorf("DesiredReplicationFactor cannot be zero") } - if cStorVolume.Spec.DesiredReplicationFactor < cStorVolume.Spec.ReplicationFactor { - return fmt.Errorf("DesiredReplicationFactor %d cannot be less "+ - "than ReplicationFactor %d", - cStorVolume.Spec.DesiredReplicationFactor, - cStorVolume.Spec.ReplicationFactor, - ) - } } if cStorVolume.Spec.ReplicationFactor == 0 { return fmt.Errorf("replicationFactor cannot be zero") diff --git a/pkg/cstor/volume/v1alpha1/cstorvolume.go b/pkg/cstor/volume/v1alpha1/cstorvolume.go index 3c2f465168..a0f6c22fd7 100644 --- a/pkg/cstor/volume/v1alpha1/cstorvolume.go +++ b/pkg/cstor/volume/v1alpha1/cstorvolume.go @@ -188,10 +188,15 @@ func (c *CStorVolume) IsDRFPending() bool { ) return false } - drfStringValue := fmt.Sprintf("%d", c.object.Spec.DesiredReplicationFactor) + drfStringValue := fmt.Sprintf(" %d", c.object.Spec.DesiredReplicationFactor) // gotConfig will have " DesiredReplicationFactor 3" and we will extract // numeric character from output - return !strings.HasSuffix(gotConfig, drfStringValue) + if !strings.HasSuffix(gotConfig, drfStringValue) { + return true + } + // reconciliation check for replica scaledown scenarion + return (len(c.object.Spec.ReplicaDetails.KnownReplicas) < + len(c.object.Status.ReplicaDetails.KnownReplicas)) } // GetCVCondition returns corresponding cstorvolume condition based argument passed @@ -215,6 +220,56 @@ func (c *CStorVolume) IsConditionPresent(condType apis.CStorVolumeConditionType) return false } +// AreSpecReplicasHealthy return true if all the spec replicas are in Healthy +// state else return false +func (c *CStorVolume) AreSpecReplicasHealthy(volStatus *apis.CVStatus) bool { + var isReplicaExist bool + var replicaInfo apis.ReplicaStatus + for _, replicaValue := range c.object.Spec.ReplicaDetails.KnownReplicas { + isReplicaExist = false + for _, replicaInfo = range volStatus.ReplicaStatuses { + if replicaInfo.ID == replicaValue { + isReplicaExist = true + break + } + } + if (isReplicaExist && replicaInfo.Mode != "Healthy") || !isReplicaExist { + return false + } + } + return true +} + +// GetRemovingReplicaID return replicaID that present in status but not in spec +func (c *CStorVolume) GetRemovingReplicaID() string { + for repID := range c.object.Status.ReplicaDetails.KnownReplicas { + // If known replica is not exist in spec but if it exist in status then + // user/operator selected that replica for removal + if _, isReplicaExist := + c.object.Spec.ReplicaDetails.KnownReplicas[repID]; !isReplicaExist { + return string(repID) + } + } + return "" +} + +// BuildScaleDownConfigData build data based on replica that needs to remove +func (c *CStorVolume) BuildScaleDownConfigData(repId string) map[string]string { + configData := map[string]string{} + newReplicationFactor := c.object.Spec.ReplicationFactor + newConsistencyFactor := (newReplicationFactor / 2) + 1 + key := fmt.Sprintf(" ReplicationFactor") + value := fmt.Sprintf(" ReplicationFactor %d", newReplicationFactor) + configData[key] = value + key = fmt.Sprintf(" ConsistencyFactor") + value = fmt.Sprintf(" ConsistencyFactor %d", newConsistencyFactor) + configData[key] = value + key = fmt.Sprintf(" Replica %s", repId) + value = fmt.Sprintf("") + configData[key] = value + return configData +} + // PredicateList holds a list of cstor volume // based predicates type PredicateList []Predicate diff --git a/pkg/util/fileoperator.go b/pkg/util/fileoperator.go index 46a47fdd60..74c23776a9 100644 --- a/pkg/util/fileoperator.go +++ b/pkg/util/fileoperator.go @@ -87,6 +87,8 @@ func (r RealFileOperator) Updatefile(fileName, updatedVal, searchString string, func (r RealFileOperator) UpdateOrAppendMultipleLines(fileName string, keyUpdateValue map[string]string, perm os.FileMode) error { var newLines []string + var index int + var line string buffer, err := ioutil.ReadFile(filepath.Clean(fileName)) if err != nil { return errors.Wrapf(err, "failed to read %s file", fileName) @@ -105,13 +107,18 @@ func (r RealFileOperator) UpdateOrAppendMultipleLines(fileName string, // will be doing after current blockers for key, updatedValue := range keyUpdateValue { found := false - for index, line := range newLines { + for index, line = range newLines { if strings.HasPrefix(line, key) { newLines[index] = updatedValue found = true break } } + // To remove particular line that matched with key + if found && updatedValue == "" { + newLines = append(newLines[:index], newLines[index+1:]...) + continue + } if found == false { newLines = append(newLines, updatedValue) } diff --git a/pkg/util/fileoperator_test.go b/pkg/util/fileoperator_test.go index 83436bd9de..ae83589505 100644 --- a/pkg/util/fileoperator_test.go +++ b/pkg/util/fileoperator_test.go @@ -96,12 +96,13 @@ var ( LUN0 Option XCOPY Disable Replica 6161 6061 Replica 6162 6062334 + Replica 7161 7061 ` ) var ( fakePath = "/tmp/istgt.conf" - defaultLines = 69 + defaultLines = 70 ) func fakeCreateConfFile() error { @@ -146,9 +147,9 @@ func TestUpdateOrAppendMultipleLines(t *testing.T) { expectedErr bool keyUpdateValue map[string]string totalLines int - searchString map[int]string + searchString map[string]bool }{ - "Matched values": { + "Add Lines": { expectedErr: false, keyUpdateValue: map[string]string{ " ReplicationFactor": " ReplicationFactor 4", @@ -157,12 +158,33 @@ func TestUpdateOrAppendMultipleLines(t *testing.T) { " Replica 6163": " Replica 6163 6163", " Replica 6164": " Replica 6164 6164", }, - searchString: map[int]string{ - 1: " Replica 6163 6163", - 2: " ReplicationFactor 4", - 3: " Replica 6162 6162", - 4: " Replica 6161 6161", - 5: " Replica 6164 6164", + searchString: map[string]bool{ + " Replica 6163 6163": true, + " ReplicationFactor 4": true, + " Replica 6162 6162": true, + " Replica 6161 6161": false, + " Replica 6164 6164": true, + " Replica 6162 6062334": false, + " ConsistencyFactor 2": false, + }, + totalLines: defaultLines + 1, + }, + "Remove Lines": { + expectedErr: false, + keyUpdateValue: map[string]string{ + " ReplicationFactor": " ReplicationFactor 5", + " ConsistencyFactor": " ConsistencyFactor 3", + " Replica 6161": "", + " Replica 6163": " Replica 6163 6163", + " Replica 6164": " Replica 6164 6164", + " Replica 7161": "", + }, + searchString: map[string]bool{ + " Replica 6163 6163": true, + " ReplicationFactor 5": true, + " Replica 6164 6164": true, + " ReplicationFactor 3": false, + " Replica 6161 6161": false, }, totalLines: defaultLines + 1, }, @@ -175,14 +197,21 @@ func TestUpdateOrAppendMultipleLines(t *testing.T) { for name, mock := range tests { name := name // pin it mock := mock // pin it + var index int err = fakeFileOperator.UpdateOrAppendMultipleLines(fakePath, mock.keyUpdateValue, 0644) if err != nil { t.Fatalf("test %q failed : expected error not to be nil but got %v", name, err) } - for _, value := range mock.searchString { - _, _, err = fakeFileOperator.GetLineDetails(fakePath, value) + for key, value := range mock.searchString { + index, _, err = fakeFileOperator.GetLineDetails(fakePath, key) if err != nil { - t.Fatalf("test %q failed :to get %s details from file %s", name, value, fakePath) + t.Fatalf("test %q failed :to get %s details from file %s", name, key, fakePath) + } + if value && index == -1 { + t.Fatalf("test %q failed :expected %s should exist in file but it doesn't exist", name, key) + } + if !value && index != -1 { + t.Fatalf("test %q failed :expected %s should not exist but it exist", name, key) } } } From 488c86c8661932fdcd9f2b75414458041662c455 Mon Sep 17 00:00:00 2001 From: mittachaitu Date: Wed, 30 Oct 2019 14:33:15 +0530 Subject: [PATCH 2/4] This commit fixes golangci linting and travis issue Signed-off-by: mittachaitu --- Gopkg.lock | 9 --------- .../controller/volume-controller/handler.go | 4 ++-- cmd/cstor-volume-mgmt/volume/volume_test.go | 2 +- pkg/cstor/volume/v1alpha1/cstorvolume.go | 4 ++-- 4 files changed, 5 insertions(+), 14 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 4a6cda8960..c28a2800bd 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -95,14 +95,6 @@ revision = "0ca988a254f991240804bf9821f3450d87ccbb1b" version = "v1.3.0" -[[projects]] - branch = "master" - digest = "1:1ba1d79f2810270045c328ae5d674321db34e3aae468eb4233883b473c5c0467" - name = "github.com/golang/glog" - packages = ["."] - pruneopts = "UT" - revision = "23def4e6c14b4da8ac2ed8007337bc5eb5007998" - [[projects]] branch = "master" digest = "1:b7cb6054d3dff43b38ad2e92492f220f57ae6087ee797dca298139776749ace8" @@ -1098,7 +1090,6 @@ "github.com/Masterminds/sprig", "github.com/docker/go-units", "github.com/ghodss/yaml", - "github.com/golang/glog", "github.com/golang/protobuf/proto", "github.com/google/gofuzz", "github.com/hashicorp/hcl", diff --git a/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go b/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go index f98ad9dec0..0a2808f652 100644 --- a/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go +++ b/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go @@ -504,13 +504,13 @@ func (c *CStorVolumeController) updateCStorVolumeDRF( ) return } + cStorVolume = copyCV c.recorder.Event(cStorVolume, corev1.EventTypeNormal, string(common.SuccessUpdated), fmt.Sprintf("Successfully updated the desiredReplicationFactor to %d", cStorVolume.Spec.DesiredReplicationFactor), ) - cStorVolume = copyCV } // triggerScaleUpProcess returns error incase of any error during scaleup @@ -574,7 +574,7 @@ func (c *CStorVolumeController) triggerScaleDownProcess( configData := cStorVolume.BuildScaleDownConfigData(replicaID) fileOperator := util.RealFileOperator{} cstorvolume_v1alpha1.ConfFileMutex.Lock() - err := fileOperator.UpdateOrAppendMultipleLines( + err = fileOperator.UpdateOrAppendMultipleLines( cstorvolume_v1alpha1.IstgtConfPath, configData, 0644) diff --git a/cmd/cstor-volume-mgmt/volume/volume_test.go b/cmd/cstor-volume-mgmt/volume/volume_test.go index 1bf130216d..b517687447 100644 --- a/cmd/cstor-volume-mgmt/volume/volume_test.go +++ b/cmd/cstor-volume-mgmt/volume/volume_test.go @@ -294,7 +294,7 @@ func TestCheckValidVolume(t *testing.T) { }, }, "invalid desiredreplicationfactor/replicationfactor": { - expectedError: true, + expectedError: false, test: &apis.CStorVolume{ ObjectMeta: v1.ObjectMeta{ Name: "testvol1", diff --git a/pkg/cstor/volume/v1alpha1/cstorvolume.go b/pkg/cstor/volume/v1alpha1/cstorvolume.go index a0f6c22fd7..010dea1155 100644 --- a/pkg/cstor/volume/v1alpha1/cstorvolume.go +++ b/pkg/cstor/volume/v1alpha1/cstorvolume.go @@ -254,7 +254,7 @@ func (c *CStorVolume) GetRemovingReplicaID() string { } // BuildScaleDownConfigData build data based on replica that needs to remove -func (c *CStorVolume) BuildScaleDownConfigData(repId string) map[string]string { +func (c *CStorVolume) BuildScaleDownConfigData(repID string) map[string]string { configData := map[string]string{} newReplicationFactor := c.object.Spec.ReplicationFactor newConsistencyFactor := (newReplicationFactor / 2) + 1 @@ -264,7 +264,7 @@ func (c *CStorVolume) BuildScaleDownConfigData(repId string) map[string]string { key = fmt.Sprintf(" ConsistencyFactor") value = fmt.Sprintf(" ConsistencyFactor %d", newConsistencyFactor) configData[key] = value - key = fmt.Sprintf(" Replica %s", repId) + key = fmt.Sprintf(" Replica %s", repID) value = fmt.Sprintf("") configData[key] = value return configData From 3a2b3f3ca979fb12522b89058ca85157911bcb05 Mon Sep 17 00:00:00 2001 From: mittachaitu Date: Thu, 31 Oct 2019 11:15:43 +0530 Subject: [PATCH 3/4] This commit fixes the bug while updating istgt.conf file Signed-off-by: mittachaitu --- pkg/cstor/volume/v1alpha1/cstorvolume.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cstor/volume/v1alpha1/cstorvolume.go b/pkg/cstor/volume/v1alpha1/cstorvolume.go index 010dea1155..9047f16422 100644 --- a/pkg/cstor/volume/v1alpha1/cstorvolume.go +++ b/pkg/cstor/volume/v1alpha1/cstorvolume.go @@ -256,7 +256,7 @@ func (c *CStorVolume) GetRemovingReplicaID() string { // BuildScaleDownConfigData build data based on replica that needs to remove func (c *CStorVolume) BuildScaleDownConfigData(repID string) map[string]string { configData := map[string]string{} - newReplicationFactor := c.object.Spec.ReplicationFactor + newReplicationFactor := c.object.Spec.DesiredReplicationFactor newConsistencyFactor := (newReplicationFactor / 2) + 1 key := fmt.Sprintf(" ReplicationFactor") value := fmt.Sprintf(" ReplicationFactor %d", newReplicationFactor) @@ -264,6 +264,10 @@ func (c *CStorVolume) BuildScaleDownConfigData(repID string) map[string]string { key = fmt.Sprintf(" ConsistencyFactor") value = fmt.Sprintf(" ConsistencyFactor %d", newConsistencyFactor) configData[key] = value + key = fmt.Sprintf(" DesiredReplicationFactor") + value = fmt.Sprintf(" DesiredReplicationFactor %d", + c.object.Spec.DesiredReplicationFactor) + configData[key] = value key = fmt.Sprintf(" Replica %s", repID) value = fmt.Sprintf("") configData[key] = value From 4ac1db2b259b4b0350b93c0481dcd9585897c2ac Mon Sep 17 00:00:00 2001 From: mittachaitu Date: Tue, 5 Nov 2019 11:15:41 +0530 Subject: [PATCH 4/4] This commit updates the code comments for func updateCStorVolumeDRF Signed-off-by: mittachaitu --- .../controller/volume-controller/handler.go | 36 ++++++++++++++----- 1 file changed, 27 insertions(+), 9 deletions(-) diff --git a/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go b/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go index 0a2808f652..06c64c9d00 100644 --- a/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go +++ b/cmd/cstor-volume-mgmt/controller/volume-controller/handler.go @@ -480,19 +480,37 @@ func (c *CStorVolumeController) addResizeConditions( return updatedCVObj, nil } -// updateCStorVolumeDRF updates desiredReplicationFactor in memory istgt -// configuration on success updates istgt.conf file +// updateCStorVolumeDRF updates volume configurations to changes made to +// cStorVolume CR. +// If no.of entries in cStorvolume spec.ReplicaDetails.KnownReplicas is less +// than entries in status.ReplicaDetails.knownReplicas then the changes are +// identified for scaledown +// process. +// If no.of entries in cStorVolume spec.ReplicaDetails.KnownReplicas is equal to +// no.of entries in stauts.ReplicaDetails.KnownReplicas then the changes are +// identified for scaleup process(Only one process should be triggered either +// scaleup or scaledown). +// NOTE: No.of entries in spec.KnownReplicas and status.knownReplicas will vary +// only in the scaledown process else always it will be same func (c *CStorVolumeController) updateCStorVolumeDRF( cStorVolume *apis.CStorVolume) { // make changes to copyCV instead of cStorVolume copyCV := cStorVolume.DeepCopy() var err error - if len(copyCV.Spec.ReplicaDetails.KnownReplicas) < - len(copyCV.Status.ReplicaDetails.KnownReplicas) { + specKnownReplicaCount := len(copyCV.Spec.ReplicaDetails.KnownReplicas) + statusKnownReplicaCount := len(copyCV.Status.ReplicaDetails.KnownReplicas) + if specKnownReplicaCount < statusKnownReplicaCount { copyCV, err = c.triggerScaleDownProcess(copyCV) - } else if len(copyCV.Spec.ReplicaDetails.KnownReplicas) == - len(copyCV.Status.ReplicaDetails.KnownReplicas) { + } else if specKnownReplicaCount == statusKnownReplicaCount { copyCV, err = c.triggerScaleUpProcess(copyCV) + } else { + // entries in spec.ReplicaDetails.KnownReplicas can't be greater than + // status.ReplicaDetails.KnownReplicas(unkown changes) + err = pkg_errors.Errorf("unkown changes are made to cStorVolume no.of "+ + "entries in spec.ReplicaDetails.knownReplicas are %d and no.of entries "+ + "in status.ReplicaDetails.KnownReplicas are %d", + specKnownReplicaCount, + statusKnownReplicaCount) } if err != nil { c.recorder.Event(cStorVolume, @@ -513,7 +531,7 @@ func (c *CStorVolumeController) updateCStorVolumeDRF( ) } -// triggerScaleUpProcess returns error incase of any error during scaleup +// triggerScaleUpProcess returns error in case of any error occurred during scaleup // process else it will return cstorvolume object func (c *CStorVolumeController) triggerScaleUpProcess( cStorVolume *apis.CStorVolume) (*apis.CStorVolume, error) { @@ -542,9 +560,9 @@ func (c *CStorVolumeController) triggerScaleUpProcess( return cStorVolume, nil } -// triggerScaleDownProcess returns error incase of any error during scaledown +// triggerScaleDownProcess returns error in case of any error during the scaledown // process else it will return cstorvolume object. Following steps are executed -// 1. Verify are all the replicas are healthy other than removing replica. +// 1. Verify whether all the replicas are healthy other than removing replica. // 2. If step1 is passed then update the istgt.conf file with latest // information. // 3. Update cStorVolume CR(replicationFactor, ConsistencyFactor and known