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: bs clean-recycle #2401

Merged
merged 1 commit into from
Apr 26, 2023

Conversation

lng2020
Copy link
Contributor

@lng2020 lng2020 commented Apr 14, 2023

What problem does this PR solve?

Issue Number: #2032

Problem Summary: support clean-recycle command in curve tool

What is changed and how it works?

What's Changed: add /tools-v2/pkg/cli/command/curvebs/clean_recycle.go, modify /tools-v2/pkg/config/bs.go and /tools-v2/README.md

How it Works:
command help

root@5bf3486a47c4:/# /root/curve bs clean-recycle -h
Usage:  curve bs clean-recycle [flags]

clean recycle bin

Flags:
  -c, --conf string            config file (default is $HOME/.curve/curve.yaml or
                               /etc/curve/curve.yaml)
      --expiredtime string     expire time (default 0s)
  -f, --format string          output format (json|plain) (default "plain")
  -h, --help                   print help
      --mdsaddr string         mds address, should be like
                               127.0.0.1:6700,127.0.0.1:6701,127.0.0.1:6702
      --password string        user password
      --recycleprefix string   recycle prefix (default "")
      --rpcretrytimes int32    rpc retry times (default 1)
      --rpctimeout duration    rpc timeout (default 10s)
      --showerror              display all errors in command
      --user string            user name
      --verbose                show some log

Examples:
$ curve bs clean-recycle --expiredtime=1h --recycleprefix=/test

command usage

root@75e1ef28f110:/# curve_ops_tool create --fileName=/test --userName=root
root@75e1ef28f110:/# /root/curve bs delete file --filename=/test --user=root
+---------+--------+
| RESULT  | REASON |
+---------+--------+
| success |        |
+---------+--------+
root@75e1ef28f110:/# /root/curve bs list dir --dir=/RecycleBin
+----+-------------------------------+----------+----------------+-------+---------------------+---------------+----------+
| ID |           FILENAME            | PARENTID |    FILETYPE    | OWNER |        CTIME        | ALLOCATEDSIZE | FILESIZE |
+----+-------------------------------+----------+----------------+-------+---------------------+---------------+----------+
| 5  | /RecycleBin/test-5-1681442546 | 1        | INODE_PAGEFILE | root  | 2023-04-14 11:22:21 | 0 B           | 20 GiB   |
+----+-------------------------------+----------+----------------+-------+---------------------+---------------+----------+
root@75e1ef28f110:/# /root/curve bs clean-recycle
+---------+--------+
| RESULT  | REASON |
+---------+--------+
| success |        |
+---------+--------+
root@75e1ef28f110:/# /root/curve bs list dir --dir=/RecycleBin

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

@Cyber-SiKu
Copy link
Contributor

Please fix the conflict

@lng2020 lng2020 force-pushed the feature/curve-bs-clean-recycle branch from 7227f6b to a4fd1c9 Compare April 17, 2023 07:01
@Cyber-SiKu
Copy link
Contributor

I just merged into a PR, seems to be a bit of a conflict?

@lng2020
Copy link
Contributor Author

lng2020 commented Apr 17, 2023

Just some configurations in tools-v2/pkg/config/bs.go. I seem to be unable to resolve them locally (perhaps due to parentheses). Could you kindly help me resolve them?

var _ basecmd.FinalCurveCmdFunc = (*CleanRecycleCommand)(nil) // check interface

// DeleteCertainFileRpc
type DeleteCertainFileRpc struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can add a wrapper call to delete file, like this.

func GetSegment(caller *cobra.Command) (*nameserver2.GetOrAllocateSegmentResponse, *cmderror.CmdError) {
getCmd := NewQuerySegmentCommand()
config.AlignFlagsValue(caller, getCmd.Cmd, []string{
config.RPCRETRYTIMES, config.RPCTIMEOUT, config.CURVEBS_MDSADDR,
config.CURVEBS_PATH, config.CURVEBS_USER, config.CURVEBS_PASSWORD,
config.CURVEBS_OFFSET,
})
getCmd.Cmd.SilenceErrors = true
getCmd.Cmd.SilenceUsage = true
getCmd.Cmd.SetArgs([]string{"--format", config.FORMAT_NOOUT})
err := getCmd.Cmd.Execute()
if err != nil {
retErr := cmderror.ErrBsCreateFileOrDirectoryType()
retErr.Format(err.Error())
return getCmd.Response, retErr
}
return getCmd.Response, cmderror.Success()
}

