added comments for the function and removed unnecessary checks

Signed-off-by: shubham <[email protected]>

cmd/cspi-mgmt/app/handler.go

@@ -58,7 +58,9 @@ func (c *CStorPoolInstanceController) reconcile(key string) error {
 	// take a lock for common package for updating variables
 	common.SyncResources.Mux.Lock()
 
-	// try to import pool
+	// try to import pool, first check for migration case in which the previous
+	// poolname is CSP uid and should be renamed to CSPC uid. If old CSP uid is
+	// not present then follow the normal code path.
 	if cspi.Annotations[string(apis.OldCSPUID)] != "" {
 		isImported, err = zpool.Import(cspi, "cstor-"+cspi.Annotations[string(apis.OldCSPUID)])
 	} else {

cmd/cstor-pool-mgmt/pool/v1alpha2/import.go

@@ -31,6 +31,8 @@ import (
 // It will return -
 // - If pool is imported or not
 // - If any error occurred during import operation
+// The oldName argument is used to rename the old CSP
+// uid name to the current PoolName(cspi) format
 func Import(cspi *apis.CStorPoolInstance, oldName string) (bool, error) {
 	if poolExist := checkIfPoolPresent(PoolName(cspi)); poolExist {
 		return true, nil

cmd/migrate/executor/options.go

@@ -41,6 +41,5 @@ func (u *MigrateOptions) RunPreFlightChecks(cmd *cobra.Command) error {
 	if len(strings.TrimSpace(u.openebsNamespace)) == 0 {
 		return errors.Errorf("Cannot execute migrate job: namespace is missing")
 	}
-
 	return nil
 }

cmd/migrate/executor/setup_job.go

@@ -18,12 +18,8 @@ package executor
 
 import (
 	"flag"
-	//"fmt"
-	//"os"
 	"strings"
 
-	//"k8s.io/klog"
-
 	"github.com/spf13/cobra"
 )
 

pkg/algorithm/nodeselect/v1alpha2/build_csp.go

@@ -61,20 +61,16 @@ func (ac *Config) GetCSPSpec() (*apis.CStorPoolInstance, error) {
 		return nil, errors.Wrapf(err, "failed to build CSP object for node selector {%v}", poolSpec.NodeSelector)
 	}
 
-	annotations := map[string]string{}
-
+	// check for OpenEBSDisableDependantsReconcileKey annotation which implies
+	// the CSPI should have OpenEBSDisableReconcileKey enabled
 	if ac.CSPC.GetAnnotations()[string(apis.OpenEBSDisableDependantsReconcileKey)] == "false" {
-		annotations[string(apis.OpenEBSDisableReconcileKey)] = "true"
+		cspObj.Object.Annotations[string(apis.OpenEBSDisableReconcileKey)] = "true"
 	}
-
+	// if old CSP has the pool with its name add annotation for renaming while importing
 	if poolSpec.OldCSPUID != "" {
-		annotations[string(apis.OldCSPUID)] = poolSpec.OldCSPUID
+		cspObj.Object.Annotations[string(apis.OldCSPUID)] = poolSpec.OldCSPUID
 	}
-
-	if len(annotations) != 0 {
-		cspObj.Object.SetAnnotations(annotations)
-	}
-
+	// if old CSP is present, it implies the BDs are already claimed for the pool
 	if poolSpec.OldCSPUID == "" {
 		err = ac.ClaimBDsForNode(ac.GetBDListForNode(poolSpec))
 		if err != nil {

pkg/cstor/migrate/cspc_generator.go

@@ -90,6 +90,7 @@ func getCSPCSpec(spc *apis.StoragePoolClaim) (*apis.CStorPoolCluster, error) {
 	return cspcObj, nil
 }
 
+// generateCSPC creates an equivalent cspc for the given spc object
 func generateCSPC(spcObj *apis.StoragePoolClaim, openebsNamespace string) (
 	*apis.CStorPoolCluster, error) {
 	cspcObj, err := cspc.NewKubeClient().
@@ -137,6 +138,10 @@ func generateCSPC(spcObj *apis.StoragePoolClaim, openebsNamespace string) (
 	if err != nil {
 		return nil, err
 	}
+	// after all the cspi come up which reconcilation disabled delete the
+	// OpenEBSDisableDependantsReconcileKey annotation so that in future when
+	// a cspi is delete and it comes back on reconciliation it should not have
+	// reconciliation disabled
 	delete(cspcObj.Annotations, string(apis.OpenEBSDisableDependantsReconcileKey))
 	cspcObj, err = cspc.NewKubeClient().
 		WithNamespace(openebsNamespace).

pkg/cstor/migrate/pool.go

@@ -55,11 +55,12 @@ const (
 	cspcFinalizer          = "cstorpoolcluster.openebs.io/finalizer"
 )
 
-// Pool ...
+// Pool migrates the pool from SPC schema to CSPC schema
 func Pool(spcName, openebsNamespace string) error {
 
 	spcObj, err := spc.NewKubeClient().
 		Get(spcName, metav1.GetOptions{})
+	// verify if the spc is already migrated.
 	if k8serrors.IsNotFound(err) {
 		klog.Infof("spc %s not found.", spcName)
 		_, err = cspc.NewKubeClient().
@@ -78,7 +79,7 @@ func Pool(spcName, openebsNamespace string) error {
 	if err != nil {
 		return err
 	}
-
+	// List all cspi created with reconcile off
 	cspiList, err := cspi.NewKubeClient().
 		WithNamespace(openebsNamespace).
 		List(metav1.ListOptions{
@@ -88,7 +89,11 @@ func Pool(spcName, openebsNamespace string) error {
 		return err
 	}
 
+	// For each cspi perform the migration from csp that present was on
+	// node on which cspi came up.
 	for _, cspiObj := range cspiList.Items {
+		// Skip the migration if cspi is already in ONLINE state,
+		// which implies the migration is done and makes it idempotent
 		if cspiObj.Status.Phase != "ONLINE" {
 			err = csptocspi(&cspiObj, cspcObj, openebsNamespace)
 			if err != nil {
@@ -99,6 +104,8 @@ func Pool(spcName, openebsNamespace string) error {
 			if err != nil {
 				return err
 			}
+			// remove the OldCSPUID name to avoid renaming in case
+			// cspi is deleted and comes up after reconciliation.
 			for i, poolspec := range cspcObj.Spec.Pools {
 				if poolspec.NodeSelector[string(apis.HostNameCPK)] ==
 					cspiObj.Labels[string(apis.HostNameCPK)] {
@@ -112,6 +119,7 @@ func Pool(spcName, openebsNamespace string) error {
 			}
 		}
 	}
+	// Clean up old SPC resources after the migration is complete
 	err = spc.NewKubeClient().
 		Delete(spcName, &metav1.DeleteOptions{})
 	if err != nil {
@@ -120,6 +128,7 @@ func Pool(spcName, openebsNamespace string) error {
 	return nil
 }
 
+// csptocspi migrates a CSP to CSPI based on hostname
 func csptocspi(cspiObj *apis.CStorPoolInstance, cspcObj *apis.CStorPoolCluster, openebsNamespace string) error {
 	hostnameLabel := string(apis.HostNameCPK) + "=" + cspiObj.Labels[string(apis.HostNameCPK)]
 	spcLabel := string(apis.StoragePoolClaimCPK) + "=" + cspcObj.Name
@@ -140,6 +149,8 @@ func csptocspi(cspiObj *apis.CStorPoolInstance, cspcObj *apis.CStorPoolCluster,
 			return err
 		}
 	}
+	// once the old pool pod is scaled down and bdcs are patched
+	// bring up the cspi pod so that the old pool can be renamed and imported.
 	delete(cspiObj.Annotations, string(apis.OpenEBSDisableReconcileKey))
 	cspiObj, err = cspi.NewKubeClient().
 		WithNamespace(openebsNamespace).
@@ -173,6 +184,8 @@ func csptocspi(cspiObj *apis.CStorPoolInstance, cspcObj *apis.CStorPoolCluster,
 	return nil
 }
 
+// get csp for cspi on the basis of cspLabel, which is the combination of
+// hostname label on which cspi came up and the spc label.
 func getCSP(cspLabel string) (*apis.CStorPool, error) {
 	cspClient := csp.KubeClient()
 	cspList, err := cspClient.List(metav1.ListOptions{
@@ -188,6 +201,8 @@ func getCSP(cspLabel string) (*apis.CStorPool, error) {
 	return &cspObj, nil
 }
 
+// The old pool pod should be scaled down before the new cspi pod comes up
+// to avoid importing the pool at two places at the same time.
 func scaleDownDeployment(cspObj *apis.CStorPool, openebsNamespace string) error {
 	klog.Infof("Scaling down deployemnt %s", cspObj.Name)
 	cspPod, err := pod.NewKubeClient().
@@ -227,6 +242,8 @@ func scaleDownDeployment(cspObj *apis.CStorPool, openebsNamespace string) error
 	return nil
 }
 
+// Update the bdc with the cspc labels instead of spc labels to allow
+// filtering of bds claimed by the migrated cspc.
 func updateBDC(bdName apis.CspBlockDevice, cspcObj *apis.CStorPoolCluster, openebsNamespace string) error {
 	bdObj, err := bd.NewKubeClient().
 		WithNamespace(openebsNamespace).
@@ -259,6 +276,8 @@ func updateBDC(bdName apis.CspBlockDevice, cspcObj *apis.CStorPoolCluster, opene
 	return nil
 }
 
+// Update the cvrs on the old csp with the migrated cspi labels and annotations
+// to allow backward compatibility with old external provisioned volumes.
 func updateCVRsLabels(cspName, openebsNamespace string, cspiObj *apis.CStorPoolInstance) error {
 	cvrList, err := cvr.NewKubeclient().
 		WithNamespace(openebsNamespace).List(metav1.ListOptions{