[MM-56348] system/ping: add new method with options (#26079)

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

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)))

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)

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)
 	}

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)
 

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()

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

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
+}

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'},
         );
     };