Comment on lines 58 to 61
type ListDirRpc struct {
Info *basecmd.Rpc
Request *nameserver2.ListDirRequest
curveFSClient nameserver2.CurveFSServiceClient
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

VIPER_CURVEBS_OFFSET = "curvebs.offset"
CURVEBS_SIZE = "size"
VIPER_CURVEBS_SIZE = "curvebs.size"
CURVEBS_RECYCLE_PREFIX = "recycleprefix"
Copy link
Contributor

Choose a reason for hiding this comment

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

add default value

CURVEBS_RECYCLE_PREFIX = "recycleprefix"
VIPER_RECYCLE_PREFIX = "curvebs.recycleprefix"
CURVE_EXPIRED_TIME = "expiredtime"
VIPER_CURVE_EXPIRED_TIME = "curvebs.expiredtime"
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@Cyber-SiKu
Copy link
Contributor

Just some configurations in tools-v2/pkg/config/bs.go. I seem to be unable to resolve them locally (perhaps due to parentheses). Could you kindly help me resolve them?

image
keep you edit.

image
keep master.

image
keep master.

image
keep master.

@Cyber-SiKu
Copy link
Contributor

It is best to combine your commits into one.

@lng2020
Copy link
Contributor Author

lng2020 commented Apr 17, 2023

It is best to combine your commits into one.

Thank you for your help! I just noticed that the error handling has changed in the master branch, and I haven't addressed that in my pr. I will take your review suggestions and refactor the code accordingly. I expect to submit the changes tomorrow and hopefully resolve the issue.

Comment on lines 27 to 28
RECYCLEBINDIRNAME = "RecycleBin"
RECYCLEBINDIR = "/" + RECYCLEBINDIRNAME
Copy link
Contributor

Choose a reason for hiding this comment

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

merge to one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will do


// method of CleanRecycleCommand struct
func (crCmd *CleanRecycleCommand) Init(cmd *cobra.Command, args []string) error {
crCmd.recyclePrefix = config.GetBsRecyclePrefix(crCmd.Cmd)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What is this for?

Delete the files in the recycle bin that its original filename has the prefix. I'm following the code in old tool of curve.

@Cyber-SiKu
Copy link
Contributor

Cyber-SiKu commented Apr 17, 2023

what is your email or wechat?
image

@lng2020
Copy link
Contributor Author

lng2020 commented Apr 17, 2023

what is your email or wechat? image

Already in the wechat group. My nick name is lng and wechat id is lng3075707637.

@lng2020 lng2020 force-pushed the feature/curve-bs-clean-recycle branch from 0d0062c to 681850b Compare April 18, 2023 11:36
@Cyber-SiKu
Copy link
Contributor

cicheck

// CleanRecycleCommand
type CleanRecycleCommand struct {
basecmd.FinalCurveCmd
recyclePrefix string
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for?

@lng2020 lng2020 force-pushed the feature/curve-bs-clean-recycle branch 3 times, most recently from 6c55fb1 to 426d432 Compare April 23, 2023 09:47
CURVEBS_DIR = "dir"
VIPER_CURVEBS_DIR = "curvebs.dir"
CURVEBS_FORCE = "force"
CURVEBS_DEFAULT_FORCE = false
Copy link
Contributor

Choose a reason for hiding this comment

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

add VIPER_CURVEBS_FORCE

@@ -77,6 +75,10 @@ const (
CURVEBS_STRIPE_COUNT = "stripecount"
VIPER_CURVEBS_STRIPE_COUNT = "curvebs.stripecount"
CURVEBS_DEFAULT_STRIPE_COUNT = uint64(32)
CURVEBS_RECYCLE_PREFIX = "recycleprefix"
VIPER_RECYCLE_PREFIX = "curvebs.recycleprefix"
Copy link
Contributor

Choose a reason for hiding this comment

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

option flag should be given default value

AddBsStringOptionFlag(cmd, CURVEBS_RECYCLE_PREFIX, "recycle prefix (default \"\")")
}

func AddBsExpireTimeFlag(cmd *cobra.Command) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func AddBsExpireTimeFlag(cmd *cobra.Command) {
func AddBsExpireTimeOptionFlag(cmd *cobra.Command) {

@@ -400,3 +391,20 @@ func GetBsFlagInt32(cmd *cobra.Command, flagName string) int32 {
}
return value
}

// flag for clean recycle bin
func AddBsRecyclePrefixFlag(cmd *cobra.Command) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
func AddBsRecyclePrefixFlag(cmd *cobra.Command) {
func AddBsRecyclePrefixOptionFlag(cmd *cobra.Command) {

)

const (
cleanRecycleBinExample = `$ curve bs clean-recycle --expiredtime=1h --recycleprefix=/test`
Copy link
Contributor

Choose a reason for hiding this comment

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

curve bs delete recycle maybe better?


filename := RECYCLEBINDIR + "/" + fileInfo.GetFileName()
crCmd.Cmd.Flags().Set(config.CURVEBS_PATH, filename)
crCmd.Cmd.Flags().Set(config.CURVEBS_FORCE, "true")
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a constant definition in tools-v2/internal/utils/string.go?

}

out := make(map[string]string)
out[cobrautil.ROW_RESULT] = "success"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
out[cobrautil.ROW_RESULT] = "success"
out[cobrautil.ROW_RESULT] = cobrautil.ROW_VALUE_SUCCESS

list := cobrautil.Map2List(out, []string{cobrautil.ROW_RESULT})
crCmd.TableNew.Append(list)

crCmd.Result = "success"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
crCmd.Result = "success"
crCmd.Result = out

@lng2020 lng2020 force-pushed the feature/curve-bs-clean-recycle branch 2 times, most recently from bb259c1 to 35885f0 Compare April 24, 2023 07:38
@Cyber-SiKu
Copy link
Contributor

cicheck

1 similar comment
@Cyber-SiKu
Copy link
Contributor

cicheck

Signed-off-by: lng2020 <[email protected]>
@lng2020 lng2020 force-pushed the feature/curve-bs-clean-recycle branch from 35885f0 to a8ee8dc Compare April 26, 2023 07:18
@Cyber-SiKu
Copy link
Contributor

cicheck

@Cyber-SiKu Cyber-SiKu merged commit 61c07b3 into opencurve:master Apr 26, 2023
@lng2020 lng2020 deleted the feature/curve-bs-clean-recycle branch April 26, 2023 09:22
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.

3 participants