Refactor health check API

Refactor the health check API

Signed-off-by: Wenkai Yin <yinw@vmware.com>

api/v2.0/legacy_swagger.yaml

@@ -19,18 +19,6 @@ securityDefinitions:
 security:
   - basicAuth: []
 paths:
-  /health:
-    get:
-      summary: 'Health check API'
-      description: |
-        The endpoint returns the health stauts of the system.
-      tags:
-        - Products
-      responses:
-        '200':
-          description: The system health status.
-          schema:
-            $ref: '#/definitions/OverallHealthStatus'
   '/projects/{project_id}/metadatas':
     get:
       summary: Get project metadata.
@@ -1210,30 +1198,6 @@ definitions:
     description: A list of label
     items:
       $ref: '#/definitions/Label'
-  OverallHealthStatus:
-    type: object
-    description: The system health status
-    properties:
-      status:
-        type: string
-        description: The overall health status. It is "healthy" only when all the components' status are "healthy"
-      components:
-        type: array
-        items:
-          $ref: '#/definitions/ComponentHealthStatus'
-  ComponentHealthStatus:
-    type: object
-    description: The health status of component
-    properties:
-      name:
-        type: string
-        description: The component name
-      status:
-        type: string
-        description: The health status of component
-      error:
-        type: string
-        description: (optional) The error message when the status is "unhealthy"
   Permission:
     type: object
     description: The permission

api/v2.0/swagger.yaml

@@ -19,6 +19,20 @@ security:
   - basic: []
   - {}
 paths:
+  /health:
+    get:
+      summary: Check the status of Harbor components
+      description: Check the status of Harbor components
+      tags:
+        - health
+      operationId: getHealth
+      responses:
+        '200':
+          description: The health status of Harbor components
+          schema:
+            $ref: '#/definitions/OverallHealthStatus'
+        '500':
+          $ref: '#/responses/500'
   /search:
     get:
       summary: 'Search for projects, repositories and helm charts'
@@ -7732,5 +7746,27 @@ definitions:
       secret:
         type: string
         description: The new secret
-
+  OverallHealthStatus:
-
+    type: object
+    description: The system health status
+    properties:
+      status:
+        type: string
+        description: The overall health status. It is "healthy" only when all the components' status are "healthy"
+      components:
+        type: array
+        items:
+          $ref: '#/definitions/ComponentHealthStatus'
+  ComponentHealthStatus:
+    type: object
+    description: The health status of component
+    properties:
+      name:
+        type: string
+        description: The component name
+      status:
+        type: string
+        description: The health status of component
+      error:
+        type: string
+        description: (optional) The error message when the status is "unhealthy"

src/controller/health/checker.go

@@ -12,114 +12,26 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package api
+package health
 
 import (
-	"errors"
 	"fmt"
-	"github.com/goharbor/harbor/src/lib/config"
 	"io/ioutil"
 	"net/http"
-	"sort"
 	"strings"
 	"sync"
 	"time"
 
+	"github.com/astaxie/beego/orm"
 	"github.com/docker/distribution/health"
-	"github.com/goharbor/harbor/src/common/dao"
 	httputil "github.com/goharbor/harbor/src/common/http"
 	"github.com/goharbor/harbor/src/common/utils"
+	"github.com/goharbor/harbor/src/lib/config"
+	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/redis"
 )
 
-var (
-	timeout = 60 * time.Second
-	// HealthCheckerRegistry ...
-	HealthCheckerRegistry = map[string]health.Checker{}
-)
-
-type overallHealthStatus struct {
-	Status     string                   `json:"status"`
-	Components []*componentHealthStatus `json:"components"`
-}
-
-type componentHealthStatus struct {
-	Name   string `json:"name"`
-	Status string `json:"status"`
-	Error  string `json:"error,omitempty"`
-}
-
-type healthy bool
-
-func (h healthy) String() string {
-	if h {
-		return "healthy"
-	}
-	return "unhealthy"
-}
-
-// HealthAPI handles the request for "/api/health"
-type HealthAPI struct {
-	BaseController
-}
-
-// CheckHealth checks the health of system
-func (h *HealthAPI) CheckHealth() {
-	var isHealthy healthy = true
-	components := []*componentHealthStatus{}
-	c := make(chan *componentHealthStatus, len(HealthCheckerRegistry))
-	for name, checker := range HealthCheckerRegistry {
-		go check(name, checker, timeout, c)
-	}
-	for i := 0; i < len(HealthCheckerRegistry); i++ {
-		componentStatus := <-c
-		if len(componentStatus.Error) != 0 {
-			isHealthy = false
-		}
-		components = append(components, componentStatus)
-	}
-
-	sort.Slice(components, func(i, j int) bool { return components[i].Name < components[j].Name })
-
-	status := &overallHealthStatus{}
-	status.Status = isHealthy.String()
-	status.Components = components
-	if !isHealthy {
-		log.Debugf("unhealthy system status: %v", status)
-	}
-	h.WriteJSONData(status)
-}
-
-func check(name string, checker health.Checker,
-	timeout time.Duration, c chan *componentHealthStatus) {
-	statusChan := make(chan *componentHealthStatus)
-	go func() {
-		err := checker.Check()
-		var healthy healthy = err == nil
-		status := &componentHealthStatus{
-			Name:   name,
-			Status: healthy.String(),
-		}
-		if !healthy {
-			status.Error = err.Error()
-		}
-		statusChan <- status
-	}()
-
-	select {
-	case status := <-statusChan:
-		c <- status
-	case <-time.After(timeout):
-		var healthy healthy = false
-		c <- &componentHealthStatus{
-			Name:   name,
-			Status: healthy.String(),
-			Error:  "failed to check the health status: timeout",
-		}
-	}
-}
-
 // HTTPStatusCodeHealthChecker implements a Checker to check that the HTTP status code
 // returned matches the expected one
 func HTTPStatusCodeHealthChecker(method string, url string, header http.Header,
@@ -255,7 +167,7 @@ func notaryHealthChecker() health.Checker {
 func databaseHealthChecker() health.Checker {
 	period := 10 * time.Second
 	checker := health.CheckFunc(func() error {
-		_, err := dao.GetOrmer().Raw("SELECT 1").Exec()
+		_, err := orm.NewOrm().Raw("SELECT 1").Exec()
 		if err != nil {
 			return fmt.Errorf("failed to run SQL \"SELECT 1\": %v", err)
 		}
@@ -282,22 +194,23 @@ func trivyHealthChecker() health.Checker {
 	return PeriodicHealthChecker(checker, period)
 }
 
-func registerHealthCheckers() {
+// RegisterHealthCheckers ...
-	HealthCheckerRegistry["core"] = coreHealthChecker()
+func RegisterHealthCheckers() {
-	HealthCheckerRegistry["portal"] = portalHealthChecker()
+	registry["core"] = coreHealthChecker()
-	HealthCheckerRegistry["jobservice"] = jobserviceHealthChecker()
+	registry["portal"] = portalHealthChecker()
-	HealthCheckerRegistry["registry"] = registryHealthChecker()
+	registry["jobservice"] = jobserviceHealthChecker()
-	HealthCheckerRegistry["registryctl"] = registryCtlHealthChecker()
+	registry["registry"] = registryHealthChecker()
-	HealthCheckerRegistry["database"] = databaseHealthChecker()
+	registry["registryctl"] = registryCtlHealthChecker()
-	HealthCheckerRegistry["redis"] = redisHealthChecker()
+	registry["database"] = databaseHealthChecker()
+	registry["redis"] = redisHealthChecker()
 	if config.WithChartMuseum() {
-		HealthCheckerRegistry["chartmuseum"] = chartmuseumHealthChecker()
+		registry["chartmuseum"] = chartmuseumHealthChecker()
 	}
 	if config.WithNotary() {
-		HealthCheckerRegistry["notary"] = notaryHealthChecker()
+		registry["notary"] = notaryHealthChecker()
 	}
 	if config.WithTrivy() {
-		HealthCheckerRegistry["trivy"] = trivyHealthChecker()
+		registry["trivy"] = trivyHealthChecker()
 	}
 }
 

src/controller/health/checker_test.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package api
+package health
 
 import (
 	"errors"
@@ -23,7 +23,6 @@ import (
 	"github.com/docker/distribution/health"
 	"github.com/goharbor/harbor/src/common/utils/test"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 )
 
 func TestStringOfHealthy(t *testing.T) {
@@ -82,53 +81,7 @@ func TestPeriodicHealthChecker(t *testing.T) {
 	assert.Equal(t, "unhealthy", checker.Check().Error())
 }
 
-func fakeHealthChecker(healthy bool) health.Checker {
-	return health.CheckFunc(func() error {
-		if healthy {
-			return nil
-		}
-		return errors.New("unhealthy")
-	})
-}
-func TestCheckHealth(t *testing.T) {
-	// component01: healthy, component02: healthy => status: healthy
-	HealthCheckerRegistry = map[string]health.Checker{}
-	HealthCheckerRegistry["component01"] = fakeHealthChecker(true)
-	HealthCheckerRegistry["component02"] = fakeHealthChecker(true)
-	status := map[string]interface{}{}
-	err := handleAndParse(&testingRequest{
-		method: http.MethodGet,
-		url:    "/api/health",
-	}, &status)
-	require.Nil(t, err)
-	assert.Equal(t, "healthy", status["status"].(string))
-
-	// component01: healthy, component02: unhealthy => status: unhealthy
-	HealthCheckerRegistry = map[string]health.Checker{}
-	HealthCheckerRegistry["component01"] = fakeHealthChecker(true)
-	HealthCheckerRegistry["component02"] = fakeHealthChecker(false)
-	status = map[string]interface{}{}
-	err = handleAndParse(&testingRequest{
-		method: http.MethodGet,
-		url:    "/api/health",
-	}, &status)
-	require.Nil(t, err)
-	assert.Equal(t, "unhealthy", status["status"].(string))
-}
-
 func TestCoreHealthChecker(t *testing.T) {
 	checker := coreHealthChecker()
 	assert.Equal(t, nil, checker.Check())
 }
-
-func TestDatabaseHealthChecker(t *testing.T) {
-	checker := databaseHealthChecker()
-	time.Sleep(1 * time.Second)
-	assert.Equal(t, nil, checker.Check())
-}
-
-func TestRegisterHealthCheckers(t *testing.T) {
-	HealthCheckerRegistry = map[string]health.Checker{}
-	registerHealthCheckers()
-	assert.NotNil(t, HealthCheckerRegistry["core"])
-}

src/controller/health/controller.go

@@ -0,0 +1,94 @@
+// Copyright 2019 Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package health
+
+import (
+	"context"
+	"sort"
+	"time"
+
+	"github.com/docker/distribution/health"
+)
+
+var (
+	timeout  = 60 * time.Second
+	registry = map[string]health.Checker{}
+	// Ctl is a global health controller
+	Ctl = NewController()
+)
+
+// NewController returns a health controller instance
+func NewController() Controller {
+	return &controller{}
+}
+
+// Controller defines the health related operations
+type Controller interface {
+	GetHealth(ctx context.Context) *OverallHealthStatus
+}
+
+type controller struct{}
+
+func (c *controller) GetHealth(ctx context.Context) *OverallHealthStatus {
+	var isHealthy healthy = true
+	components := []*ComponentHealthStatus{}
+	ch := make(chan *ComponentHealthStatus, len(registry))
+	for name, checker := range registry {
+		go check(name, checker, timeout, ch)
+	}
+	for i := 0; i < len(registry); i++ {
+		componentStatus := <-ch
+		if len(componentStatus.Error) != 0 {
+			isHealthy = false
+		}
+		components = append(components, componentStatus)
+	}
+
+	sort.Slice(components, func(i, j int) bool { return components[i].Name < components[j].Name })
+
+	return &OverallHealthStatus{
+		Status:     isHealthy.String(),
+		Components: components,
+	}
+}
+
+func check(name string, checker health.Checker,
+	timeout time.Duration, c chan *ComponentHealthStatus) {
+	statusChan := make(chan *ComponentHealthStatus)
+	go func() {
+		err := checker.Check()
+		var healthy healthy = err == nil
+		status := &ComponentHealthStatus{
+			Name:   name,
+			Status: healthy.String(),
+		}
+		if !healthy {
+			status.Error = err.Error()
+		}
+		statusChan <- status
+	}()
+
+	select {
+	case status := <-statusChan:
+		c <- status
+	case <-time.After(timeout):
+		var healthy healthy = false
+		c <- &ComponentHealthStatus{
+			Name:   name,
+			Status: healthy.String(),
+			Error:  "failed to check the health status: timeout",
+		}
+	}
+}

src/controller/health/controller_test.go

@@ -0,0 +1,50 @@
+// Copyright 2019 Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package health
+
+import (
+	"testing"
+
+	"github.com/docker/distribution/health"
+	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/stretchr/testify/assert"
+)
+
+func fakeHealthChecker(healthy bool) health.Checker {
+	return health.CheckFunc(func() error {
+		if healthy {
+			return nil
+		}
+		return errors.New("unhealthy")
+	})
+}
+
+func TestCheckHealth(t *testing.T) {
+	ctl := controller{}
+
+	// component01: healthy, component02: healthy => status: healthy
+	registry = map[string]health.Checker{}
+	registry["component01"] = fakeHealthChecker(true)
+	registry["component02"] = fakeHealthChecker(true)
+	status := ctl.GetHealth(nil)
+	assert.Equal(t, "healthy", status.Status)
+
+	// component01: healthy, component02: unhealthy => status: unhealthy
+	registry = map[string]health.Checker{}
+	registry["component01"] = fakeHealthChecker(true)
+	registry["component02"] = fakeHealthChecker(false)
+	status = ctl.GetHealth(nil)
+	assert.Equal(t, "unhealthy", status.Status)
+}

src/controller/health/model.go

@@ -0,0 +1,23 @@
+package health
+
+// OverallHealthStatus defines the overall health status of the system
+type OverallHealthStatus struct {
+	Status     string                   `json:"status"`
+	Components []*ComponentHealthStatus `json:"components"`
+}
+
+// ComponentHealthStatus defines the specific component health status
+type ComponentHealthStatus struct {
+	Name   string `json:"name"`
+	Status string `json:"status"`
+	Error  string `json:"error,omitempty"`
+}
+
+type healthy bool
+
+func (h healthy) String() string {
+	if h {
+		return "healthy"
+	}
+	return "unhealthy"
+}

src/core/api/base.go

@@ -161,8 +161,6 @@ func (b *BaseController) PopulateUserSession(u models.User) {
 
 // Init related objects/configurations for the API controllers
 func Init() error {
-	registerHealthCheckers()
-
 	// init chart controller
 	if err := initChartController(); err != nil {
 		return err

src/core/api/harborapi_test.go

@@ -93,7 +93,6 @@ func init() {
 	beego.BConfig.WebConfig.Session.SessionOn = true
 	beego.TestBeegoInit(apppath)
 
-	beego.Router("/api/health", &HealthAPI{}, "get:CheckHealth")
 	beego.Router("/api/projects/:id([0-9]+)/metadatas/?:name", &MetadataAPI{}, "get:Get")
 	beego.Router("/api/projects/:id([0-9]+)/metadatas/", &MetadataAPI{}, "post:Post")
 	beego.Router("/api/projects/:id([0-9]+)/metadatas/:name", &MetadataAPI{}, "put:Put;delete:Delete")

src/core/main.go

@@ -18,7 +18,6 @@ import (
 	"context"
 	"encoding/gob"
 	"fmt"
-	"github.com/goharbor/harbor/src/lib/config"
 	"net/url"
 	"os"
 	"os/signal"
@@ -35,6 +34,7 @@ import (
 	"github.com/goharbor/harbor/src/common/models"
 	"github.com/goharbor/harbor/src/common/utils"
 	_ "github.com/goharbor/harbor/src/controller/event/handler"
+	"github.com/goharbor/harbor/src/controller/health"
 	"github.com/goharbor/harbor/src/controller/registry"
 	"github.com/goharbor/harbor/src/core/api"
 	_ "github.com/goharbor/harbor/src/core/auth/authproxy"
@@ -47,6 +47,7 @@ import (
 	"github.com/goharbor/harbor/src/lib/cache"
 	_ "github.com/goharbor/harbor/src/lib/cache/memory" // memory cache
 	_ "github.com/goharbor/harbor/src/lib/cache/redis"  // redis cache
+	"github.com/goharbor/harbor/src/lib/config"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/lib/metric"
 	"github.com/goharbor/harbor/src/lib/orm"
@@ -206,6 +207,7 @@ func main() {
 		log.Fatalf("Failed to initialize API handlers with error: %s", err.Error())
 	}
 
+	health.RegisterHealthCheckers()
 	registerScanners(orm.Context())
 
 	closing := make(chan struct{})

src/server/v2.0/handler/handler.go

@@ -60,6 +60,7 @@ func New() http.Handler {
 		ConfigureAPI:          newConfigAPI(),
 		UsergroupAPI:          newUserGroupAPI(),
 		UserAPI:               newUsersAPI(),
+		HealthAPI:             newHealthAPI(),
 	})
 	if err != nil {
 		log.Fatal(err)

src/server/v2.0/handler/health.go

@@ -0,0 +1,50 @@
+// Copyright Project Harbor Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package handler
+
+import (
+	"context"
+
+	"github.com/go-openapi/runtime/middleware"
+	"github.com/goharbor/harbor/src/controller/health"
+	"github.com/goharbor/harbor/src/server/v2.0/models"
+	operations "github.com/goharbor/harbor/src/server/v2.0/restapi/operations/health"
+)
+
+func newHealthAPI() *healthAPI {
+	return &healthAPI{
+		ctl: health.Ctl,
+	}
+}
+
+type healthAPI struct {
+	BaseAPI
+	ctl health.Controller
+}
+
+func (r *healthAPI) GetHealth(ctx context.Context, params operations.GetHealthParams) middleware.Responder {
+	status := r.ctl.GetHealth(ctx)
+	s := &models.OverallHealthStatus{
+		Status: status.Status,
+	}
+	for _, c := range status.Components {
+		s.Components = append(s.Components, &models.ComponentHealthStatus{
+			Error:  c.Error,
+			Name:   c.Name,
+			Status: c.Status,
+		})
+	}
+	return operations.NewGetHealthOK().WithPayload(s)
+}

src/server/v2.0/route/legacy.go

@@ -24,7 +24,6 @@ import (
 func registerLegacyRoutes() {
 	version := APIVersion
 	beego.Router("/api/"+version+"/email/ping", &api.EmailAPI{}, "post:Ping")
-	beego.Router("/api/"+version+"/health", &api.HealthAPI{}, "get:CheckHealth")
 	beego.Router("/api/"+version+"/projects/:id([0-9]+)/metadatas/?:name", &api.MetadataAPI{}, "get:Get")
 	beego.Router("/api/"+version+"/projects/:id([0-9]+)/metadatas/", &api.MetadataAPI{}, "post:Post")
 	beego.Router("/api/"+version+"/statistics", &api.StatisticAPI{})

tests/apitests/python/library/base.py

@@ -31,7 +31,7 @@ def _create_client(server, credential, debug, api_type="products"):
     cfg = None
     if api_type in ('projectv2', 'artifact', 'repository', 'scanner', 'scan', 'scanall', 'preheat', 'quota',
                     'replication', 'registry', 'robot', 'gc', 'retention', 'immutable', 'system_cve_allowlist',
-                    'configure', 'user', 'member'):
+                    'configure', 'user', 'member', 'health'):
         cfg = v2_swagger_client.Configuration()
     else:
         cfg = swagger_client.Configuration()
@@ -74,6 +74,7 @@ def _create_client(server, credential, debug, api_type="products"):
         "configure":   v2_swagger_client.ConfigureApi(v2_swagger_client.ApiClient(cfg)),
         "user": v2_swagger_client.UserApi(v2_swagger_client.ApiClient(cfg)),
         "member": v2_swagger_client.MemberApi(v2_swagger_client.ApiClient(cfg)),
+        "health":   v2_swagger_client.HealthApi(v2_swagger_client.ApiClient(cfg)),
     }.get(api_type,'Error: Wrong API type')
 
 def _assert_status_code(expect_code, return_code, err_msg = r"HTTPS status code s not as we expected. Expected {}, while actual HTTPS status code is {}."):

tests/apitests/python/test_health_check.py

@@ -1,16 +1,11 @@
 # coding: utf-8
 
-from __future__ import absolute_import
+from library.base import Base
 
-import unittest
+class Health(Base, object):
-import testutils
+    def __init__(self):
-
+        super(Health,self).__init__(api_type = "health")
-class TestHealthCheck(unittest.TestCase):
     def testHealthCheck(self):
-        client = testutils.GetProductApi("admin", "Harbor12345")
+        status, code, _ = self._get_client(**kwargs).get_health_with_http_info()
-        status, code, _ = client.health_get_with_http_info()
         self.assertEqual(code, 200)
         self.assertEqual("healthy", status.status)
-
-if __name__ == '__main__':
-    unittest.main()