From 0ddd9d540907ac490b94c886dd17affc28cabf0d Mon Sep 17 00:00:00 2001 From: shubham Date: Tue, 10 Dec 2019 16:30:35 +0530 Subject: [PATCH] This commit chnages the design from schema changes to use of annotations: - removed oldCSPUID schema change - added two generic annotation which can be used in different scenarios Signed-off-by: shubham --- cmd/cspi-mgmt/app/handler.go | 10 +- cmd/cstor-pool-mgmt/pool/v1alpha2/create.go | 2 +- cmd/cstor-pool-mgmt/pool/v1alpha2/import.go | 34 +++-- .../pool/v1alpha2/pool_utils.go | 5 +- .../nodeselect/v1alpha2/build_csp.go | 28 ++-- pkg/apis/openebs.io/v1alpha1/cas_pool.go | 4 +- .../openebs.io/v1alpha1/cstorpool_cluster.go | 5 +- pkg/cstor/migrate/cspc_generator.go | 1 - pkg/cstor/migrate/pool.go | 138 ++++++++++-------- 9 files changed, 124 insertions(+), 103 deletions(-) diff --git a/cmd/cspi-mgmt/app/handler.go b/cmd/cspi-mgmt/app/handler.go index da1d2d7d3a..9f46967691 100644 --- a/cmd/cspi-mgmt/app/handler.go +++ b/cmd/cspi-mgmt/app/handler.go @@ -58,14 +58,7 @@ func (c *CStorPoolInstanceController) reconcile(key string) error { // take a lock for common package for updating variables common.SyncResources.Mux.Lock() - // 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 { - isImported, err = zpool.Import(cspi, "") - } + isImported, err = zpool.Import(cspi) if isImported { if err != nil { common.SyncResources.Mux.Unlock() @@ -77,7 +70,6 @@ func (c *CStorPoolInstanceController) reconcile(key string) error { } zpool.CheckImportedPoolVolume() common.SyncResources.Mux.Unlock() - delete(cspi.Annotations, string(apis.OldCSPUID)) err = c.update(cspi) if err != nil { c.recorder.Event(cspi, diff --git a/cmd/cstor-pool-mgmt/pool/v1alpha2/create.go b/cmd/cstor-pool-mgmt/pool/v1alpha2/create.go index 63321d3632..8d1a3fe256 100644 --- a/cmd/cstor-pool-mgmt/pool/v1alpha2/create.go +++ b/cmd/cstor-pool-mgmt/pool/v1alpha2/create.go @@ -29,7 +29,7 @@ func Create(csp *apis.CStorPoolInstance) error { // Let's check if there is any disk having the pool config // If so then we will not create the pool - ret, notImported, err := checkIfPoolNotImported(csp) + ret, notImported, err := checkIfPoolIsImportable(csp) if err != nil { return errors.Errorf("Failed to check not imported pool %s", err.Error()) } diff --git a/cmd/cstor-pool-mgmt/pool/v1alpha2/import.go b/cmd/cstor-pool-mgmt/pool/v1alpha2/import.go index 35ed0c4f0d..649355657a 100644 --- a/cmd/cstor-pool-mgmt/pool/v1alpha2/import.go +++ b/cmd/cstor-pool-mgmt/pool/v1alpha2/import.go @@ -33,7 +33,7 @@ import ( // - 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) { +func Import(cspi *apis.CStorPoolInstance) (bool, error) { if poolExist := checkIfPoolPresent(PoolName(cspi)); poolExist { return true, nil } @@ -42,7 +42,14 @@ func Import(cspi *apis.CStorPoolInstance, oldName string) (bool, error) { var cmdOut []byte var err error common.SyncResources.IsImported = false - var poolImported bool + var poolImported, poolNotImported bool + + _, poolNotImported, err = checkIfPoolIsImportable(cspi) + if poolNotImported { + // if the pool is renamed but not imported remove the + // annotation to avoid not found errors + delete(cspi.Annotations, string(apis.OldPoolName)) + } bdPath, err := getPathForBDev(cspi.Spec.RaidGroups[0].BlockDevices[0].BlockDeviceName) if err != nil { @@ -51,14 +58,19 @@ func Import(cspi *apis.CStorPoolInstance, oldName string) (bool, error) { klog.Infof("Importing pool %s %s", string(cspi.GetUID()), PoolName(cspi)) devID := pool.GetDevPathIfNotSlashDev(bdPath[0]) + cmd := zfs.NewPoolImport(). + WithCachefile(cspi.Spec.PoolConfig.CacheFile). + WithProperty("cachefile", cspi.Spec.PoolConfig.CacheFile). + WithDirectory(devID). + WithNewPool(PoolName(cspi)) + // oldName denotes the pool name that may be present + // from previous version and needs to be imported with new name + oldName := cspi.Annotations[string(apis.OldPoolName)] + if oldName != "" { + cmd.WithPool(oldName) + } if len(devID) != 0 { - cmdOut, err = zfs.NewPoolImport(). - WithCachefile(cspi.Spec.PoolConfig.CacheFile). - WithProperty("cachefile", cspi.Spec.PoolConfig.CacheFile). - WithDirectory(devID). - WithPool(oldName). - WithNewPool(PoolName(cspi)). - Execute() + cmdOut, err = cmd.Execute() if err == nil { poolImported = true } else { @@ -83,6 +95,10 @@ func Import(cspi *apis.CStorPoolInstance, oldName string) (bool, error) { } klog.Infof("Pool Import successful: %v", string(PoolName(cspi))) + // after successful import of pool the annotation needs to be deleted + // to avoid renaming of pool that is already renamed which will cause + // pool not found errors + delete(cspi.Annotations, string(apis.OldPoolName)) return true, nil } diff --git a/cmd/cstor-pool-mgmt/pool/v1alpha2/pool_utils.go b/cmd/cstor-pool-mgmt/pool/v1alpha2/pool_utils.go index b08537b79f..982f46b773 100644 --- a/cmd/cstor-pool-mgmt/pool/v1alpha2/pool_utils.go +++ b/cmd/cstor-pool-mgmt/pool/v1alpha2/pool_utils.go @@ -136,7 +136,10 @@ func checkIfDeviceUsed(path []string, t zpool.Topology) (string, bool) { return usedPath, isUsed } -func checkIfPoolNotImported(cspi *apis.CStorPoolInstance) (string, bool, error) { +// checkIfPoolIsImportable checks if the pool is imported or not. If the pool +// is present on the disk but not imported it returns true as the pool can be +// imported. It also returns false if pool is not found on the disk. +func checkIfPoolIsImportable(cspi *apis.CStorPoolInstance) (string, bool, error) { var cmdOut []byte var err error diff --git a/pkg/algorithm/nodeselect/v1alpha2/build_csp.go b/pkg/algorithm/nodeselect/v1alpha2/build_csp.go index 0382f18ef3..831fe2e1ee 100644 --- a/pkg/algorithm/nodeselect/v1alpha2/build_csp.go +++ b/pkg/algorithm/nodeselect/v1alpha2/build_csp.go @@ -43,7 +43,7 @@ func (ac *Config) GetCSPSpec() (*apis.CStorPoolInstance, error) { } csplabels := ac.buildLabelsForCSPI(nodeName) - cspObj, err := apiscsp.NewBuilder(). + cspBuilder := apiscsp.NewBuilder(). WithName(ac.CSPC.Name + "-" + rand.String(4)). WithNamespace(ac.Namespace). WithNodeSelectorByReference(poolSpec.NodeSelector). @@ -55,27 +55,21 @@ func (ac *Config) GetCSPSpec() (*apis.CStorPoolInstance, error) { WithLabelsNew(csplabels). WithFinalizer(apicspsc.CSPCFinalizer). WithNewVersion(version.GetVersion()). - WithDependentsUpgraded(). - Build() - if err != nil { - return nil, errors.Wrapf(err, "failed to build CSP object for node selector {%v}", poolSpec.NodeSelector) - } - + WithDependentsUpgraded() // check for OpenEBSDisableDependantsReconcileKey annotation which implies // the CSPI should have OpenEBSDisableReconcileKey enabled if ac.CSPC.GetAnnotations()[string(apis.OpenEBSDisableDependantsReconcileKey)] == "false" { - cspObj.Object.Annotations[string(apis.OpenEBSDisableReconcileKey)] = "true" + cspBuilder.WithAnnotationsNew(map[string]string{ + string(apis.OpenEBSDisableReconcileKey): "true", + }) } - // if old CSP has the pool with its name add annotation for renaming while importing - if poolSpec.OldCSPUID != "" { - cspObj.Object.Annotations[string(apis.OldCSPUID)] = poolSpec.OldCSPUID + cspObj, err := cspBuilder.Build() + if err != nil { + return nil, errors.Wrapf(err, "failed to build CSP object for node selector {%v}", poolSpec.NodeSelector) } - // 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 { - return nil, errors.Wrapf(err, "failed to claim block devices for node {%s}", nodeName) - } + err = ac.ClaimBDsForNode(ac.GetBDListForNode(poolSpec)) + if err != nil { + return nil, errors.Wrapf(err, "failed to claim block devices for node {%s}", nodeName) } return cspObj.Object, nil } diff --git a/pkg/apis/openebs.io/v1alpha1/cas_pool.go b/pkg/apis/openebs.io/v1alpha1/cas_pool.go index bd97f1d5c0..fe6dfaed79 100644 --- a/pkg/apis/openebs.io/v1alpha1/cas_pool.go +++ b/pkg/apis/openebs.io/v1alpha1/cas_pool.go @@ -63,8 +63,8 @@ const ( RaidzBlockDeviceCountCPV CasPoolValInt = 3 // Raidz2BlockDeviceCountCPV is the count for raidz2 type pool Raidz2BlockDeviceCountCPV CasPoolValInt = 6 - // OldCSPUID is the uid of the csp that is being migrated to cspi - OldCSPUID CasPoolKey = "cspuid" + // OldPoolName is the old pool that needs to be imported and renamed + OldPoolName CasPoolKey = "cstorpoolinstance.openebs.io/oldname" ) // CasPool is a type which will be utilised by CAS engine to perform diff --git a/pkg/apis/openebs.io/v1alpha1/cstorpool_cluster.go b/pkg/apis/openebs.io/v1alpha1/cstorpool_cluster.go index 8075ccd491..45f5c8e80e 100644 --- a/pkg/apis/openebs.io/v1alpha1/cstorpool_cluster.go +++ b/pkg/apis/openebs.io/v1alpha1/cstorpool_cluster.go @@ -17,7 +17,7 @@ limitations under the License. package v1alpha1 import ( - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -89,9 +89,6 @@ type PoolSpec struct { // PoolConfig is the default pool config that applies to the // pool on node. PoolConfig PoolConfig `json:"poolConfig"` - // OldCSPUID is used to migrate old csp to cspi. This will be the - // old pool name which needs to imported and renamed as new pool - OldCSPUID string `json:"oldCSPUID,omitempty"` } // PoolConfig is the default pool config that applies to the diff --git a/pkg/cstor/migrate/cspc_generator.go b/pkg/cstor/migrate/cspc_generator.go index d8ae6f0c5d..67784ab0f2 100644 --- a/pkg/cstor/migrate/cspc_generator.go +++ b/pkg/cstor/migrate/cspc_generator.go @@ -82,7 +82,6 @@ func getCSPCSpec(spc *apis.StoragePoolClaim) (*apis.CStorPoolCluster, error) { DefaultRaidGroupType: typeMap[cspObj.Spec.PoolSpec.PoolType], OverProvisioning: cspObj.Spec.PoolSpec.OverProvisioning, }, - OldCSPUID: string(cspObj.UID), }, ) diff --git a/pkg/cstor/migrate/pool.go b/pkg/cstor/migrate/pool.go index 2898f54c50..59eeccd9c7 100644 --- a/pkg/cstor/migrate/pool.go +++ b/pkg/cstor/migrate/pool.go @@ -23,7 +23,6 @@ import ( "k8s.io/klog" apis "github.com/openebs/maya/pkg/apis/openebs.io/v1alpha1" - bd "github.com/openebs/maya/pkg/blockdevice/v1alpha2" bdc "github.com/openebs/maya/pkg/blockdeviceclaim/v1alpha1" csp "github.com/openebs/maya/pkg/cstor/pool/v1alpha3" cspc "github.com/openebs/maya/pkg/cstor/poolcluster/v1alpha1" @@ -43,7 +42,7 @@ const ( replicaPatch = `{ "spec": { "replicas": 0 - } + } }` cspNameLabel = "cstorpool.openebs.io/name" cspUIDLabel = "cstorpool.openebs.io/uid" @@ -58,27 +57,27 @@ const ( // 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(). - WithNamespace(openebsNamespace).Get(spcName, metav1.GetOptions{}) - if err != nil { - return errors.Wrapf(err, "failed to get equivalent cspc for spc %s", spcName) - } + spcObj, migrated, err := getSPC(spcName, openebsNamespace) + if migrated { klog.Infof("spc %s is already migrated to cspc", spcName) return nil } if err != nil { return err } + err = updateBDCLabels(spcName, openebsNamespace) + if err != nil { + return err + } klog.Infof("Creating equivalent cspc for spc %s", spcName) cspcObj, err := generateCSPC(spcObj, openebsNamespace) if err != nil { return err } + err = updateBDCOwnerRef(cspcObj, openebsNamespace) + if err != nil { + return err + } // List all cspi created with reconcile off cspiList, err := cspi.NewKubeClient(). WithNamespace(openebsNamespace). @@ -99,24 +98,6 @@ func Pool(spcName, openebsNamespace string) error { if err != nil { return err } - cspcObj, err = cspc.NewKubeClient(). - WithNamespace(openebsNamespace).Get(cspcObj.Name, metav1.GetOptions{}) - 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)] { - cspcObj.Spec.Pools[i].OldCSPUID = "" - } - } - cspcObj, err = cspc.NewKubeClient(). - WithNamespace(openebsNamespace).Update(cspcObj) - if err != nil { - return err - } } } // Clean up old SPC resources after the migration is complete @@ -128,8 +109,33 @@ func Pool(spcName, openebsNamespace string) error { return nil } +// getSPC gets the spc by name and verifies if the spc is already +// migrated or not +func getSPC(spcName, openebsNamespace string) (*apis.StoragePoolClaim, bool, 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(). + WithNamespace(openebsNamespace).Get(spcName, metav1.GetOptions{}) + if err != nil { + return nil, false, errors.Wrapf(err, "failed to get equivalent cspc for spc %s", spcName) + } + return nil, true, nil + } + if err != nil { + return nil, false, err + } + return spcObj, false, nil +} + // csptocspi migrates a CSP to CSPI based on hostname -func csptocspi(cspiObj *apis.CStorPoolInstance, cspcObj *apis.CStorPoolCluster, openebsNamespace string) error { +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 cspLabel := hostnameLabel + "," + spcLabel @@ -143,14 +149,9 @@ func csptocspi(cspiObj *apis.CStorPoolInstance, cspcObj *apis.CStorPoolCluster, if err != nil { return err } - for _, bdName := range cspObj.Spec.Group[0].Item { - err = updateBDC(bdName, cspcObj, openebsNamespace) - if err != nil { - 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. + cspiObj.Annotations[string(apis.OldPoolName)] = "cstor-" + string(cspObj.UID) delete(cspiObj.Annotations, string(apis.OpenEBSDisableReconcileKey)) cspiObj, err = cspi.NewKubeClient(). WithNamespace(openebsNamespace). @@ -244,35 +245,54 @@ func scaleDownDeployment(cspObj *apis.CStorPool, openebsNamespace string) error // 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). - Get(bdName.Name, metav1.GetOptions{}) - if err != nil { - return err - } - bdcObj, err := bdc.NewKubeClient(). - WithNamespace(openebsNamespace). - Get(bdObj.Spec.ClaimRef.Name, metav1.GetOptions{}) +func updateBDCLabels(cspcName, openebsNamespace string) error { + bdcList, err := bdc.NewKubeClient().List(metav1.ListOptions{ + LabelSelector: string(apis.StoragePoolClaimCPK) + "=" + cspcName, + }) if err != nil { return err } - klog.Infof("Updating bdc %s with cspc %s info.", bdcObj.Name, cspcObj.Name) - delete(bdcObj.Labels, string(apis.StoragePoolClaimCPK)) - bdcObj.Labels[string(apis.CStorPoolClusterCPK)] = cspcObj.Name - for i, finalizer := range bdcObj.Finalizers { - if finalizer == spcFinalizer { - bdcObj.Finalizers[i] = cspcFinalizer + for _, bdcObj := range bdcList.Items { + klog.Infof("Updating bdc %s with cspc labels & finalizer.", bdcObj.Name) + delete(bdcObj.Labels, string(apis.StoragePoolClaimCPK)) + bdcObj.Labels[string(apis.CStorPoolClusterCPK)] = cspcName + for i, finalizer := range bdcObj.Finalizers { + if finalizer == spcFinalizer { + bdcObj.Finalizers[i] = cspcFinalizer + } + } + // bdcObj.OwnerReferences[0].Kind = "CStorPoolCluster" + // bdcObj.OwnerReferences[0].UID = cspcObj.UID + _, err := bdc.NewKubeClient(). + WithNamespace(openebsNamespace). + Update(&bdcObj) + if err != nil { + return errors.Wrapf(err, "failed to update bdc %s with cspc label & finalizer", bdcObj.Name) } } - bdcObj.OwnerReferences[0].Kind = "CStorPoolCluster" - bdcObj.OwnerReferences[0].UID = cspcObj.UID - _, err = bdc.NewKubeClient(). - WithNamespace(openebsNamespace). - Update(bdcObj) + return nil +} + +// Update the bdc with the cspc OwnerReferences instead of spc OwnerReferences +// to allow clean up of bdcs on deletion of cspc. +func updateBDCOwnerRef(cspcObj *apis.CStorPoolCluster, openebsNamespace string) error { + bdcList, err := bdc.NewKubeClient().List(metav1.ListOptions{ + LabelSelector: string(apis.CStorPoolClusterCPK) + "=" + cspcObj.Name, + }) if err != nil { return err } + for _, bdcObj := range bdcList.Items { + klog.Infof("Updating bdc %s with cspc ownerRef.", bdcObj.Name) + bdcObj.OwnerReferences[0].Kind = "CStorPoolCluster" + bdcObj.OwnerReferences[0].UID = cspcObj.UID + _, err := bdc.NewKubeClient(). + WithNamespace(openebsNamespace). + Update(&bdcObj) + if err != nil { + return errors.Wrapf(err, "failed to update bdc %s with cspc onwerRef", bdcObj.Name) + } + } return nil } @@ -297,7 +317,7 @@ func updateCVRsLabels(cspName, openebsNamespace string, cspiObj *apis.CStorPoolI _, err = cvr.NewKubeclient().WithNamespace(openebsNamespace). Update(&cvrObj) if err != nil { - return err + return errors.Wrapf(err, "failed to update cvr %s with cspc info", cvrObj.Name) } } return nil