From 7d8a56019bc6b83378972bc29267e1915303bb10 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Tue, 20 Feb 2024 14:22:28 +0100 Subject: [PATCH] [MM-56348] system/ping: add new method with options (#26079) --- api/v4/source/system.yaml | 11 ++++++ server/channels/api4/system.go | 3 +- server/cmd/mmctl/client/client.go | 1 + server/cmd/mmctl/commands/system.go | 6 +++- server/cmd/mmctl/commands/system_test.go | 10 ++++-- server/cmd/mmctl/mocks/client_mock.go | 16 +++++++++ server/public/model/client4.go | 46 ++++++++++++++---------- server/public/model/system.go | 11 ++++++ webapp/platform/client/src/client4.ts | 4 +-- 9 files changed, 83 insertions(+), 25 deletions(-) diff --git a/api/v4/source/system.yaml b/api/v4/source/system.yaml index dbd1718684..cf8bfc5538 100644 --- a/api/v4/source/system.yaml +++ b/api/v4/source/system.yaml @@ -47,6 +47,11 @@ __Minimum server version__: 6.5 + If "use_rest_semantics" is set to true in the query, the endpoint will not return + an error status code in the header if the request is somehow completed successfully. + + __Minimum server version__: 9.6 + ##### Permissions None. @@ -64,6 +69,12 @@ required: false schema: type: string + - name: use_rest_semantics + in: query + description: Returns 200 status code even if the server status is unhealthy. + required: false + schema: + type: boolean responses: "200": description: Status of the system diff --git a/server/channels/api4/system.go b/server/channels/api4/system.go index 49f3feadd9..ad6938e2a8 100644 --- a/server/channels/api4/system.go +++ b/server/channels/api4/system.go @@ -179,6 +179,7 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) { s[filestoreStatusKey] = model.StatusOk appErr := c.App.TestFileStoreConnection() if appErr != nil { + c.Logger.Warn("Unable to test filestore connection.", mlog.Err(appErr)) s[filestoreStatusKey] = model.StatusUnhealthy s[model.STATUS] = model.StatusUnhealthy } @@ -194,7 +195,7 @@ func getSystemPing(c *Context, w http.ResponseWriter, r *http.Request) { s["ActiveSearchBackend"] = c.App.ActiveSearchBackend() - if s[model.STATUS] != model.StatusOk { + if s[model.STATUS] != model.StatusOk && r.FormValue("use_rest_semantics") != "true" { w.WriteHeader(http.StatusInternalServerError) } w.Write([]byte(model.MapToJSON(s))) diff --git a/server/cmd/mmctl/client/client.go b/server/cmd/mmctl/client/client.go index f6d476d789..ef90c1a3e8 100644 --- a/server/cmd/mmctl/client/client.go +++ b/server/cmd/mmctl/client/client.go @@ -119,6 +119,7 @@ type Client interface { MigrateAuthToSaml(ctx context.Context, fromAuthService string, usersMap map[string]string, auto bool) (*model.Response, error) GetPing(ctx context.Context) (string, *model.Response, error) GetPingWithFullServerStatus(ctx context.Context) (map[string]string, *model.Response, error) + GetPingWithOptions(ctx context.Context, options model.SystemPingOptions) (map[string]string, *model.Response, error) CreateUpload(ctx context.Context, us *model.UploadSession) (*model.UploadSession, *model.Response, error) GetUpload(ctx context.Context, uploadID string) (*model.UploadSession, *model.Response, error) GetUploadsForUser(ctx context.Context, userID string) ([]*model.UploadSession, *model.Response, error) diff --git a/server/cmd/mmctl/commands/system.go b/server/cmd/mmctl/commands/system.go index 3f92403aa5..d29b7674e0 100644 --- a/server/cmd/mmctl/commands/system.go +++ b/server/cmd/mmctl/commands/system.go @@ -12,6 +12,7 @@ import ( "github.com/pkg/errors" "github.com/spf13/cobra" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/cmd/mmctl/client" "github.com/mattermost/mattermost/server/v8/cmd/mmctl/printer" ) @@ -150,7 +151,10 @@ func systemVersionCmdF(c client.Client, cmd *cobra.Command, _ []string) error { func systemStatusCmdF(c client.Client, cmd *cobra.Command, _ []string) error { printer.SetSingle(true) - status, _, err := c.GetPingWithFullServerStatus(context.TODO()) + status, _, err := c.GetPingWithOptions(context.TODO(), model.SystemPingOptions{ + FullStatus: true, + RESTSemantics: true, + }) if err != nil { return fmt.Errorf("unable to fetch server status: %w", err) } diff --git a/server/cmd/mmctl/commands/system_test.go b/server/cmd/mmctl/commands/system_test.go index 734d807059..e0c84b371f 100644 --- a/server/cmd/mmctl/commands/system_test.go +++ b/server/cmd/mmctl/commands/system_test.go @@ -188,7 +188,10 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() { expectedStatus := map[string]string{"status": "OK"} s.client. EXPECT(). - GetPingWithFullServerStatus(context.Background()). + GetPingWithOptions(context.Background(), model.SystemPingOptions{ + FullStatus: true, + RESTSemantics: true, + }). Return(expectedStatus, &model.Response{}, nil). Times(1) @@ -204,7 +207,10 @@ func (s *MmctlUnitTestSuite) TestServerStatusCmd() { s.client. EXPECT(). - GetPingWithFullServerStatus(context.Background()). + GetPingWithOptions(context.Background(), model.SystemPingOptions{ + FullStatus: true, + RESTSemantics: true, + }). Return(nil, &model.Response{}, errors.New("mock error")). Times(1) diff --git a/server/cmd/mmctl/mocks/client_mock.go b/server/cmd/mmctl/mocks/client_mock.go index c67b5a9e09..927dc617d5 100644 --- a/server/cmd/mmctl/mocks/client_mock.go +++ b/server/cmd/mmctl/mocks/client_mock.go @@ -1054,6 +1054,22 @@ func (mr *MockClientMockRecorder) GetPingWithFullServerStatus(arg0 interface{}) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPingWithFullServerStatus", reflect.TypeOf((*MockClient)(nil).GetPingWithFullServerStatus), arg0) } +// GetPingWithOptions mocks base method. +func (m *MockClient) GetPingWithOptions(arg0 context.Context, arg1 model.SystemPingOptions) (map[string]string, *model.Response, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPingWithOptions", arg0, arg1) + ret0, _ := ret[0].(map[string]string) + ret1, _ := ret[1].(*model.Response) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetPingWithOptions indicates an expected call of GetPingWithOptions. +func (mr *MockClientMockRecorder) GetPingWithOptions(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPingWithOptions", reflect.TypeOf((*MockClient)(nil).GetPingWithOptions), arg0, arg1) +} + // GetPlugins mocks base method. func (m *MockClient) GetPlugins(arg0 context.Context) (*model.PluginsResponse, *model.Response, error) { m.ctrl.T.Helper() diff --git a/server/public/model/client4.go b/server/public/model/client4.go index f2999c19a8..38f216ec99 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -4607,38 +4607,46 @@ func (c *Client4) GenerateSupportPacket(ctx context.Context) ([]byte, *Response, } // GetPing will return ok if the running goRoutines are below the threshold and unhealthy for above. +// DEPRECATED: Use GetPingWithOptions method instead. func (c *Client4) GetPing(ctx context.Context) (string, *Response, error) { - r, err := c.DoAPIGet(ctx, c.systemRoute()+"/ping", "") - if r != nil && r.StatusCode == 500 { - defer r.Body.Close() - return StatusUnhealthy, BuildResponse(r), err + ping, resp, err := c.GetPingWithOptions(ctx, SystemPingOptions{}) + status := "" + if ping != nil { + status = ping["status"] } - if err != nil { - return "", BuildResponse(r), err - } - defer closeBody(r) - return MapFromJSON(r.Body)["status"], BuildResponse(r), nil + return status, resp, err } // GetPingWithServerStatus will return ok if several basic server health checks // all pass successfully. +// DEPRECATED: Use GetPingWithOptions method instead. func (c *Client4) GetPingWithServerStatus(ctx context.Context) (string, *Response, error) { - r, err := c.DoAPIGet(ctx, c.systemRoute()+"/ping?get_server_status="+c.boolString(true), "") - if r != nil && r.StatusCode == 500 { - defer r.Body.Close() - return StatusUnhealthy, BuildResponse(r), err + ping, resp, err := c.GetPingWithOptions(ctx, SystemPingOptions{FullStatus: true}) + status := "" + if ping != nil { + status = ping["status"] } - if err != nil { - return "", BuildResponse(r), err - } - defer closeBody(r) - return MapFromJSON(r.Body)["status"], BuildResponse(r), nil + return status, resp, err } // GetPingWithFullServerStatus will return the full status if several basic server // health checks all pass successfully. +// DEPRECATED: Use GetPingWithOptions method instead. func (c *Client4) GetPingWithFullServerStatus(ctx context.Context) (map[string]string, *Response, error) { - r, err := c.DoAPIGet(ctx, c.systemRoute()+"/ping?get_server_status="+c.boolString(true), "") + return c.GetPingWithOptions(ctx, SystemPingOptions{FullStatus: true}) +} + +// GetPingWithOptions will return the status according to the options +func (c *Client4) GetPingWithOptions(ctx context.Context, options SystemPingOptions) (map[string]string, *Response, error) { + pingURL, err := url.Parse(c.systemRoute() + "/ping") + if err != nil { + return nil, nil, fmt.Errorf("could not parse query: %w", err) + } + values := pingURL.Query() + values.Set("get_server_status", c.boolString(options.FullStatus)) + values.Set("use_rest_semantics", c.boolString(options.RESTSemantics)) + pingURL.RawQuery = values.Encode() + r, err := c.DoAPIGet(ctx, pingURL.String(), "") if r != nil && r.StatusCode == 500 { defer r.Body.Close() return map[string]string{"status": StatusUnhealthy}, BuildResponse(r), err diff --git a/server/public/model/system.go b/server/public/model/system.go index f830f05388..00748463a5 100644 --- a/server/public/model/system.go +++ b/server/public/model/system.go @@ -245,3 +245,14 @@ type LogEntry struct { Timestamp string Level string } + +// SystemPingOptions is the options for setting contents of the system ping +// response. +type SystemPingOptions struct { + // FullStatus allows server to set the detailed information about + // the system status. + FullStatus bool + // RestSemantics allows server to return 200 code even if the server + // status is unhealthy. + RESTSemantics bool +} diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 4976695bf3..d8b1cc6b6d 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -2456,8 +2456,8 @@ export default class Client4 { database_status: string; filestore_status: string; }>( - `${this.getBaseRoute()}/system/ping${buildQueryString({get_server_status: getServerStatus, device_id: deviceId})}`, - {method: 'get', ignoreStatus: true}, + `${this.getBaseRoute()}/system/ping${buildQueryString({get_server_status: getServerStatus, device_id: deviceId, use_rest_semantics: true})}`, + {method: 'get'}, ); };