From 5e529a1c96069a812108d73a225851def61aa986 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 4 Jun 2020 14:58:34 -0700 Subject: [PATCH] simplify context timeout for readiness (#9772) additionally also add CORS support to restrict for specific origin, adds a new config and updated the documentation as well --- Makefile | 8 ++-- cmd/config-current.go | 8 +--- cmd/config/api/api.go | 26 ++++++++++--- cmd/config/api/help.go | 6 +++ cmd/config/constants.go | 5 --- cmd/generic-handlers.go | 10 ++++- cmd/globals.go | 8 ++-- cmd/{throttling.go => handler-api.go} | 53 ++++++++++++++++++++------- cmd/healthcheck-handler.go | 11 +++++- cmd/notification.go | 4 +- cmd/peer-rest-client.go | 14 +------ cmd/xl-v1.go | 2 +- cmd/xl-zones.go | 2 +- docs/config/README.md | 14 ++++--- docs/metrics/healthcheck/README.md | 14 ++++++- go.mod | 2 +- go.sum | 4 +- 17 files changed, 121 insertions(+), 70 deletions(-) rename cmd/{throttling.go => handler-api.go} (59%) diff --git a/Makefile b/Makefile index d0d7608c7..4f6f3f6c3 100644 --- a/Makefile +++ b/Makefile @@ -8,8 +8,6 @@ GOOS := $(shell go env GOOS) VERSION ?= $(shell git describe --tags) TAG ?= "minio/minio:$(VERSION)" -BUILD_LDFLAGS := '$(LDFLAGS)' - all: build checks: @@ -48,19 +46,19 @@ test-race: verifiers build # Verify minio binary verify: @echo "Verifying build with race" - @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags $(BUILD_LDFLAGS) -o $(PWD)/minio 1>/dev/null + @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null @(env bash $(PWD)/buildscripts/verify-build.sh) # Verify healing of disks with minio binary verify-healing: @echo "Verify healing build with race" - @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags $(BUILD_LDFLAGS) -o $(PWD)/minio 1>/dev/null + @GO111MODULE=on CGO_ENABLED=1 go build -race -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null @(env bash $(PWD)/buildscripts/verify-healing.sh) # Builds minio locally. build: checks @echo "Building minio binary to './minio'" - @GO111MODULE=on CGO_ENABLED=0 go build -tags kqueue -trimpath --ldflags $(BUILD_LDFLAGS) -o $(PWD)/minio 1>/dev/null + @GO111MODULE=on CGO_ENABLED=0 go build -tags kqueue -trimpath --ldflags "$(LDFLAGS)" -o $(PWD)/minio 1>/dev/null docker: build @docker build -t $(TAG) . -f Dockerfile.dev diff --git a/cmd/config-current.go b/cmd/config-current.go index 1046f5057..8ce9ceaef 100644 --- a/cmd/config-current.go +++ b/cmd/config-current.go @@ -365,13 +365,7 @@ func lookupConfigs(s config.Config) { logger.LogIf(ctx, fmt.Errorf("Invalid api configuration: %w", err)) } - apiRequestsMax := apiConfig.APIRequestsMax - if len(globalEndpoints.Hosts()) > 0 { - apiRequestsMax /= len(globalEndpoints.Hosts()) - } - - globalAPIThrottling.init(apiRequestsMax, apiConfig.APIRequestsDeadline) - globalReadyDeadline = apiConfig.APIReadyDeadline + globalAPIConfig.init(apiConfig) if globalIsXL { globalStorageClass, err = storageclass.LookupConfig(s[config.StorageClassSubSys][config.Default], diff --git a/cmd/config/api/api.go b/cmd/config/api/api.go index 2880db17a..3ae52dbe5 100644 --- a/cmd/config/api/api.go +++ b/cmd/config/api/api.go @@ -20,16 +20,24 @@ import ( "encoding/json" "errors" "strconv" + "strings" "time" "github.com/minio/minio/cmd/config" "github.com/minio/minio/pkg/env" ) +// API sub-system constants const ( apiRequestsMax = "requests_max" apiRequestsDeadline = "requests_deadline" apiReadyDeadline = "ready_deadline" + apiCorsAllowOrigin = "cors_allow_origin" + + EnvAPIRequestsMax = "MINIO_API_REQUESTS_MAX" + EnvAPIRequestsDeadline = "MINIO_API_REQUESTS_DEADLINE" + EnvAPIReadyDeadline = "MINIO_API_READY_DEADLINE" + EnvAPICorsAllowOrigin = "MINIO_API_CORS_ALLOW_ORIGIN" ) // DefaultKVS - default storage class config @@ -47,6 +55,10 @@ var ( Key: apiReadyDeadline, Value: "10s", }, + config.KV{ + Key: apiCorsAllowOrigin, + Value: "*", + }, } ) @@ -55,6 +67,7 @@ type Config struct { APIRequestsMax int `json:"requests_max"` APIRequestsDeadline time.Duration `json:"requests_deadline"` APIReadyDeadline time.Duration `json:"ready_deadline"` + APICorsAllowOrigin []string `json:"cors_allow_origin"` } // UnmarshalJSON - Validate SS and RRS parity when unmarshalling JSON. @@ -75,7 +88,7 @@ func LookupConfig(kvs config.KVS) (cfg Config, err error) { } // Check environment variables parameters - requestsMax, err := strconv.Atoi(env.Get(config.EnvAPIRequestsMax, kvs.Get(apiRequestsMax))) + requestsMax, err := strconv.Atoi(env.Get(EnvAPIRequestsMax, kvs.Get(apiRequestsMax))) if err != nil { return cfg, err } @@ -84,20 +97,21 @@ func LookupConfig(kvs config.KVS) (cfg Config, err error) { return cfg, errors.New("invalid API max requests value") } - requestsDeadline, err := time.ParseDuration(env.Get(config.EnvAPIRequestsDeadline, kvs.Get(apiRequestsDeadline))) + requestsDeadline, err := time.ParseDuration(env.Get(EnvAPIRequestsDeadline, kvs.Get(apiRequestsDeadline))) if err != nil { return cfg, err } - readyDeadline, err := time.ParseDuration(env.Get(config.EnvAPIReadyDeadline, kvs.Get(apiReadyDeadline))) + readyDeadline, err := time.ParseDuration(env.Get(EnvAPIReadyDeadline, kvs.Get(apiReadyDeadline))) if err != nil { return cfg, err } - cfg = Config{ + corsAllowOrigin := strings.Split(env.Get(EnvAPICorsAllowOrigin, kvs.Get(apiCorsAllowOrigin)), ",") + return Config{ APIRequestsMax: requestsMax, APIRequestsDeadline: requestsDeadline, APIReadyDeadline: readyDeadline, - } - return cfg, nil + APICorsAllowOrigin: corsAllowOrigin, + }, nil } diff --git a/cmd/config/api/help.go b/cmd/config/api/help.go index ae0b4484f..4f241e091 100644 --- a/cmd/config/api/help.go +++ b/cmd/config/api/help.go @@ -39,5 +39,11 @@ var ( Optional: true, Type: "duration", }, + config.HelpKV{ + Key: apiCorsAllowOrigin, + Description: `set comma separated list of origins allowed for CORS requests e.g. "https://example1.com,https://example2.com"`, + Optional: true, + Type: "csv", + }, } ) diff --git a/cmd/config/constants.go b/cmd/config/constants.go index 22b985196..3a8bf5868 100644 --- a/cmd/config/constants.go +++ b/cmd/config/constants.go @@ -34,11 +34,6 @@ const ( EnvEndpoints = "MINIO_ENDPOINTS" EnvFSOSync = "MINIO_FS_OSYNC" - // API sub-system - EnvAPIRequestsMax = "MINIO_API_REQUESTS_MAX" - EnvAPIRequestsDeadline = "MINIO_API_REQUESTS_DEADLINE" - EnvAPIReadyDeadline = "MINIO_API_READY_DEADLINE" - EnvUpdate = "MINIO_UPDATE" EnvWorm = "MINIO_WORM" // legacy diff --git a/cmd/generic-handlers.go b/cmd/generic-handlers.go index c642db745..b42e23d05 100644 --- a/cmd/generic-handlers.go +++ b/cmd/generic-handlers.go @@ -30,6 +30,7 @@ import ( "github.com/minio/minio/cmd/http/stats" "github.com/minio/minio/cmd/logger" "github.com/minio/minio/pkg/handlers" + "github.com/minio/minio/pkg/wildcard" "github.com/rs/cors" ) @@ -410,7 +411,14 @@ func setCorsHandler(h http.Handler) http.Handler { } c := cors.New(cors.Options{ - AllowedOrigins: []string{"*"}, + AllowOriginFunc: func(origin string) bool { + for _, allowedOrigin := range globalAPIConfig.getCorsAllowOrigins() { + if wildcard.MatchSimple(allowedOrigin, origin) { + return true + } + } + return false + }, AllowedMethods: []string{ http.MethodGet, http.MethodPut, diff --git a/cmd/globals.go b/cmd/globals.go index 9ebf212f4..fae732fc9 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -154,9 +154,9 @@ var ( globalLifecycleSys *LifecycleSys globalBucketSSEConfigSys *BucketSSEConfigSys - // globalAPIThrottling controls S3 requests throttling when - // enabled in the config or in the shell environment. - globalAPIThrottling apiThrottling + // globalAPIConfig controls S3 API requests throttling, + // healthcheck readiness deadlines and cors settings. + globalAPIConfig apiConfig globalStorageClass storageclass.Config globalLDAPConfig xldap.Config @@ -275,8 +275,6 @@ var ( // If writes to FS backend should be O_SYNC. globalFSOSync bool - // Deadline by which /minio/health/ready should respond. - globalReadyDeadline time.Duration // Add new variable global values here. ) diff --git a/cmd/throttling.go b/cmd/handler-api.go similarity index 59% rename from cmd/throttling.go rename to cmd/handler-api.go index 7bd1259df..f946a4391 100644 --- a/cmd/throttling.go +++ b/cmd/handler-api.go @@ -20,34 +20,61 @@ import ( "net/http" "sync" "time" + + "github.com/minio/minio/cmd/config/api" ) -type apiThrottling struct { - mu sync.RWMutex - enabled bool +type apiConfig struct { + mu sync.RWMutex requestsDeadline time.Duration requestsPool chan struct{} + readyDeadline time.Duration + corsAllowOrigins []string } -func (t *apiThrottling) init(max int, deadline time.Duration) { - if max <= 0 { +func (t *apiConfig) init(cfg api.Config) { + t.mu.Lock() + defer t.mu.Unlock() + + t.readyDeadline = cfg.APIReadyDeadline + t.corsAllowOrigins = cfg.APICorsAllowOrigin + if cfg.APIRequestsMax <= 0 { return } - t.mu.Lock() - defer t.mu.Unlock() + apiRequestsMax := cfg.APIRequestsMax + if len(globalEndpoints.Hosts()) > 0 { + apiRequestsMax /= len(globalEndpoints.Hosts()) + } + + t.requestsPool = make(chan struct{}, apiRequestsMax) + t.requestsDeadline = cfg.APIRequestsDeadline +} + +func (t *apiConfig) getCorsAllowOrigins() []string { + t.mu.RLock() + defer t.mu.RUnlock() + + return t.corsAllowOrigins +} + +func (t *apiConfig) getReadyDeadline() time.Duration { + t.mu.RLock() + defer t.mu.RUnlock() + + if t.readyDeadline == 0 { + return 10 * time.Second + } - t.requestsPool = make(chan struct{}, max) - t.requestsDeadline = deadline - t.enabled = true + return t.readyDeadline } -func (t *apiThrottling) get() (chan struct{}, <-chan time.Time) { +func (t *apiConfig) getRequestsPool() (chan struct{}, <-chan time.Time) { t.mu.RLock() defer t.mu.RUnlock() - if !t.enabled { + if t.requestsPool == nil { return nil, nil } @@ -57,7 +84,7 @@ func (t *apiThrottling) get() (chan struct{}, <-chan time.Time) { // maxClients throttles the S3 API calls func maxClients(f http.HandlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - pool, deadlineTimer := globalAPIThrottling.get() + pool, deadlineTimer := globalAPIConfig.getRequestsPool() if pool == nil { f.ServeHTTP(w, r) return diff --git a/cmd/healthcheck-handler.go b/cmd/healthcheck-handler.go index ea3283881..098ef802a 100644 --- a/cmd/healthcheck-handler.go +++ b/cmd/healthcheck-handler.go @@ -17,6 +17,7 @@ package cmd import ( + "context" "net/http" ) @@ -28,7 +29,15 @@ func ReadinessCheckHandler(w http.ResponseWriter, r *http.Request) { objLayer := newObjectLayerWithoutSafeModeFn() // Service not initialized yet - if objLayer == nil || !objLayer.IsReady(ctx) { + if objLayer == nil { + writeResponse(w, http.StatusServiceUnavailable, nil, mimeNone) + return + } + + ctx, cancel := context.WithTimeout(ctx, globalAPIConfig.getReadyDeadline()) + defer cancel() + + if !objLayer.IsReady(ctx) { writeResponse(w, http.StatusServiceUnavailable, nil, mimeNone) return } diff --git a/cmd/notification.go b/cmd/notification.go index 66fae45b5..37fa8e323 100644 --- a/cmd/notification.go +++ b/cmd/notification.go @@ -1164,7 +1164,7 @@ func (sys *NotificationSys) ServerInfo() []madmin.ServerProperties { } // GetLocalDiskIDs - return disk ids of the local disks of the peers. -func (sys *NotificationSys) GetLocalDiskIDs() []string { +func (sys *NotificationSys) GetLocalDiskIDs(ctx context.Context) []string { var diskIDs []string var mu sync.Mutex @@ -1176,7 +1176,7 @@ func (sys *NotificationSys) GetLocalDiskIDs() []string { wg.Add(1) go func(client *peerRESTClient) { defer wg.Done() - ids := client.GetLocalDiskIDs() + ids := client.GetLocalDiskIDs(ctx) mu.Lock() diskIDs = append(diskIDs, ids...) mu.Unlock() diff --git a/cmd/peer-rest-client.go b/cmd/peer-rest-client.go index f6556f50d..cd9074356 100644 --- a/cmd/peer-rest-client.go +++ b/cmd/peer-rest-client.go @@ -677,19 +677,7 @@ func (client *peerRESTClient) BackgroundHealStatus() (madmin.BgHealState, error) } // GetLocalDiskIDs - get a peer's local disks' IDs. -func (client *peerRESTClient) GetLocalDiskIDs() []string { - doneCh := make(chan struct{}) - defer close(doneCh) - - ctx, cancel := context.WithCancel(GlobalContext) - go func() { - select { - case <-doneCh: - return - case <-time.After(globalReadyDeadline): - cancel() - } - }() +func (client *peerRESTClient) GetLocalDiskIDs(ctx context.Context) []string { respBody, err := client.callWithContext(ctx, peerRESTMethodGetLocalDiskIDs, nil, nil, -1) if err != nil { return nil diff --git a/cmd/xl-v1.go b/cmd/xl-v1.go index 5e6400254..886b7e1c3 100644 --- a/cmd/xl-v1.go +++ b/cmd/xl-v1.go @@ -403,7 +403,7 @@ func (xl xlObjects) crawlAndGetDataUsage(ctx context.Context, buckets []BucketIn return nil } -// IsReady - No Op. +// IsReady - shouldn't be called will panic. func (xl xlObjects) IsReady(ctx context.Context) bool { logger.CriticalIf(ctx, NotImplemented{}) return true diff --git a/cmd/xl-zones.go b/cmd/xl-zones.go index 4cb717b0b..0575fd6e8 100644 --- a/cmd/xl-zones.go +++ b/cmd/xl-zones.go @@ -1615,7 +1615,7 @@ func (z *xlZones) IsReady(ctx context.Context) bool { erasureSetUpCount[i] = make([]int, len(z.zones[i].sets)) } - diskIDs := globalNotificationSys.GetLocalDiskIDs() + diskIDs := globalNotificationSys.GetLocalDiskIDs(ctx) diskIDs = append(diskIDs, getLocalDiskIDs(z)...) diff --git a/docs/config/README.md b/docs/config/README.md index 6152a5b89..f1522fc9e 100644 --- a/docs/config/README.md +++ b/docs/config/README.md @@ -172,7 +172,6 @@ MINIO_ETCD_COMMENT (sentence) optionally add a comment to this setting ``` ### API - By default, there is no limitation on the number of concurrents requests that a server/cluster processes at the same time. However, it is possible to impose such limitation using the API subsystem. Read more about throttling limitation in MinIO server [here](https://github.com/minio/minio/blob/master/docs/throttle/README.md). ``` @@ -180,16 +179,19 @@ KEY: api manage global HTTP API call specific features, such as throttling, authentication types, etc. ARGS: -requests_max (number) set the maximum number of concurrent requests -requests_deadline (duration) set the deadline for API requests waiting to be processed +requests_max (number) set the maximum number of concurrent requests, e.g. "1600" +requests_deadline (duration) set the deadline for API requests waiting to be processed e.g. "1m" +ready_deadline (duration) set the deadline for health check API /minio/health/ready e.g. "1m" +cors_allow_origin (csv) set comma separated list of origins allowed for CORS requests e.g. "https://example1.com,https://example2.com" ``` or environment variables ``` -MINIO_API_REQUESTS_MAX (number) set the maximum number of concurrent requests -MINIO_API_REQUESTS_DEADLINE (duration) set the deadline for API requests waiting to be processed - +MINIO_API_REQUESTS_MAX (number) set the maximum number of concurrent requests, e.g. "1600" +MINIO_API_REQUESTS_DEADLINE (duration) set the deadline for API requests waiting to be processed e.g. "1m" +MINIO_API_READY_DEADLINE (duration) set the deadline for health check API /minio/health/ready e.g. "1m" +MINIO_API_CORS_ALLOW_ORIGIN (csv) set comma separated list of origins allowed for CORS requests e.g. "https://example1.com,https://example2.com" ``` #### Notifications diff --git a/docs/metrics/healthcheck/README.md b/docs/metrics/healthcheck/README.md index 65f06bf59..b6de19e6a 100644 --- a/docs/metrics/healthcheck/README.md +++ b/docs/metrics/healthcheck/README.md @@ -16,8 +16,20 @@ This probe is used to identify situations where the server is not ready to accep Internally, MinIO readiness probe handler checks for backend is alive and in read quorum then the server returns 200 OK, otherwise 503 Service Unavailable. -Platforms like Kubernetes *do not* forward traffic to a pod until its readiness probe is successful. +Platforms like Kubernetes *do not* forward traffic to a pod until its readiness probe is successful. ### Configuration example Sample `liveness` and `readiness` probe configuration in a Kubernetes `yaml` file can be found [here](https://github.com/minio/minio/blob/master/docs/orchestration/kubernetes/minio-standalone-deployment.yaml). + +### Configure readiness deadline +Readiness checks need to respond faster in orchestrated environments, to facilitate this you can use the following environment variable before starting MinIO + +``` +MINIO_API_READY_DEADLINE (duration) set the deadline for health check API /minio/health/ready e.g. "1m" +``` + +Set a *5s* deadline for MinIO to ensure readiness handler responds with-in 5seconds. +``` +export MINIO_API_READY_DEADLINE=5s +``` diff --git a/go.mod b/go.mod index c103cbaf3..206037fed 100644 --- a/go.mod +++ b/go.mod @@ -94,7 +94,7 @@ require ( github.com/prometheus/client_golang v0.9.3 github.com/rcrowley/go-metrics v0.0.0-20190704165056-9c2d0518ed81 // indirect github.com/rjeczalik/notify v0.9.2 - github.com/rs/cors v1.6.0 + github.com/rs/cors v1.7.0 github.com/secure-io/sio-go v0.3.0 github.com/shirou/gopsutil v2.20.3-0.20200314133625-53cec6b37e6a+incompatible github.com/sirupsen/logrus v1.5.0 diff --git a/go.sum b/go.sum index eba0b4ea4..95089c5ab 100644 --- a/go.sum +++ b/go.sum @@ -377,8 +377,8 @@ github.com/rjeczalik/notify v0.9.2 h1:MiTWrPj55mNDHEiIX5YUSKefw/+lCQVoAFmD6oQm5w github.com/rjeczalik/notify v0.9.2/go.mod h1:aErll2f0sUX9PXZnVNyeiObbmTlk5jnMoCa4QEjJeqM= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= -github.com/rs/cors v1.6.0 h1:G9tHG9lebljV9mfp9SNPDL36nCDxmo3zTlAf1YgvzmI= -github.com/rs/cors v1.6.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU= +github.com/rs/cors v1.7.0 h1:+88SsELBHx5r+hZ8TCkggzSstaWNbDvThkVK8H6f9ik= +github.com/rs/cors v1.7.0/go.mod h1:gFx+x8UowdsKA9AchylcLynDq+nNFfI8FkUZdN/jGCU= github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc=