tools: add component subcommand #2092

rleungx · 2020-01-09T12:43:36Z

What problem does this PR solve?

We need a way to support change the config of other components.

What is changed and how it works?

This PR adds a component subcommand to manipulate the config manager's configuration.

Check List

Tests

Unit test

codecov-io · 2020-02-05T09:15:58Z

Codecov Report

Merging #2092 into master will decrease coverage by 0.71%.
The diff coverage is 33.08%.

@@            Coverage Diff             @@
##           master    #2092      +/-   ##
==========================================
- Coverage   76.68%   75.96%   -0.72%     
==========================================
  Files         191      195       +4     
  Lines       19902    20228     +326     
==========================================
+ Hits        15262    15367     +105     
- Misses       3475     3686     +211     
- Partials     1165     1175      +10

Impacted Files	Coverage Δ
server/config/config.go	`85.92% <ø> (ø)`	⬆️
server/api/util.go	`67.69% <ø> (ø)`	⬆️
tools/pd-ctl/pdctl/command/global.go	`68.46% <0%> (-3.23%)`	⬇️
server/api/server.go	`100% <100%> (ø)`	⬆️
server/region_syncer/client.go	`84.48% <100%> (+3.44%)`	⬆️
client/base_client.go	`84.82% <100%> (ø)`	⬆️
pkg/dashboard/apiserver/apiserver.go	`81.81% <100%> (ø)`
tools/pd-ctl/pdctl/ctl.go	`95% <100%> (+0.12%)`	⬆️
tools/pd-ctl/pdctl/command/component_command.go	`34.54% <34.54%> (ø)`
server/api/router.go	`81.42% <37.73%> (-17.83%)`	⬇️
... and 23 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 23d632c...9dd25d5. Read the comment docs.

server/server.go

server/api/router.go

NingLin-P · 2020-02-10T06:19:59Z

LGTM

rleungx · 2020-02-10T06:35:28Z

/run-all-tests

overvenus

LGTM

shafreeck · 2020-02-13T03:41:14Z

server/api/router.go

