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.

Sign up for GitHub

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

ctl: replace doRequest with HTTP client to get health status #8212

Merged
merged 18 commits into from
May 27, 2024
Merged

ctl: replace doRequest with HTTP client to get health status #8212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f1fb830
command: replace doRequest of HTTP client to get health status
okJiang May 23, 2024
ee483b0
add a note
okJiang May 23, 2024
9f60979
tools/ut: add a parallel parameter (#8186)
HuSharp May 23, 2024
27a74a5
metrics: fix the duplicate avg metrics (#8210)
nolouch May 23, 2024
09bc239
Apply suggestions from code review
okJiang May 23, 2024
ec44793
Merge branch 'master' into pdctl-health
okJiang May 23, 2024
fd7e738
fix comment
okJiang May 23, 2024
794c735
Merge branch 'pdctl-health' of https://github.com/okJiang/pd into pdc…
okJiang May 23, 2024
b123494
fix comment: add ut
okJiang May 23, 2024
d31faf8
fix lint
okJiang May 23, 2024
70d567c
Merge branch 'master' into pdctl-health
okJiang May 23, 2024
cef9427
fix ci
okJiang May 24, 2024
f6f0f28
Merge branch 'pdctl-health' of https://github.com/okJiang/pd into pdc…
okJiang May 24, 2024
120c0d4
make check
okJiang May 24, 2024
a425cba
make check
okJiang May 24, 2024
eea4858
fix lint
okJiang May 24, 2024
76a0902
Merge branch 'master' of https://github.com/tikv/pd into pdctl-health
okJiang May 27, 2024
132fe16
revert comment
okJiang May 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/http/api.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
membersPrefix = "/pd/api/v1/members"
leaderPrefix = "/pd/api/v1/leader"
transferLeader = "/pd/api/v1/leader/transfer"
health = "/pd/api/v1/health"
// Config
Config = "/pd/api/v1/config"
ClusterVersion = "/pd/api/v1/config/cluster-version"
Expand Down
15 changes: 15 additions & 0 deletions client/http/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
GetStores(context.Context) (*StoresInfo, error)
GetStore(context.Context, uint64) (*StoreInfo, error)
SetStoreLabels(context.Context, int64, map[string]string) error
GetHealthStatus(context.Context) ([]Health, error)
/* Config-related interfaces */
GetConfig(context.Context) (map[string]any, error)
SetConfig(context.Context, map[string]any, ...float64) error
Expand Down Expand Up @@ -337,6 +338,20 @@
WithBody(jsonInput))
}

// GetHealthStatus gets the health status of the cluster.
func (c *client) GetHealthStatus(ctx context.Context) ([]Health, error) {
okJiang marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

BTW, could you please add tests for this new method in /tests/integrations/client/http_client_test.go?

Copy link
Member Author

Choose a reason for hiding this comment

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

b123494

var healths []Health
err := c.request(ctx, newRequestInfo().
WithName(getHealthStatusName).
WithURI(health).
WithMethod(http.MethodGet).
WithResp(&healths))
if err != nil {
return nil, err

Check warning on line 350 in client/http/interface.go

View check run for this annotation

Codecov / codecov/patch

client/http/interface.go#L350 

Added line #L350 was not covered by tests
}
return healths, nil
}

