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

🍀 Proposal: Support for rollback in case of error in intermediate steps #1173

Closed
aFlyBird0 opened this issue Oct 13, 2022 · 5 comments
Closed
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@aFlyBird0
Copy link
Member

What Would You Like to Add? Why Is This Needed?

// applyUpgrade use os.Rename replace old version dtm with the latest one
// (1) rename current dtm file name to `dtm-bak`.
// (2) rename `dtm-tmp` to current dtm file name.
// (3) grant new dtm file execute permission.
// (4) remove `dtm-bak` binary file.
// TODO(hxcGit): Support for rollback in case of error in intermediate steps
func applyUpgrade(workDir string) error {
dtmFilePath := filepath.Join(workDir, dtmFileName)
dtmBakFilePath := filepath.Join(workDir, dtmBakFileName)
dtmTmpFilePath := filepath.Join(workDir, dtmTmpFileName)
if err := os.Rename(dtmFilePath, dtmBakFilePath); err != nil {
return err
}
log.Debugf("Dtm upgrade: rename %s to dtm-bak successfully.", dtmFileName)
if err := os.Rename(dtmTmpFilePath, dtmFilePath); err != nil {
return err
}
log.Debugf("Dtm upgrade: rename dtm-tmp to %s successfully.", dtmFileName)
if err := os.Chmod(dtmFilePath, 0755); err != nil {
return err
}
log.Debugf("Dtm upgrade: grant %s execute permission successfully.", dtmFileName)
if err := os.Remove(dtmBakFilePath); err != nil {
return err
}
log.Debug("Dtm upgrade: remove dtm-bak successfully.")
return nil
}

Design

Anything else

@aFlyBird0 aFlyBird0 added enhancement New feature or request help wanted Extra attention is needed labels Oct 13, 2022
@0zyt
Copy link
Contributor

0zyt commented Oct 16, 2022

I want to do it

@aFlyBird0
Copy link
Member Author

I want to do it

Do as much as you like!

@0zyt
Copy link
Contributor

0zyt commented Oct 16, 2022

@xavier-hou Hi,Do you think it is necessary to consider being interrupted by a signal such as CTRL+C?

@0zyt
Copy link
Contributor

0zyt commented Oct 16, 2022

This is the commit on my branch:0zyt@3f62a93

I had a discussion with @aFlyBird0 about writing test for this feature because I couldn't find a better way to test this feature.

06a239906039e95fe4b78db8e2cdbff

The codes above from @aFlyBird0 ,

I think this is a practical and effective way to test, but it doesn't look elegant at first glance.
The other way is to keep it simple and elegant and not change the function signature and to verify it locally by me or other members, but this is not a transparent way to test an open source project and may not be good enough.
So I would like to hear your thoughts.

@xavier-hou @Danielhui @steinliber @IronCore864

I will PR after getting a better way.

@IronCore864
Copy link
Member

nd a better way to test this feature.

I think this is a way to test it, but as you said, it changes the signature of the func, which might be a bit confusing to read.

I personally suggest a better way to test it. What do you guys think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants