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 update copyset availflag #2455

Merged
merged 1 commit into from
Jun 5, 2023

Conversation

baytan0720
Copy link
Member

@baytan0720 baytan0720 commented May 5, 2023

What problem does this PR solve?

Issue Number: #2357

Problem Summary: support update availflag command in curve tool

What is changed and how it works?

What's Changed:
add tools-v2/pkg/cli/command/curvebs/list/unavailcopysets/unavailcopysets.go
add tools-v2/pkg/cli/command/curvebs/query/chunk/chunkinfo.go
add tools-v2/pkg/cli/command/curvebs/list/chunkserver/copyset_by_host.go
add tools-v2/pkg/cli/command/curvebs/set/set.go
add tools-v2/pkg/cli/command/curvebs/update/copyset/copyset.go
add tools-v2/pkg/cli/command/curvebs/update/copyset/availflag/availflag.go
modify tools-v2/internal/error/error.go
modify tools-v2/internal/utils/row.go
modify tools-v2/pkg/cli/command/curvebs/bs.go
modify tools-v2/pkg/config/bs.go
modify tools-v2/pkg/cli/command/curvebs/update/update.go
modify tools-v2/pkg/cli/command/curvebs/check/copyset/copyset.go

How it Works:

root@0ad49a8af612:/# curve bs update copyset-availflag -h               
Usage:  curve bs update copyset-availflag [flags]

set copyset availflag

Flags:
      --availflag             copysets available flag[required]
  -c, --conf string           config file (default is
                              $HOME/.curve/curve.yaml or
                              /etc/curve/curve.yaml)
      --dryrun                when dry run set true, no changes will be
                              made (default true)
  -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
      --rpcretrytimes int32   rpc retry times (default 1)
      --rpctimeout duration   rpc timeout (default 10s)
      --showerror             display all errors in command
      --verbose               show some log

Examples:
$ curve bs update copyset-availflag --availflag=true --dryrun=false

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 baytan0720 marked this pull request as draft May 5, 2023 10:48
@baytan0720 baytan0720 changed the title [feat]tools-v2: add set availflag [feat]tools-v2: add set copyset-availflag May 5, 2023
@baytan0720 baytan0720 force-pushed the availflag branch 2 times, most recently from 99a7afa to 0c358b5 Compare May 8, 2023 04:14
@Cyber-SiKu Cyber-SiKu marked this pull request as ready for review May 8, 2023 08:07
Copy link
Contributor

@aspirer aspirer left a comment

Choose a reason for hiding this comment

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

we also need to update doc in tools-v2/README.md.

you can refer to pr #2465