// GetConfig gets the configurations.
func (c *client) GetConfig(ctx context.Context) (map[string]any, error) {
var config map[string]any
Expand Down
1 change: 1 addition & 0 deletions client/http/request_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ const (
getStoresName = "GetStores"
getStoreName = "GetStore"
setStoreLabelsName = "SetStoreLabels"
getHealthStatusName = "GetHealthStatus"
getConfigName = "GetConfig"
setConfigName = "SetConfig"
getScheduleConfigName = "GetScheduleConfig"
Expand Down
9 changes: 9 additions & 0 deletions client/http/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -661,3 +661,12 @@ func stringToKeyspaceState(str string) (keyspacepb.KeyspaceState, error) {
return keyspacepb.KeyspaceState(0), fmt.Errorf("invalid KeyspaceState string: %s", str)
}
}

// Health reflects the cluster's health.
// NOTE: This type is moved from `server/api/health.go`, maybe move them to the same place later.
type Health struct {
Name string `json:"name"`
MemberID uint64 `json:"member_id"`
ClientUrls []string `json:"client_urls"`
Health bool `json:"health"`
}
1 change: 1 addition & 0 deletions pkg/dashboard/uiserver/embedded_assets_rewriter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
var once sync.Once

// Assets returns the Assets FileSystem of the dashboard UI
// NOTE: if you see "undefined: assets" error, please run `make dashboard-ui` in the root directory of the repository.
func Assets(cfg *config.Config) http.FileSystem {
once.Do(func() {
resPath := distroutil.MustGetResPath()
Expand Down
19 changes: 19 additions & 0 deletions tests/integrations/client/http_client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,3 +811,22 @@ func (suite *httpClientTestSuite) checkUpdateKeyspaceGCManagementType(mode mode,
re.True(ok)
re.Equal(expectGCManagementType, val)
}

func (suite *httpClientTestSuite) TestGetHealthStatus() {
suite.RunTestInTwoModes(suite.checkGetHealthStatus)
}

func (suite *httpClientTestSuite) checkGetHealthStatus(mode mode, client pd.Client) {
re := suite.Require()
env := suite.env[mode]

healths, err := client.GetHealthStatus(env.ctx)
re.NoError(err)
re.Len(healths, 2)
sort.Slice(healths, func(i, j int) bool {
return healths[i].Name < healths[j].Name
})
re.Equal("pd1", healths[0].Name)
re.Equal("pd2", healths[1].Name)
re.True(healths[0].Health && healths[1].Health)
}
17 changes: 6 additions & 11 deletions tools/pd-ctl/pdctl/command/health_command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,30 +15,25 @@
package command

import (
"net/http"

"github.com/spf13/cobra"
)

var (
healthPrefix = "pd/api/v1/health"
)

// NewHealthCommand return a health subcommand of rootCmd
func NewHealthCommand() *cobra.Command {
m := &cobra.Command{
Use: "health",
Short: "show all node's health information of the pd cluster",
Run: showHealthCommandFunc,
Use: "health",
Short: "show all node's health information of the PD cluster",
PersistentPreRunE: requirePDClient,
Copy link
Member

Choose a reason for hiding this comment

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

TestSendAndGetComponent ci failed :(

  • This is because the requirePDClient command executes the function shouldInitPDClient, which requires querying pd/api/v1/cluster.

  • The handler is replaced in TestSendAndGetComponent, so you need to add this router to the handler in TestSendAndGetComponent.

		mux.HandleFunc("/pd/api/v1/cluster", func(w http.ResponseWriter, r *http.Request) {
			cluster := &metapb.Cluster{
				Id: 0,
			}
			var rd render.Render
			rd.JSON(w, http.StatusOK, cluster)
		})

Copy link
Member

Choose a reason for hiding this comment

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

You can check code here

pd/tools/pd-ctl/tests/global_test.go

Lines 37 to 52 in 7e18a69

handler := func(context.Context, *server.Server) (http.Handler, apiutil.APIServiceGroup, error) {
mux := http.NewServeMux()
mux.HandleFunc("/pd/api/v1/health", func(w http.ResponseWriter, r *http.Request) {
callerID := apiutil.GetCallerIDOnHTTP(r)
for k := range r.Header {
log.Info("header", zap.String("key", k))
}
log.Info("caller id", zap.String("caller-id", callerID))
re.Equal(pdControlCallerID, callerID)
fmt.Fprint(w, callerID)
})
info := apiutil.APIServiceGroup{
IsCore: true,
}
return mux, info, nil
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for your reminder and suggestion. I will fix it tomorrow.

Run: showHealthCommandFunc,
}
return m
}

func showHealthCommandFunc(cmd *cobra.Command, _ []string) {
r, err := doRequest(cmd, healthPrefix, http.MethodGet, http.Header{})
health, err := PDCli.GetHealthStatus(cmd.Context())
if err != nil {
cmd.Println(err)
return
}
cmd.Println(r)
jsonPrint(cmd, health)
}
10 changes: 0 additions & 10 deletions tools/pd-ctl/tests/global_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,6 @@ func TestSendAndGetComponent(t *testing.T) {
})
// check http client api
// TODO: remove this comment after replacing dialClient with the PD HTTP client completely.
Copy link
Member

Choose a reason for hiding this comment

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

please retain this comment, rest LGTM!

mux.HandleFunc("/pd/api/v1/health", func(w http.ResponseWriter, r *http.Request) {
callerID := apiutil.GetCallerIDOnHTTP(r)
re.Equal(command.PDControlCallerID, callerID)
fmt.Fprint(w, callerID)
})
mux.HandleFunc("/pd/api/v1/stores", func(w http.ResponseWriter, r *http.Request) {
callerID := apiutil.GetCallerIDOnHTTP(r)
re.Equal(command.PDControlCallerID, callerID)
Expand Down Expand Up @@ -82,11 +77,6 @@ func TestSendAndGetComponent(t *testing.T) {
"id": 1
}`), string(output))

args = []string{"-u", pdAddr, "health"}
output, err = ExecuteCommand(cmd, args...)
re.NoError(err)
re.Equal(fmt.Sprintf("%s\n", command.PDControlCallerID), string(output))

args = []string{"-u", pdAddr, "store"}
output, err = ExecuteCommand(cmd, args...)
re.NoError(err)
Expand Down