Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(migrate): add support to rename spc while migrating to cspc #13

Merged
merged 6 commits into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cmd/migrate/executor/cstor_pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@ func NewMigratePoolJob() *cobra.Command {
options.spcName,
"cstor SPC name to be migrated. Run \"kubectl get spc\", to get spc-name")

cmd.Flags().StringVarP(&options.cspcName,
"cspc-name", "",
options.cspcName,
"[optional] custom cspc name. By default cspc is created with same name as spc")

return cmd
}

Expand All @@ -72,6 +77,10 @@ func (m *MigrateOptions) RunCStorSPCMigrate() error {

klog.Infof("Migrating spc %s to cspc", m.spcName)
migrator := cstor.CSPCMigrator{}
if m.cspcName != "" {
klog.Infof("using custom cspc name as %s", m.cspcName)
migrator.SetCSPCName(m.cspcName)
}
err := migrator.Migrate(m.spcName, m.openebsNamespace)
if err != nil {
klog.Error(err)
Expand Down
1 change: 1 addition & 0 deletions cmd/migrate/executor/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
type MigrateOptions struct {
openebsNamespace string
spcName string
cspcName string
pvName string
}

Expand Down
13 changes: 7 additions & 6 deletions pkg/migrate/cstor/cspc_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func getBDList(rg apis.BlockDeviceGroup) []cstor.CStorPoolInstanceBlockDevice {
return list
}

func (c *CSPCMigrator) getCSPCSpecForSPC() (*cstor.CStorPoolCluster, error) {
func (c *CSPCMigrator) getCSPCSpecForSPC(spcName string) (*cstor.CStorPoolCluster, error) {
cspClient := csp.KubeClient()
cspList, err := cspClient.List(metav1.ListOptions{
LabelSelector: string(apis.StoragePoolClaimCPK) + "=" + c.SPCObj.Name,
Expand All @@ -74,11 +74,12 @@ func (c *CSPCMigrator) getCSPCSpecForSPC() (*cstor.CStorPoolCluster, error) {
return nil, err
}
cspcObj := &cstor.CStorPoolCluster{}
cspcObj.Name = c.SPCObj.Name
cspcObj.Name = c.CSPCName
cspcObj.Annotations = map[string]string{
// This annotation will be used to disable reconciliation on the dependants.
// In this case that will be CSPI
types.OpenEBSDisableDependantsReconcileKey: "true",
"openebs.io/migrated-from": spcName,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Instead of passing spcName as an argument we can make use of c.SPCObj.Name

}
for _, cspObj := range cspList.Items {
cspDeployList, err := deploy.NewKubeClient().WithNamespace(c.OpenebsNamespace).
Expand Down Expand Up @@ -130,15 +131,15 @@ func getCSPAuxResources(cspDeploy appsv1.Deployment) *corev1.ResourceRequirement
}

// generateCSPC creates an equivalent cspc for the given spc object
func (c *CSPCMigrator) generateCSPC() (
func (c *CSPCMigrator) generateCSPC(spcName string) (
*cstor.CStorPoolCluster, error) {
cspcObj, err := c.OpenebsClientset.CstorV1().
CStorPoolClusters(c.OpenebsNamespace).Get(c.SPCObj.Name, metav1.GetOptions{})
CStorPoolClusters(c.OpenebsNamespace).Get(c.CSPCName, metav1.GetOptions{})
if !k8serrors.IsNotFound(err) && err != nil {
return nil, err
}
if err != nil {
cspcObj, err = c.getCSPCSpecForSPC()
cspcObj, err = c.getCSPCSpecForSPC(spcName)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -176,7 +177,7 @@ func (c *CSPCMigrator) generateCSPC() (
return nil, err
}
cspcObj, err = c.OpenebsClientset.CstorV1().
CStorPoolClusters(c.OpenebsNamespace).Get(c.SPCObj.Name, metav1.GetOptions{})
CStorPoolClusters(c.OpenebsNamespace).Get(c.CSPCName, metav1.GetOptions{})
if err != nil {
return nil, err
}
Expand Down
101 changes: 90 additions & 11 deletions pkg/migrate/cstor/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,12 @@ type CSPCMigrator struct {
CSPCObj *cstor.CStorPoolCluster
SPCObj *apis.StoragePoolClaim
OpenebsNamespace string
CSPCName string
}

// SetCSPCName is used to initialize custom name if provided
func (c *CSPCMigrator) SetCSPCName(name string) {
c.CSPCName = name
}

// Migrate ...
Expand All @@ -86,6 +92,13 @@ func (c *CSPCMigrator) Migrate(name, namespace string) error {
if err != nil {
return err
}
if c.CSPCName == "" {
c.CSPCName = name
shubham14bajpai marked this conversation as resolved.
Show resolved Hide resolved
}
err = c.checkForExistingCSPC(name)
if err != nil {
return err
}
err = c.migrate(name)
return err
}
Expand Down Expand Up @@ -133,12 +146,12 @@ func (c *CSPCMigrator) migrate(spcName string) error {
if err != nil {
return errors.Wrapf(err, "failed to validate spc %s", spcName)
}
err = c.updateBDCLabels(spcName)
err = c.updateBDCLabels()
if err != nil {
return errors.Wrapf(err, "failed to update bdc labels for spc %s", spcName)
}
klog.Infof("Creating equivalent cspc for spc %s", spcName)
c.CSPCObj, err = c.generateCSPC()
klog.Infof("Creating equivalent cspc %s for spc %s", c.CSPCName, spcName)
c.CSPCObj, err = c.generateCSPC(spcName)
if err != nil {
return err
}
Expand Down Expand Up @@ -178,6 +191,57 @@ func (c *CSPCMigrator) migrate(spcName string) error {
return nil
}

func (c *CSPCMigrator) checkForExistingCSPC(spcName string) error {
spcObj, err := spc.NewKubeClient().Get(spcName, metav1.GetOptions{})
if err == nil {
// if cspc-name flag is used and the spc does not have the cspc
// annotation add the cspc annotation
if spcObj.Annotations == nil || spcObj.Annotations[types.CStorPoolClusterLabelKey] == "" {
if spcName != c.CSPCName {
shubham14bajpai marked this conversation as resolved.
Show resolved Hide resolved
err = addCSPCAnnotationToSPC(spcObj, c.CSPCName)
if err != nil {
return err
}
}
} else {
// if the first attempt at using cspc-name flag fails check for the new cspc
// name proivded and if no cspc exists update the annotation
if spcObj.Annotations[types.CStorPoolClusterLabelKey] != c.CSPCName {
_, err := c.OpenebsClientset.CstorV1().
CStorPoolClusters(c.OpenebsNamespace).
Get(c.CSPCName, metav1.GetOptions{})
if k8serrors.IsNotFound(err) {
err = addCSPCAnnotationToSPC(spcObj, c.CSPCName)
return err
}
if err != nil {
return err
}
return errors.Errorf("invalid cspc-name: the spc %s already set to be renamed as %s",
shubham14bajpai marked this conversation as resolved.
Show resolved Hide resolved
spcName,
spcObj.Annotations[types.CStorPoolClusterLabelKey],
)
}
}
}
if err != nil && !k8serrors.IsNotFound(err) {
return err
}
cspc, err := c.OpenebsClientset.CstorV1().
CStorPoolClusters(c.OpenebsNamespace).
Get(c.CSPCName, metav1.GetOptions{})
if k8serrors.IsNotFound(err) {
return nil
}
if err == nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I have another concern with configuration if the user does in below manner:

  • The user has multiple SPCs in his setup and launched a job(By applying job YAML by setting cspc-name argument) to migrate to CSPC(Consider pool is successfully migrated).
  • He reused the same Job YAML to migrate another SPC but now somehow he didn't change the cspc-name with this steps things will wrong(I can see job will update the blockdeviceclaim labels with cspc-name argument and return error).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can overcome this by adding old spc name on the cspc and verify that it isn't from another migrated spc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, this problem exists but now it will patch SPC and fail(Can we prevent updating SPC annotation itself?) @shubham14bajpai

if cspc.Annotations == nil || cspc.Annotations["openebs.io/migrated-from"] != spcName {
return errors.Errorf("cspc with same name %s already exists. Use the flag \"--cspc-name\" to rename the spc during migration", c.CSPCName)
}
return nil
}
return err
}

// validateSPC determines that if the spc is allowed to migrate or not.
// If the max pool count does not match the number of csp in case auto spc provisioning,
// or the blocldevice list in spc does not match bds from the csp, in case of manual provisioning
Expand Down Expand Up @@ -232,9 +296,9 @@ func (c *CSPCMigrator) getSPCWithMigrationStatus(spcName string) (*apis.StorageP
if k8serrors.IsNotFound(err) {
klog.Infof("spc %s not found.", spcName)
_, err = c.OpenebsClientset.CstorV1().
CStorPoolClusters(c.OpenebsNamespace).Get(spcName, metav1.GetOptions{})
CStorPoolClusters(c.OpenebsNamespace).Get(c.CSPCName, metav1.GetOptions{})
if err != nil {
return nil, false, errors.Wrapf(err, "failed to get equivalent cspc for spc %s", spcName)
return nil, false, errors.Wrapf(err, "failed to get equivalent cspc %s for spc %s", c.CSPCName, spcName)
}
return nil, true, nil
}
Expand All @@ -248,7 +312,7 @@ func (c *CSPCMigrator) getSPCWithMigrationStatus(spcName string) (*apis.StorageP
func (c *CSPCMigrator) cspTocspi(cspiObj *cstor.CStorPoolInstance) error {
var err1 error
hostnameLabel := types.HostNameLabelKey + "=" + cspiObj.Labels[types.HostNameLabelKey]
spcLabel := string(apis.StoragePoolClaimCPK) + "=" + c.CSPCObj.Name
spcLabel := string(apis.StoragePoolClaimCPK) + "=" + c.SPCObj.Name
cspLabel := hostnameLabel + "," + spcLabel
cspObj, err := getCSP(cspLabel)
if err != nil {
Expand Down Expand Up @@ -374,9 +438,9 @@ func (c *CSPCMigrator) scaleDownDeployment(cspObj *apis.CStorPool, openebsNamesp

// Update the bdc with the cspc labels instead of spc labels to allow
// filtering of bds claimed by the migrated cspc.
func (c *CSPCMigrator) updateBDCLabels(cspcName string) error {
func (c *CSPCMigrator) updateBDCLabels() error {
bdcList, err := c.OpenebsClientset.OpenebsV1alpha1().BlockDeviceClaims(c.OpenebsNamespace).List(metav1.ListOptions{
LabelSelector: string(apis.StoragePoolClaimCPK) + "=" + cspcName,
LabelSelector: string(apis.StoragePoolClaimCPK) + "=" + c.SPCObj.Name,
})
if err != nil {
return err
Expand All @@ -387,7 +451,7 @@ func (c *CSPCMigrator) updateBDCLabels(cspcName string) error {
bdcObj := &bdcItem
klog.Infof("Updating bdc %s with cspc labels & finalizer.", bdcObj.Name)
delete(bdcObj.Labels, string(apis.StoragePoolClaimCPK))
bdcObj.Labels[types.CStorPoolClusterLabelKey] = cspcName
bdcObj.Labels[types.CStorPoolClusterLabelKey] = c.CSPCName
for i, finalizer := range bdcObj.Finalizers {
if finalizer == spcFinalizer {
bdcObj.Finalizers[i] = cspcFinalizer
Expand Down Expand Up @@ -466,13 +530,28 @@ func (c *CSPCMigrator) updateCVRsLabels(cspObj *apis.CStorPool, cspiObj *cstor.C

func addSkipAnnotationToSPC(spcObj *apis.StoragePoolClaim) error {
retry:
spcObj.Annotations = map[string]string{
"openebs.io/skip-validations": "true",
if spcObj.Annotations == nil {
spcObj.Annotations = map[string]string{}
}
spcObj.Annotations["openebs.io/skip-validations"] = "true"
_, err := spc.NewKubeClient().Update(spcObj)
if k8serrors.IsConflict(err) {
klog.Errorf("failed to update spc with skip-validation annotation due to conflict error")
goto retry
}
return err
}

func addCSPCAnnotationToSPC(spcObj *apis.StoragePoolClaim, cspcName string) error {
retry:
if spcObj.Annotations == nil {
spcObj.Annotations = map[string]string{}
}
spcObj.Annotations[types.CStorPoolClusterLabelKey] = cspcName
_, err := spc.NewKubeClient().Update(spcObj)
if k8serrors.IsConflict(err) {
klog.Errorf("failed to update spc with cspc annotation due to conflict error")
shubham14bajpai marked this conversation as resolved.
Show resolved Hide resolved
goto retry
}
return err
}
13 changes: 8 additions & 5 deletions pkg/migrate/cstor/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ func (s *SnapshotMigrator) migrate(pvName string) error {
}

func (s *SnapshotMigrator) migrateSnapshots() error {
_, err := s.snapClient.SnapshotV1beta1().VolumeSnapshotClasses().
Get(snapClass, metav1.GetOptions{})
if err != nil {
return errors.Wrapf(err, "failed to get snapshotclass %s", snapClass)
}
snapshotList, err := snap.NewKubeClient().
WithNamespace("").
List(metav1.ListOptions{
Expand All @@ -70,6 +65,14 @@ func (s *SnapshotMigrator) migrateSnapshots() error {
if err != nil {
return err
}
if len(snapshotList.Items) == 0 {
return nil
}
_, err = s.snapClient.SnapshotV1beta1().VolumeSnapshotClasses().
Get(snapClass, metav1.GetOptions{})
if err != nil {
return errors.Wrapf(err, "failed to get snapshotclass %s", snapClass)
}
for _, snapshot := range snapshotList.Items {
snapshot := snapshot // pin it
if len(snapshot.Spec.SnapshotDataName) == 0 {
Expand Down