@@ -420,7 +420,18 @@ var (
ErrBsOpNameNotSupport = func() *CmdError {
return NewInternalCmdError(53, "not support op[%s], only support: operator, change_peer, add_peer, remove_peer, transfer_leader")
}

ErrBsGetChunkServerInCluster = func() *CmdError {
return NewInternalCmdError(53, "get chunkserver in cluster fail, err: %s")
Copy link
Contributor

Choose a reason for hiding this comment

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

the errorno 53 is already used above, we should start with errorno 54.

)

const (
copysetAvailflagExample = `$ curve bs update copyset availflag --availflag=true --dryrun=false`
Copy link
Contributor

Choose a reason for hiding this comment

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

set or update?
you use set in conversation, I think it should be update, we should correct it.

@baytan0720
Copy link
Member Author

we also need to update doc in tools-v2/README.md.

you can refer to pr #2465

Ok, I will fix all. This pr is not ready yet. I've been a little busy lately.

@baytan0720 baytan0720 changed the title [feat]tools-v2: add set copyset-availflag [feat]tools-v2: add update copyset availflag May 11, 2023
Comment on lines 110 to 111
CURVEBS_CHUNKSERVER_ADDRESS = "csaddress"
VIPER_CURVEBS_CHUNKSERVER_ADDRESS = "curvebs.csaddress"
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
CURVEBS_CHUNKSERVER_ADDRESS = "csaddress"
VIPER_CURVEBS_CHUNKSERVER_ADDRESS = "curvebs.csaddress"
CURVEBS_CHUNKSERVER_ADDRESS = "chunkserveraddr"
VIPER_CURVEBS_CHUNKSERVER_ADDRESS = "curvebs.chunkserverAddr"

type ChunkserverCommand struct {
basecmd.FinalCurveCmd
Rpc *GetChunkServerInClusterRpc
response *topology.GetChunkServerInClusterResponse
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
response *topology.GetChunkServerInClusterResponse
ChunkServerInfos []*topology.ChunkServerInfo

is better?

type UnAvailCopySetsCommand struct {
basecmd.FinalCurveCmd
Rpc *ListUnAvailCopySets
response *topology.ListUnAvailCopySetsResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -0,0 +1,167 @@
/*
* Copyright (c) 2022 NetEase Inc.
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
* Copyright (c) 2022 NetEase Inc.
* Copyright (c) 2023 NetEase Inc.

@@ -0,0 +1,131 @@
/*
* Copyright (c) 2022 NetEase Inc.
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
* Copyright (c) 2022 NetEase Inc.
* Copyright (c) 2023 NetEase Inc.

CopysetId: &copysetIds[i],
ChunkId: &chunkIds[i],
},
Info: &basecmd.Rpc{[]string{addressList[i]}, timeout, retrytimes, "GetChunkInfo"},
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
Info: &basecmd.Rpc{[]string{addressList[i]}, timeout, retrytimes, "GetChunkInfo"},
Info: &basecmd.NewRpc{[]string{addressList[i]}, timeout, retrytimes, "GetChunkInfo"},

@@ -0,0 +1,259 @@
/*
* Copyright (c) 2022 NetEase Inc.
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
* Copyright (c) 2022 NetEase Inc.
* Copyright (c) 2023 NetEase Inc.

@@ -0,0 +1,259 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

add use in readme

@baytan0720 baytan0720 force-pushed the availflag branch 3 times, most recently from c9d5cee to cc9f6cb Compare May 17, 2023 10:15
@opencurveadmin
Copy link
Collaborator

@Cyber-SiKu PTAL

Copy link
Contributor

Choose a reason for hiding this comment

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

this file has been implemented as chunkserver_cluster.go in the latest master branch. If necessary, use that.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's this file for? chunk is a different concept from chunkserver

Copy link
Contributor

Choose a reason for hiding this comment

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

list/chunkserver/copyset.go in current master do the same thing, use the current one.

@@ -0,0 +1,132 @@
/*
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 directly use chunkserver.go
Or migrate it over here and modify it according to your needs.

row[cobrautil.ROW_POOL_ID] = strconv.FormatUint(uint64(info.GetLogicalPoolId()), 10)
row[cobrautil.ROW_COPYSET_ID] = strconv.FormatUint(uint64(info.GetCopysetId()), 10)
row[cobrautil.ROW_AVAILFLAG] = fmt.Sprintf("%v => %v", !*cCmd.Rpc.Request.AvailFlag, *cCmd.Rpc.Request.AvailFlag)
row[cobrautil.ROW_DRYRUN] = "true"
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
row[cobrautil.ROW_DRYRUN] = "true"
row[cobrautil.ROW_DRYRUN] = cobrautil.ROW_VALUE_TRUE

row[cobrautil.ROW_POOL_ID] = strconv.FormatUint(uint64(info.GetLogicalPoolId()), 10)
row[cobrautil.ROW_COPYSET_ID] = strconv.FormatUint(uint64(info.GetCopysetId()), 10)
row[cobrautil.ROW_AVAILFLAG] = fmt.Sprintf("%v => %v", !*cCmd.Rpc.Request.AvailFlag, *cCmd.Rpc.Request.AvailFlag)
row[cobrautil.ROW_DRYRUN] = "false"
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
row[cobrautil.ROW_DRYRUN] = "false"
row[cobrautil.ROW_DRYRUN] = ROW_VALUE_FALSE

@baytan0720
Copy link
Member Author

cicheck

Signed-off-by: baytan0720 <baytan2@hotmail.com>
@baytan0720
Copy link
Member Author

cicheck

@Cyber-SiKu Cyber-SiKu merged commit 26a51f0 into opencurve:master Jun 5, 2023
3 checks passed
@baytan0720 baytan0720 deleted the availflag branch June 5, 2023 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants