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]tools-v2: add bs create volume snapshot #2772

Merged
merged 1 commit into from
Dec 26, 2023

Conversation

baytan0720
Copy link
Member

@baytan0720 baytan0720 commented Sep 23, 2023

What problem does this PR solve?

Issue Number: #2585

Problem Summary:

What is changed and how it works?

What's Changed:
modify tools-v2/internal/utils/snapshot.go
modify tools-v2/internal/utils/row.go
modify tools-v2/pkg/cli/command/curvebs/create/create.go
modify tools-v2/pkg/config/bs.go
add tools-v2/pkg/cli/command/curvebs/create/volume/snapshot/snapshot.go
add tools-v2/pkg/cli/command/curvebs/create/volume/volume.go

How it Works:

Usage:

curve bs create volume snapshot --user root --filename test --snapshotname snap-test

Output:

+------+----------+--------------+
| USER | FILENAME | SNAPSHOTNAME |
+------+----------+---------------
| root |   test   |  snap-test   |
+------+----------+--------------+

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@baytan0720
Copy link
Member Author

cicheck

1 similar comment
@baytan0720
Copy link
Member Author

cicheck

@baytan0720 baytan0720 force-pushed the create_volume_snapshot branch from a5e0878 to 448e3f1 Compare September 26, 2023 15:44
@baytan0720
Copy link
Member Author

cicheck

1 similar comment
@baytan0720
Copy link
Member Author

cicheck

@baytan0720 baytan0720 force-pushed the create_volume_snapshot branch from 448e3f1 to 067f655 Compare October 8, 2023 13:04
@baytan0720
Copy link
Member Author

cicheck

2 similar comments
@baytan0720
Copy link
Member Author

cicheck

@baytan0720
Copy link
Member Author

cicheck

@SeanHai SeanHai requested a review from caoxianfei1 October 10, 2023 02:36
@baytan0720 baytan0720 force-pushed the create_volume_snapshot branch from 067f655 to cd28014 Compare October 10, 2023 08:05
@baytan0720
Copy link
Member Author

cicheck

1 similar comment
@baytan0720
Copy link
Member Author

cicheck

@caoxianfei1
Copy link
Contributor

@baytan0720
image

@caoxianfei1
Copy link
Contributor

@baytan0720 need to rebase.

@baytan0720 baytan0720 force-pushed the create_volume_snapshot branch 2 times, most recently from 2a4e051 to 0786363 Compare November 3, 2023 10:33
@baytan0720 baytan0720 force-pushed the create_volume_snapshot branch from 0786363 to b762c43 Compare November 3, 2023 10:38
@baytan0720
Copy link
Member Author

cicheck

@baytan0720 baytan0720 closed this Nov 3, 2023
@baytan0720 baytan0720 reopened this Nov 3, 2023
@baytan0720
Copy link
Member Author

cicheck

1 similar comment
@baytan0720
Copy link
Member Author

cicheck

@baytan0720 baytan0720 requested a review from Cyber-SiKu November 6, 2023 15:28
@wuhongsong
Copy link
Contributor

@caoxianfei1 PTAL


Usage:
```bash
curve bs create volume snapshot --user root --filename test --snapshotname snap-test
Copy link
Contributor

Choose a reason for hiding this comment

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

The filename is '/test'?

metric := basecmd.NewMetric(sCmd.snapshotAddrs, subUri, sCmd.timeout)
result, err := basecmd.QueryMetric(metric)
if err.TypeCode() != cmderror.CODE_SUCCESS {
return err.ToError()
Copy link
Contributor

@caoxianfei1 caoxianfei1 Dec 12, 2023

Choose a reason for hiding this comment

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

we should get error code and return proper error description?

Copy link
Contributor

@caoxianfei1 caoxianfei1 left a comment

Choose a reason for hiding this comment

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

LGTM!

@caoxianfei1 caoxianfei1 merged commit 35846be into opencurve:master Dec 26, 2023
3 checks passed
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.

4 participants