Reverse Proxy Hotfix (#243)

- Improved the start/stop of the reverse proxy to avoid some potential hanging inside sub routines - added timeouts to all orchestrator calls to the reverse proxy endpoints - improved some of the error messages that shows up
Parallels · Nov 20, 2024 · d58c626 · d58c626
1 parent 1a9730b
commit d58c626
Show file tree

Hide file tree

Showing 13 changed files with 82 additions and 11 deletions.
diff --git a/src/controllers/common.go b/src/controllers/common.go
@@ -22,14 +22,14 @@ func GetBaseContext(r *http.Request) *basecontext.BaseContext {
 func Recover(ctx basecontext.ApiContext, r *http.Request, w http.ResponseWriter) {
 	if err := recover(); err != nil {
 		ctx.LogErrorf("Recovered from panic: %v", err)
-		ReturnApiError(ctx, w, models.NewFromErrorWithCode(fmt.Errorf("Internal Server Error"), http.StatusInternalServerError))
+		ReturnApiError(ctx, w, models.NewFromErrorWithCode(fmt.Errorf("internal server error"), http.StatusInternalServerError))
 
 		fmt.Printf("Recovered from panic: %v", err)
 	}
 }
 
 func ReturnApiError(ctx basecontext.ApiContext, w http.ResponseWriter, err models.ApiErrorResponse) {
-	ctx.LogErrorf("Error: %s", err.Message)
+	ctx.LogErrorf("Error: %v", err.Message)
 	w.WriteHeader(err.Code)
 
 	_ = json.NewEncoder(w).Encode(err)

diff --git a/src/errors/main.go b/src/errors/main.go
@@ -28,6 +28,14 @@ func (e SystemError) Code() int {
 	return e.code
 }
 
+func (e SystemError) Description() string {
+	return e.description
+}
+
+func (e SystemError) Message() string {
+	return e.message
+}
+
 func New(message string) *SystemError {
 	err := &SystemError{
 		message: message,

diff --git a/src/models/api_error_response.go b/src/models/api_error_response.go
@@ -2,11 +2,17 @@ package models
 
 import "github.com/Parallels/prl-devops-service/errors"
 
-type ApiErrorResponse struct {
+type ApiNestedError struct {
 	Message string `json:"message"`
 	Code    int    `json:"code"`
 }
 
+type ApiErrorResponse struct {
+	Message     string           `json:"message"`
+	NestedError []ApiNestedError `json:"nested_error,omitempty"`
+	Code        int              `json:"code"`
+}
+
 func IsSystemError(err error) bool {
 	_, ok := err.(*errors.SystemError)
 	if !ok {
@@ -42,7 +48,11 @@ func GetSystemErrorCode(err error) int {
 func NewFromError(err error) ApiErrorResponse {
 	if IsSystemError(err) {
 		code := GetSystemErrorCode(err)
-		return NewFromErrorWithCode(err, code)
+		result := ApiErrorResponse{
+			Message: err.Error(),
+			Code:    code,
+		}
+		return result
 	} else {
 		return NewFromErrorWithCode(err, 404)
 	}

diff --git a/src/orchestrator/create_host_reverse_proxy_host.go b/src/orchestrator/create_host_reverse_proxy_host.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	data_models "github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/helpers"
@@ -68,6 +70,7 @@ func (s *OrchestratorService) CreateHostReverseProxyHost(ctx basecontext.ApiCont
 
 func (s *OrchestratorService) CallCreateHostReverseProxyHost(host *data_models.OrchestratorHost, r models.ReverseProxyHostCreateRequest) (*models.ReverseProxyHost, error) {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
 	path := "/reverse-proxy/hosts"
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/delete_host_reverse_proxy_host.go b/src/orchestrator/delete_host_reverse_proxy_host.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	data_models "github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/errors"
@@ -42,6 +44,8 @@ func (s *OrchestratorService) DeleteHostReverseProxyHost(ctx basecontext.ApiCont
 
 func (s *OrchestratorService) CallDeleteHostReverseProxyHost(host *data_models.OrchestratorHost, rpHostId string) error {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/hosts/" + rpHostId
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/delete_host_reverse_proxy_host_http_route.go b/src/orchestrator/delete_host_reverse_proxy_host_http_route.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	data_models "github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/errors"
@@ -42,6 +44,8 @@ func (s *OrchestratorService) DeleteHostReverseProxyHostHttpRoute(ctx basecontex
 
 func (s *OrchestratorService) CallHostReverseProxyHostHttpRoute(host *data_models.OrchestratorHost, rpHostId string, httpRouteId string) error {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/hosts/" + rpHostId + "/http_routes/" + httpRouteId
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/disable_host_reverse_proxy.go b/src/orchestrator/disable_host_reverse_proxy.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	"github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/errors"
@@ -35,6 +37,8 @@ func (s *OrchestratorService) DisableHostReverseProxy(ctx basecontext.ApiContext
 
 func (s *OrchestratorService) CallDisableHostReverseProxy(host *models.OrchestratorHost) error {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/disable"
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/enable_host_reverse_proxy.go b/src/orchestrator/enable_host_reverse_proxy.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	"github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/errors"
@@ -35,6 +37,8 @@ func (s *OrchestratorService) EnableHostReverseProxy(ctx basecontext.ApiContext,
 
 func (s *OrchestratorService) CallEnableHostReverseProxy(host *models.OrchestratorHost) error {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/enable"
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/restart_host_reverse_proxy.go b/src/orchestrator/restart_host_reverse_proxy.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	"github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/errors"
@@ -35,6 +37,8 @@ func (s *OrchestratorService) RestartHostReverseProxy(ctx basecontext.ApiContext
 
 func (s *OrchestratorService) CallRestartHostReverseProxy(host *models.OrchestratorHost) error {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/restart"
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/update_host_reverse_proxy_host.go b/src/orchestrator/update_host_reverse_proxy_host.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	data_models "github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/helpers"
@@ -68,6 +70,8 @@ func (s *OrchestratorService) UpdateHostReverseProxyHost(ctx basecontext.ApiCont
 
 func (s *OrchestratorService) CallUpdateHostReverseProxyHost(host *data_models.OrchestratorHost, rpHostId string, r models.ReverseProxyHostUpdateRequest) (*models.ReverseProxyHost, error) {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/hosts/" + rpHostId
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/update_host_reverse_proxy_host_tcp_route.go b/src/orchestrator/update_host_reverse_proxy_host_tcp_route.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	data_models "github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/helpers"
@@ -68,6 +70,8 @@ func (s *OrchestratorService) UpdateHostReverseProxyHostTcpRoute(ctx basecontext
 
 func (s *OrchestratorService) CallUpdateHostReverseProxyHostTcpRoute(host *data_models.OrchestratorHost, rpHostId string, r models.ReverseProxyHostTcpRouteCreateRequest) (*models.ReverseProxyHost, error) {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/hosts/" + rpHostId + "/tcp_route"
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/orchestrator/upsert_host_reverse_proxy_host_http_route.go b/src/orchestrator/upsert_host_reverse_proxy_host_http_route.go
@@ -1,6 +1,8 @@
 package orchestrator
 
 import (
+	"time"
+
 	"github.com/Parallels/prl-devops-service/basecontext"
 	data_models "github.com/Parallels/prl-devops-service/data/models"
 	"github.com/Parallels/prl-devops-service/helpers"
@@ -67,6 +69,8 @@ func (s *OrchestratorService) UpsertHostReverseProxyHostHttpRoute(ctx basecontex
 
 func (s *OrchestratorService) CallUpsertHostReverseProxyHostHttpRoute(host *data_models.OrchestratorHost, rpHostId string, r models.ReverseProxyHostHttpRouteCreateRequest) (*models.ReverseProxyHost, error) {
 	httpClient := s.getApiClient(*host)
+	httpClient.WithTimeout(1 * time.Minute)
+
 	path := "/reverse-proxy/hosts/" + rpHostId + "/http_routes"
 	url, err := helpers.JoinUrl([]string{host.GetHost(), path})
 	if err != nil {

diff --git a/src/reverse_proxy/main.go b/src/reverse_proxy/main.go
@@ -202,7 +202,23 @@ func (rps *ReverseProxyService) LoadFromDb() error {
 				if route.TargetVmId != "" {
 					vm, err := prl_svc.GetVm(rps.api_ctx, route.TargetVmId)
 					if err != nil || vm.InternalIpAddress == "" || vm.InternalIpAddress == "-" || vm.State != "running" {
-						rps.api_ctx.LogErrorf("Error getting vm %s for reverse proxy route: %s", route.TargetVmId, err)
+						e := ""
+						if err != nil {
+							e = err.Error()
+						}
+						if vm == nil {
+							e = "vm could not be found"
+						}
+						if vm != nil && vm.InternalIpAddress == "" {
+							e = "vm internal ip address is empty"
+						}
+						if vm != nil && vm.InternalIpAddress == "-" {
+							e = "vm internal ip address is not assigned"
+						}
+						if vm != nil && vm.State != "running" {
+							e = "vm is not running"
+						}
+						rps.api_ctx.LogErrorf("Error getting vm %s for reverse proxy route: %s", route.TargetVmId, e)
 						hostCopy.HttpRoutes[i].TargetHost = "---"
 					} else {
 						hostCopy.HttpRoutes[i].TargetHost = vm.InternalIpAddress
@@ -267,9 +283,9 @@ func (rps *ReverseProxyService) Start() error {
 			rps.api_ctx.LogErrorf("[Reverse Proxy] Error starting reverse proxy: %s", err)
 		}
 		return err
-		// case <-rps.ctx.Done():
-		// 	rps.api_ctx.LogInfof("[Reverse Proxy] Stopping reverse proxy due to context cancellation")
-		// 	return nil
+	case <-rps.ctx.Done():
+		rps.api_ctx.LogInfof("[Reverse Proxy] Stopping reverse proxy due to context cancellation")
+		return nil
 	default:
 	}
 
@@ -291,10 +307,10 @@ func (rps *ReverseProxyService) Stop() error {
 		rps.api_ctx.LogInfof("[Reverse Proxy] [TCP Route] Listener closed")
 	}
 
-	for _, server := range rps.httpListeners {
-		ctxShutdown, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-		defer cancel()
+	ctxShutdown, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
 
+	for _, server := range rps.httpListeners {
 		rps.api_ctx.LogInfof("[Reverse Proxy] [HTTP Route] Closing server")
 		if err := server.Shutdown(ctxShutdown); err != nil {
 			rps.api_ctx.LogErrorf("[Reverse Proxy] [HTTP Route] Error shutting down server: %s", err)
@@ -303,6 +319,8 @@ func (rps *ReverseProxyService) Stop() error {
 		rps.api_ctx.LogInfof("[Reverse Proxy] [HTTP Route] Server closed")
 	}
 
+	cancel()
+
 	rps.wg.Wait()
 
 	rps.api_ctx.LogInfof("[Reverse Proxy] Reverse proxy service stopped")