+
+type customMarshaler struct{}
+
+func (c *customMarshaler) Marshal(v interface{}) ([]byte, error) {


Why are they all empty implementations?

Because we need to implement Marshaler interface here

how about nopMarshaler

tools/pd-ctl/pdctl/command/component_command.go

client/base_client.go

server/api/config_test.go

tools/pd-ctl/pdctl/command/component_command.go

disksing · 2020-02-14T05:46:10Z

server/api/middleware.go

+}
+
+func (m componentMiddleware) Middleware(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {


I wonder if it is suitable to use middleware here. After all, the logic here is too complicated.

Any better idea?

disksing · 2020-02-14T05:48:52Z

server/api/middleware.go

+}
+
+func (m componentMiddleware) Middleware(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {


I wonder if it is suitable to use middleware here. After all, the logic here is too complicated.

Signed-off-by: Ryan Leung <[email protected]>

disksing · 2020-02-17T02:45:10Z

server/api/router.go

+
+type customMarshaler struct{}
+
+func (c *customMarshaler) Marshal(v interface{}) ([]byte, error) {


how about nopMarshaler

disksing · 2020-02-17T02:54:42Z

server/api/router.go

@@ -34,7 +45,7 @@ func createIndentRender() *render.Render {
 	})
 }

-func createRouter(prefix string, svr *server.Server) *mux.Router {
+func createRouter(ctx context.Context, prefix string, svr *server.Server) (*mux.Router, func()) {


could you add some comments to clarify what's the returned func() used for?

disksing · 2020-02-17T02:59:45Z

server/api/router.go

@@ -188,5 +199,77 @@ func createRouter(prefix string, svr *server.Server) *mux.Router {
 	// Deprecated
 	rootRouter.HandleFunc("/ping", func(w http.ResponseWriter, r *http.Request) {}).Methods("GET")

-	return rootRouter
+	if svr.GetConfig().EnableDynamicConfig {
+		f := func() {


seems better define it separetly

Signed-off-by: Ryan Leung <[email protected]>

rleungx · 2020-02-17T07:23:07Z

PTAL. @disksing

disksing · 2020-02-17T08:54:31Z

server/api/router.go

+		f := func() {
+			lazyComponentRouter(ctx, svr, apiRouter)
+		}
+		return rootRouter, f


return rootRouter, func(){lazyComponentRouter(ctx, svr, apiRouter)}

Signed-off-by: Ryan Leung <[email protected]>

rleungx · 2020-02-17T12:34:04Z

/merge

sre-bot · 2020-02-17T12:36:21Z

/run-all-tests

rleungx added the area/pdctl label Jan 9, 2020

rleungx force-pushed the config-subcommand branch from 765ae13 to 2861a84 Compare January 19, 2020 09:11

rleungx force-pushed the config-subcommand branch 2 times, most recently from 12ffe35 to ee3adb8 Compare February 5, 2020 09:04

rleungx requested review from nolouch, lhy1024, NingLin-P and overvenus February 5, 2020 09:18

rleungx added this to the v4.0.0-beta.1 milestone Feb 5, 2020

overvenus reviewed Feb 7, 2020

View reviewed changes

server/server.go Outdated Show resolved Hide resolved

server/api/router.go Outdated Show resolved Hide resolved

overvenus approved these changes Feb 10, 2020

View reviewed changes

shafreeck reviewed Feb 13, 2020

View reviewed changes

disksing reviewed Feb 14, 2020

View reviewed changes

rleungx force-pushed the config-subcommand branch from 29dfbd4 to a0b1421 Compare February 14, 2020 10:39

rleungx added 13 commits February 14, 2020 20:34

add config manager

62208aa

Signed-off-by: Ryan Leung <[email protected]>

address comments

2a18e82

Signed-off-by: Ryan Leung <[email protected]>

add config gRPC service

0d4595a

Signed-off-by: Ryan Leung <[email protected]>

support from HTTP API to gRPC gateway for config

7f7ac7e

Signed-off-by: Ryan Leung <[email protected]>

make original HTTP API compatible with config manager

123cb7d

Signed-off-by: Ryan Leung <[email protected]>

add config client

5d95816

Signed-off-by: Ryan Leung <[email protected]>

support from HTTP API to gRPC gateway for config

5bd5131

Signed-off-by: Ryan Leung <[email protected]>

add component subcommand

cea2ee1

Signed-off-by: Ryan Leung <[email protected]>

support component show subcommand

da52e42

Signed-off-by: Ryan Leung <[email protected]>

address comments

3395025

Signed-off-by: Ryan Leung <[email protected]>

resolve the conflict

dec3ef0

Signed-off-by: Ryan Leung <[email protected]>

remove the prefix of URL

f0324fc

Signed-off-by: Ryan Leung <[email protected]>

modify getUpdateValue

f157a55

Signed-off-by: Ryan Leung <[email protected]>

rleungx added 6 commits February 14, 2020 20:44

update kvproto

9979baf

Signed-off-by: Ryan Leung <[email protected]>

handle more status

000bf7d

Signed-off-by: Ryan Leung <[email protected]>

address comments

7f14ea7

Signed-off-by: Ryan Leung <[email protected]>

validate schedule and replication config

7b56b63

Signed-off-by: Ryan Leung <[email protected]>

address comments

15dda70

Signed-off-by: Ryan Leung <[email protected]>

resolve the conflicts

9df4c54

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the config-subcommand branch from a0b1421 to 9df4c54 Compare February 14, 2020 12:57

disksing reviewed Feb 17, 2020

View reviewed changes

address comments

c83d718

Signed-off-by: Ryan Leung <[email protected]>

disksing reviewed Feb 17, 2020

View reviewed changes

address the comment

9dd25d5

Signed-off-by: Ryan Leung <[email protected]>

disksing approved these changes Feb 17, 2020

View reviewed changes

sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Feb 17, 2020

Merge branch 'master' into config-subcommand

f1ae5cb

sre-bot merged commit 249b42d into tikv:master Feb 17, 2020

rleungx mentioned this pull request Feb 20, 2020

pkg: remove key visual #2146

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools: add component subcommand #2092

tools: add component subcommand #2092

rleungx commented Jan 9, 2020

codecov-io commented Feb 5, 2020 •

edited

Loading

NingLin-P commented Feb 10, 2020

rleungx commented Feb 10, 2020

overvenus left a comment

shafreeck Feb 13, 2020

rleungx Feb 14, 2020

disksing Feb 17, 2020

disksing Feb 14, 2020

rleungx Feb 14, 2020

disksing Feb 14, 2020

disksing Feb 17, 2020

disksing Feb 17, 2020

disksing Feb 17, 2020

rleungx commented Feb 17, 2020

disksing Feb 17, 2020

rleungx commented Feb 17, 2020

sre-bot commented Feb 17, 2020


		type customMarshaler struct{}

		func (c *customMarshaler) Marshal(v interface{}) ([]byte, error) {

tools: add component subcommand #2092

tools: add component subcommand #2092

Conversation

rleungx commented Jan 9, 2020

What problem does this PR solve?

What is changed and how it works?

Check List

codecov-io commented Feb 5, 2020 • edited Loading

Codecov Report

NingLin-P commented Feb 10, 2020

rleungx commented Feb 10, 2020

overvenus left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rleungx commented Feb 17, 2020

Choose a reason for hiding this comment

rleungx commented Feb 17, 2020

sre-bot commented Feb 17, 2020

codecov-io commented Feb 5, 2020 •

edited

Loading