From f4173eb6984e8998ef64c60c01f9db591224920c Mon Sep 17 00:00:00 2001 From: Ondrej Belusky Date: Tue, 10 Nov 2020 16:29:40 +0100 Subject: [PATCH] fix(restore): set targetip on CVRs after restore is completed Auto-setting of targetip on CVR if openebs.io/restore-completed is found Signed-off-by: Ondrej Belusky --- changelogs/unreleased/1761-zlymeda | 1 + .../controller/replica-controller/handler.go | 33 +++++++++++++- .../new_replica_controller.go | 16 ------- .../volumereplica/volumereplica.go | 43 ++++++++++++++++--- tests/framework/v1alpha1/framework.go | 3 +- tests/sts/sts_suite_test.go | 10 +++-- tests/sts/sts_test.go | 16 ++++--- 7 files changed, 88 insertions(+), 34 deletions(-) create mode 100644 changelogs/unreleased/1761-zlymeda diff --git a/changelogs/unreleased/1761-zlymeda b/changelogs/unreleased/1761-zlymeda new file mode 100644 index 0000000000..c85ed2085c --- /dev/null +++ b/changelogs/unreleased/1761-zlymeda @@ -0,0 +1 @@ +fix(restore): set targetip on CVRs after restore is completed diff --git a/cmd/cstor-pool-mgmt/controller/replica-controller/handler.go b/cmd/cstor-pool-mgmt/controller/replica-controller/handler.go index 4a3474bec5..bda081213a 100644 --- a/cmd/cstor-pool-mgmt/controller/replica-controller/handler.go +++ b/cmd/cstor-pool-mgmt/controller/replica-controller/handler.go @@ -39,7 +39,8 @@ import ( ) const ( - v130 = "1.3.0" + v130 = "1.3.0" + restoreCompletedAnnotation = "openebs.io/restore-completed" ) type upgradeParams struct { @@ -135,6 +136,18 @@ func (c *CStorVolumeReplicaController) syncHandler( // TODO // need to rethink on this logic !! // status holds more importance than error + + if err != nil { + klog.Errorf("handling operation %s failed: %v", operation, err) + + c.recorder.Event( + cvrGot, + corev1.EventTypeWarning, + "SyncFailed", + fmt.Sprintf("handling operation %s failed: %v", operation, err), + ) + } + return nil } cvrGot.Status.LastUpdateTime = metav1.Now() @@ -281,6 +294,24 @@ func (c *CStorVolumeReplicaController) cVREventHandler( if isCVRCreateStatus(cvrObj) { return c.cVRAddEventHandler(cvrObj, fullVolName) } + + if cvrObj.Annotations[restoreCompletedAnnotation] == "true" { + targetIp, err := volumereplica.GetTargetIp(fullVolName) + if err != nil { + return "", err + } + + if targetIp == "" { + err := volumereplica.SetTargetIp(fullVolName, cvrObj.Spec.TargetIP) + if err != nil { + return "", err + } + } + + delete(cvrObj.Annotations, restoreCompletedAnnotation) + delete(cvrObj.Annotations, volumereplica.IsRestoreVol) + } + return c.getCVRStatus(cvrObj) } diff --git a/cmd/cstor-pool-mgmt/controller/replica-controller/new_replica_controller.go b/cmd/cstor-pool-mgmt/controller/replica-controller/new_replica_controller.go index 970d12f2a0..538f027893 100644 --- a/cmd/cstor-pool-mgmt/controller/replica-controller/new_replica_controller.go +++ b/cmd/cstor-pool-mgmt/controller/replica-controller/new_replica_controller.go @@ -214,22 +214,6 @@ func NewCStorVolumeReplicaController( return } - if newCVR.ResourceVersion != oldCVR.ResourceVersion { - // cvr modify is not implemented - // hence below is commented - // ql.Operation = common.QOpModify - - controller.recorder.Event( - newCVR, - corev1.EventTypeNormal, - string(common.SuccessSynced), - string(common.MessageModifySynced), - ) - - // no further handling needed - return - } - // finally !!! // push this operation to workqueue ql.Operation = common.QOpSync diff --git a/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go b/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go index 8241da2479..8b34f02aad 100644 --- a/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go +++ b/cmd/cstor-pool-mgmt/volumereplica/volumereplica.go @@ -41,6 +41,8 @@ import ( const ( // VolumeReplicaOperator is the name of the tool that makes volume-related operations. VolumeReplicaOperator = "zfs" + // Execute is the name of the tool that evaluates the provided command + Execute = "/usr/local/bin/execute.sh" // BinaryCapacityUnitSuffix is the suffix for binary capacity unit. BinaryCapacityUnitSuffix = "i" // CreateCmd is the create command for zfs volume. @@ -69,6 +71,8 @@ const ( MaxRestoreRetryCount = 10 // RestoreRetryDelay is time(in seconds) to wait before the next attempt for restore transfer RestoreRetryDelay = 5 + // IsRestoreVol marks CVRs created through restore + IsRestoreVol = "isRestoreVol" ) const ( @@ -244,7 +248,7 @@ func CreateVolumeReplica(cStorVolumeReplica *apis.CStorVolumeReplica, fullVolNam return nil } -// builldVolumeCreateCommand returns volume create command along with attributes as a string array +// buildVolumeCreateCommand returns volume create command along with attributes as a string array func buildVolumeCreateCommand(cStorVolumeReplica *apis.CStorVolumeReplica, fullVolName string, quorum bool) []string { var createVolCmd []string @@ -277,7 +281,7 @@ func buildVolumeCreateCommand(cStorVolumeReplica *apis.CStorVolumeReplica, fullV ) } - if cStorVolumeReplica.Annotations["isRestoreVol"] != "true" { + if cStorVolumeReplica.Annotations[IsRestoreVol] != "true" { createVolCmd = append(createVolCmd, "-o", openebsTargetIP, ) @@ -319,6 +323,35 @@ func buildVolumeCloneCommand(cStorVolumeReplica *apis.CStorVolumeReplica, snapNa return cloneVolCmd } +func SetTargetIp(fullVolName, targetIp string) error { + out, err := zfs.NewVolumeSetProperty(). + WithProperty("io.openebs:targetip", targetIp). + WithDataset(fullVolName). + Execute() + if err != nil { + klog.Errorf("Unable to set target ip to %s. error : %v %s", fullVolName, err, string(out)) + return errors.Wrapf(err, "failed to set target ip to %s", fullVolName) + } + + return nil +} + +func GetTargetIp(fullVolName string) (string, error) { + ret, err := zfs.NewVolumeGetProperty(). + WithScriptedMode(true). + WithParsableMode(true). + WithField("value"). + WithProperty("io.openebs:targetip"). + WithDataset(fullVolName). + Execute() + + if err != nil { + klog.Errorf("Unable to get target ip from %s. error : %v", fullVolName, err) + return "", errors.Wrapf(err, "failed to get target ip: %s", fullVolName) + } + return strings.Split(string(ret), "\n")[0], nil +} + // CreateVolumeBackup sends cStor snapshots to remote location specified by cstorbackup. func CreateVolumeBackup(bkp *apis.CStorBackup) error { var cmd []string @@ -332,7 +365,7 @@ func CreateVolumeBackup(bkp *apis.CStorBackup) error { klog.Infof("Backup Command for volume: %v created, Cmd: %v\n", bkp.Spec.VolumeName, cmd) for retryCount < MaxBackupRetryCount { - stdoutStderr, err = RunnerVar.RunCombinedOutput("/usr/local/bin/execute.sh", cmd...) + stdoutStderr, err = RunnerVar.RunCombinedOutput(Execute, cmd...) if err != nil { klog.Errorf("Unable to start backup %s. error : %v retry:%v :%s", bkp.Spec.VolumeName, string(stdoutStderr), retryCount, err.Error()) retryCount++ @@ -357,7 +390,7 @@ func CreateVolumeBackup(bkp *apis.CStorBackup) error { return err } -// builldVolumeBackupCommand returns volume create command along with attributes as a string array +// buildVolumeBackupCommand returns volume create command along with attributes as a string array func buildVolumeBackupCommand(poolName, fullVolName, oldSnapName, newSnapName, backupDest string) []string { var startBackupCmd []string @@ -383,7 +416,7 @@ func CreateVolumeRestore(rst *apis.CStorRestore) error { klog.Infof("Restore Command for volume: %v created, Cmd: %v\n", rst.Spec.VolumeName, cmd) for retryCount < MaxRestoreRetryCount { - stdoutStderr, err = RunnerVar.RunCombinedOutput("/usr/local/bin/execute.sh", cmd...) + stdoutStderr, err = RunnerVar.RunCombinedOutput(Execute, cmd...) if err != nil { klog.Errorf("Unable to start restore %s. error : %v.. trying again", rst.Spec.VolumeName, string(stdoutStderr)) time.Sleep(RestoreRetryDelay * time.Second) diff --git a/tests/framework/v1alpha1/framework.go b/tests/framework/v1alpha1/framework.go index 8547c18db4..964017a7aa 100644 --- a/tests/framework/v1alpha1/framework.go +++ b/tests/framework/v1alpha1/framework.go @@ -16,6 +16,7 @@ package v1alpha1 import ( "os" + "strconv" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -196,6 +197,6 @@ func checkComponentStatus(namespace, lselector string, Count int) (pods *corev1. Len() }, DefaultTimeOut, DefaultPollingInterval). - Should(Equal(Count), "Pod count should be "+string(Count)) + Should(Equal(Count), "Pod count should be "+strconv.Itoa(Count)) return } diff --git a/tests/sts/sts_suite_test.go b/tests/sts/sts_suite_test.go index 7a4cdb61b6..709833a466 100644 --- a/tests/sts/sts_suite_test.go +++ b/tests/sts/sts_suite_test.go @@ -17,6 +17,7 @@ package sts import ( "flag" "os" + "strconv" "testing" @@ -43,7 +44,7 @@ const ( // time in seconds for the Eventually block defaultPollingInterval int = 10 // minNodeCount is the minimum number of nodes - // neede to run this test + // needed to run this test minNodeCount int = 3 ) @@ -61,7 +62,7 @@ var _ = BeforeSuite(func() { configPath, err := kubernetes.GetConfigPath() Expect(err).ShouldNot(HaveOccurred()) - // Setting the path in environemnt variable + // Setting the path in environment variable err = os.Setenv(string(v1alpha1.KubeConfigEnvironmentKey), configPath) Expect(err).ShouldNot(HaveOccurred()) // Getting clientset @@ -70,6 +71,7 @@ var _ = BeforeSuite(func() { // Checking appropriate node numbers. This test is designed to run on a 3 node cluster nodes, err := cl.CoreV1().Nodes().List(v1.ListOptions{}) + Expect(err).ShouldNot(HaveOccurred()) Expect(nodes.Items).Should(HaveLen(minNodeCount)) // Fetching the openebs component artifacts @@ -159,7 +161,7 @@ var _ = BeforeSuite(func() { Len() }, defaultTimeOut, defaultPollingInterval). - Should(Equal(minNodeCount), "NDM pod count should be "+string(minNodeCount)) + Should(Equal(minNodeCount), "NDM pod count should be "+strconv.Itoa(minNodeCount)) // Check for cstor storage pool pods to get created and running Eventually(func() int { @@ -174,7 +176,7 @@ var _ = BeforeSuite(func() { Len() }, defaultTimeOut, defaultPollingInterval). - Should(Equal(minNodeCount), "CStor pool pod count should be "+string(minNodeCount)) + Should(Equal(minNodeCount), "CStor pool pod count should be "+strconv.Itoa(minNodeCount)) }) var _ = AfterSuite(func() { diff --git a/tests/sts/sts_test.go b/tests/sts/sts_test.go index 23c72ad0f3..e1ff9496be 100644 --- a/tests/sts/sts_test.go +++ b/tests/sts/sts_test.go @@ -15,6 +15,8 @@ package sts import ( + "strconv" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" k8s "github.com/openebs/maya/pkg/client/k8s/v1alpha1" @@ -140,7 +142,7 @@ var _ = Describe("StatefulSet", func() { return pvcCount }, defaultTimeOut, defaultPollingInterval). - Should(Equal(3), "PVC count should be "+string(3)) + Should(Equal(3), "PVC count should be "+strconv.Itoa(3)) // Check for CVR to get healthy Eventually(func() int { @@ -156,7 +158,7 @@ var _ = Describe("StatefulSet", func() { Len() }, defaultTimeOut, defaultPollingInterval). - Should(Equal(3), "CVR count should be "+string(3)) + Should(Equal(3), "CVR count should be "+strconv.Itoa(3)) // Check for statefulset pods to get created and running Eventually(func() int { @@ -172,7 +174,7 @@ var _ = Describe("StatefulSet", func() { Len() }, defaultTimeOut, defaultPollingInterval). - Should(Equal(3), "Pod count should be "+string(3)) + Should(Equal(3), "Pod count should be "+strconv.Itoa(3)) }) AfterEach(func() { @@ -267,25 +269,25 @@ var _ = Describe("StatefulSet", func() { WithFilter(pvc.ContainsName(STSUnstructured.GetName())). List() Expect(err).ShouldNot(HaveOccurred()) - Expect(pvcList.Len()).Should(Equal(3), "pvc count should be "+string(3)) + Expect(pvcList.Len()).Should(Equal(3), "pvc count should be "+strconv.Itoa(3)) cvrs, err := cvr. NewKubeclient(cvr.WithNamespace("")). List(metav1.ListOptions{LabelSelector: replicaAntiAffinityLabel}) - Expect(cvrs.Items).Should(HaveLen(3), "cvr count should be "+string(3)) + Expect(cvrs.Items).Should(HaveLen(3), "cvr count should be "+strconv.Itoa(3)) poolNames := cvr. NewListBuilder(). WithAPIList(cvrs). List() Expect(poolNames.GetUniquePoolNames()). - Should(HaveLen(3), "pool names count should be "+string(3)) + Should(HaveLen(3), "pool names count should be "+strconv.Itoa(3)) pools, err := csp.NewKubeClient().List(metav1.ListOptions{}) Expect(err).ShouldNot(HaveOccurred()) nodeNames := csp.ListBuilder().WithAPIList(pools).List() Expect(nodeNames.GetPoolUIDs()). - Should(HaveLen(3), "node names count should be "+string(3)) + Should(HaveLen(3), "node names count should be "+strconv.Itoa(3)) }) PIt("should co-locate the cstor volume targets with application instances", func() {