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

fix(migrate): update lock file path when csp is scaled down #85

Merged
merged 2 commits into from Jan 19, 2021

Conversation

shubham14bajpai
Copy link
Contributor

@shubham14bajpai shubham14bajpai commented Jan 15, 2021

Signed-off-by: shubham shubham.bajpai@mayadata.io

What this PR does:
This PR handles the scenario where the csp deployments are scaled up by mistake. The path to the lock file is different for csp and cspi deployments which leads to multiple simultaneous imports. Patching the tmp path while scaling down the csp deployment fixes this problem.

csp Volumes

volumes:
    - hostPath:
        path: /dev
        type: Directory
        name: device
    - hostPath:
        path: /run/udev
        type: Directory
        name: udev
    - hostPath:
        path: /var/openebs/sparse/shared-cstor-pool
        type: DirectoryOrCreate
        name: tmp
    - hostPath:
        path: /var/openebs/sparse
        type: DirectoryOrCreate
        name: sparse
    - hostPath:
        path: /var/openebs/cstor-pool/cstor-pool
        type: DirectoryOrCreate
        name: storagepath
    - emptyDir: {}
        name: sockfile

cspi Volumes

volumes:
    - hostPath:
        path: /dev
        type: Directory
        name: device
    - hostPath:
        path: /run/udev
        type: Directory
        name: udev
    - hostPath:
        path: /var/openebs/cstor-pool/cstor-pool
        type: DirectoryOrCreate
        name: tmp
    - hostPath:
        path: /var/openebs/sparse
        type: DirectoryOrCreate
        name: sparse
    - hostPath:
        path: /var/openebs/cstor-pool/cstor-pool
        type: DirectoryOrCreate
        name: storagepath
    - emptyDir: {}
        name: sockfile

Which issue(s) this PR fixes:
Fixes #82

Special notes for your reviewer:

Checklist

  • PR messages has document related information
  • Labelled this PR & related issue with documentation tag
  • PR messages has breaking changes related information
  • PR messages has upgrade related information
  • Labelled this PR & related issue with requires-upgrade tag
  • Tests updated

Signed-off-by: shubham <shubham.bajpai@mayadata.io>
@shubham14bajpai shubham14bajpai linked an issue Jan 15, 2021 that may be closed by this pull request
@shubham14bajpai shubham14bajpai added this to Pre-commits and Designs - Due: Jan 31 2020 in 2.6 Release Tracker - Due Feb 15th. Jan 18, 2021
Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

I also need to go through an import code path/doc to understand in a case where the process was crashed during the renaming of the pool

pkg/migrate/cstor/pool.go Outdated Show resolved Hide resolved
pkg/migrate/cstor/pool.go Outdated Show resolved Hide resolved
pkg/migrate/cstor/pool.go Show resolved Hide resolved
Signed-off-by: shubham shubham.bajpai@mayadata.io

Co-authored-by: sai chaithanya <sai.chaithanya@mayadata.io>
Copy link

@mittachaitu mittachaitu left a comment

Choose a reason for hiding this comment

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

changes are good

newCSPDeploy.Spec.Template.Spec.Volumes = cspiDeploy.Spec.Template.Spec.Volumes
patchData, err := GetPatchData(cspDeployList.Items[0], newCSPDeploy)
if err != nil {
return errors.Wrapf(err, "failed to patch data for csp %s", cspName)

Choose a reason for hiding this comment

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

nit:

Suggested change
return errors.Wrapf(err, "failed to patch data for csp %s", cspName)
return errors.Wrapf(err, "failed to get patch data for csp deployment %s", cspName)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error returned by GetPatchData function is wrapped like "failed to generate patch data"

Copy link

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

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

lgtm

@prateekpandey14 prateekpandey14 merged commit 852b904 into openebs-archive:master Jan 19, 2021
2.6 Release Tracker - Due Feb 15th. automation moved this from Pre-commits and Designs - Due: Jan 31 2020 to Done Jan 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migration failure ended into data corruption
3 participants