diff --git a/.github/workflows/canary-integration-test.yml b/.github/workflows/canary-integration-test.yml index 53fbee517..fd1d6a0ab 100644 --- a/.github/workflows/canary-integration-test.yml +++ b/.github/workflows/canary-integration-test.yml @@ -945,6 +945,7 @@ jobs: run: | kubectl delete -f tests/manifests/test-object.yaml yq write -i tests/manifests/test-object.yaml "spec.security.kms.connectionDetails.VAULT_SECRET_ENGINE" transit + timeout 120 bash -c 'while kubectl -n rook-ceph get cephobjectstore my-store; do echo "waiting for objectstore my-store to delete"; sleep 5; done' kubectl create -f tests/manifests/test-object.yaml tests/scripts/validate_cluster.sh rgw tests/scripts/deploy-validate-vault.sh validate_rgw diff --git a/.github/workflows/rgw-multisite-test/action.yml b/.github/workflows/rgw-multisite-test/action.yml index 02e4b04da..a8f0865af 100644 --- a/.github/workflows/rgw-multisite-test/action.yml +++ b/.github/workflows/rgw-multisite-test/action.yml @@ -86,7 +86,7 @@ runs: tests/scripts/github-action-helper.sh restart_operator not_committed_msg="there are no changes to commit for RGW configuration period for CephObjectStore" tests/scripts/github-action-helper.sh wait_for_operator_log_message "${not_committed_msg} ${ns_name_primary_object_zone}" 120 - tests/scripts/github-action-helper.sh wait_for_operator_log_message "${not_committed_msg} ${ns_name_secondary_object_zone}" 90 + tests/scripts/github-action-helper.sh wait_for_operator_log_message "${not_committed_msg} ${ns_name_secondary_object_zone}" 120 - name: collect common logs if: always() diff --git a/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md b/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md index c7d2d9f3b..d87a49862 100644 --- a/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md +++ b/Documentation/CRDs/Object-Storage/ceph-object-store-crd.md @@ -168,7 +168,6 @@ The following CRD settings are available: * `healthCheck`: main object store health monitoring section * `startupProbe`: Disable, or override timing and threshold values of the object gateway startup probe. - * `livenessProbe`: Disable, or override timing and threshold values of the object gateway liveness probe. * `readinessProbe`: Disable, or override timing and threshold values of the object gateway readiness probe. Here is a complete example: @@ -177,10 +176,6 @@ Here is a complete example: healthCheck: startupProbe: disabled: false - livenessProbe: - disabled: false - periodSeconds: 30 - failureThreshold: 13 readinessProbe: disabled: false periodSeconds: 5 diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index 52879b35f..ac552dc27 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -12204,115 +12204,6 @@ spec: description: The RGW health probes nullable: true properties: - livenessProbe: - description: ProbeSpec is a wrapper around Probe so it can be enabled or disabled for a Ceph daemon - properties: - disabled: - description: Disabled determines whether probe is disable or not - type: boolean - probe: - description: Probe describes a health check to be performed against a container to determine whether it is alive or ready to receive traffic. - properties: - exec: - description: Exec specifies the action to take. - properties: - command: - description: Command is the command line to execute inside the container, the working directory for the command is root ('/') in the container's filesystem. The command is simply exec'd, it is not run inside a shell, so traditional shell instructions ('|', etc) won't work. To use a shell, you need to explicitly call out to that shell. Exit status of 0 is treated as live/healthy and non-zero is unhealthy. - items: - type: string - type: array - type: object - failureThreshold: - description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1. - format: int32 - type: integer - grpc: - description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate. - properties: - port: - description: Port number of the gRPC service. Number must be in the range 1 to 65535. - format: int32 - type: integer - service: - description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC." - type: string - required: - - port - type: object - httpGet: - description: HTTPGet specifies the http request to perform. - properties: - host: - description: Host name to connect to, defaults to the pod IP. You probably want to set "Host" in httpHeaders instead. - type: string - httpHeaders: - description: Custom headers to set in the request. HTTP allows repeated headers. - items: - description: HTTPHeader describes a custom header to be used in HTTP probes - properties: - name: - description: The header field name - type: string - value: - description: The header field value - type: string - required: - - name - - value - type: object - type: array - path: - description: Path to access on the HTTP server. - type: string - port: - anyOf: - - type: integer - - type: string - description: Name or number of the port to access on the container. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. - x-kubernetes-int-or-string: true - scheme: - description: Scheme to use for connecting to the host. Defaults to HTTP. - type: string - required: - - port - type: object - initialDelaySeconds: - description: 'Number of seconds after the container has started before liveness probes are initiated. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#container-probes' - format: int32 - type: integer - periodSeconds: - description: How often (in seconds) to perform the probe. Default to 10 seconds. Minimum value is 1. - format: int32 - type: integer - successThreshold: - description: Minimum consecutive successes for the probe to be considered successful after having failed. Defaults to 1. Must be 1 for liveness and startup. Minimum value is 1. - format: int32 - type: integer - tcpSocket: - description: TCPSocket specifies an action involving a TCP port. - properties: - host: - description: 'Optional: Host name to connect to, defaults to the pod IP.' - type: string - port: - anyOf: - - type: integer - - type: string - description: Number or name of the port to access on the container. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. - x-kubernetes-int-or-string: true - required: - - port - type: object - terminationGracePeriodSeconds: - description: Optional duration in seconds the pod needs to terminate gracefully upon probe failure. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. If this value is nil, the pod's terminationGracePeriodSeconds will be used. Otherwise, this value overrides the value provided by the pod spec. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). This is a beta field and requires enabling ProbeTerminationGracePeriod feature gate. Minimum value is 1. spec.terminationGracePeriodSeconds is used if unset. - format: int64 - type: integer - timeoutSeconds: - description: 'Number of seconds after which the probe times out. Defaults to 1 second. Minimum value is 1. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#container-probes' - format: int32 - type: integer - type: object - type: object readinessProbe: description: ProbeSpec is a wrapper around Probe so it can be enabled or disabled for a Ceph daemon properties: @@ -12422,6 +12313,7 @@ spec: type: integer type: object type: object + x-kubernetes-preserve-unknown-fields: true startupProbe: description: ProbeSpec is a wrapper around Probe so it can be enabled or disabled for a Ceph daemon properties: diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index 44bb7ac96..83301bbb5 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -12196,115 +12196,6 @@ spec: description: The RGW health probes nullable: true properties: - livenessProbe: - description: ProbeSpec is a wrapper around Probe so it can be enabled or disabled for a Ceph daemon - properties: - disabled: - description: Disabled determines whether probe is disable or not - type: boolean - probe: - description: Probe describes a health check to be performed against a container to determine whether it is alive or ready to receive traffic. - properties: - exec: - description: Exec specifies the action to take. - properties: - command: - description: Command is the command line to execute inside the container, the working directory for the command is root ('/') in the container's filesystem. The command is simply exec'd, it is not run inside a shell, so traditional shell instructions ('|', etc) won't work. To use a shell, you need to explicitly call out to that shell. Exit status of 0 is treated as live/healthy and non-zero is unhealthy. - items: - type: string - type: array - type: object - failureThreshold: - description: Minimum consecutive failures for the probe to be considered failed after having succeeded. Defaults to 3. Minimum value is 1. - format: int32 - type: integer - grpc: - description: GRPC specifies an action involving a GRPC port. This is a beta field and requires enabling GRPCContainerProbe feature gate. - properties: - port: - description: Port number of the gRPC service. Number must be in the range 1 to 65535. - format: int32 - type: integer - service: - description: "Service is the name of the service to place in the gRPC HealthCheckRequest (see https://github.com/grpc/grpc/blob/master/doc/health-checking.md). \n If this is not specified, the default behavior is defined by gRPC." - type: string - required: - - port - type: object - httpGet: - description: HTTPGet specifies the http request to perform. - properties: - host: - description: Host name to connect to, defaults to the pod IP. You probably want to set "Host" in httpHeaders instead. - type: string - httpHeaders: - description: Custom headers to set in the request. HTTP allows repeated headers. - items: - description: HTTPHeader describes a custom header to be used in HTTP probes - properties: - name: - description: The header field name - type: string - value: - description: The header field value - type: string - required: - - name - - value - type: object - type: array - path: - description: Path to access on the HTTP server. - type: string - port: - anyOf: - - type: integer - - type: string - description: Name or number of the port to access on the container. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. - x-kubernetes-int-or-string: true - scheme: - description: Scheme to use for connecting to the host. Defaults to HTTP. - type: string - required: - - port - type: object - initialDelaySeconds: - description: 'Number of seconds after the container has started before liveness probes are initiated. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#container-probes' - format: int32 - type: integer - periodSeconds: - description: How often (in seconds) to perform the probe. Default to 10 seconds. Minimum value is 1. - format: int32 - type: integer - successThreshold: - description: Minimum consecutive successes for the probe to be considered successful after having failed. Defaults to 1. Must be 1 for liveness and startup. Minimum value is 1. - format: int32 - type: integer - tcpSocket: - description: TCPSocket specifies an action involving a TCP port. - properties: - host: - description: 'Optional: Host name to connect to, defaults to the pod IP.' - type: string - port: - anyOf: - - type: integer - - type: string - description: Number or name of the port to access on the container. Number must be in the range 1 to 65535. Name must be an IANA_SVC_NAME. - x-kubernetes-int-or-string: true - required: - - port - type: object - terminationGracePeriodSeconds: - description: Optional duration in seconds the pod needs to terminate gracefully upon probe failure. The grace period is the duration in seconds after the processes running in the pod are sent a termination signal and the time when the processes are forcibly halted with a kill signal. Set this value longer than the expected cleanup time for your process. If this value is nil, the pod's terminationGracePeriodSeconds will be used. Otherwise, this value overrides the value provided by the pod spec. Value must be non-negative integer. The value zero indicates stop immediately via the kill signal (no opportunity to shut down). This is a beta field and requires enabling ProbeTerminationGracePeriod feature gate. Minimum value is 1. spec.terminationGracePeriodSeconds is used if unset. - format: int64 - type: integer - timeoutSeconds: - description: 'Number of seconds after which the probe times out. Defaults to 1 second. Minimum value is 1. More info: https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle#container-probes' - format: int32 - type: integer - type: object - type: object readinessProbe: description: ProbeSpec is a wrapper around Probe so it can be enabled or disabled for a Ceph daemon properties: @@ -12414,6 +12305,7 @@ spec: type: integer type: object type: object + x-kubernetes-preserve-unknown-fields: true startupProbe: description: ProbeSpec is a wrapper around Probe so it can be enabled or disabled for a Ceph daemon properties: diff --git a/deploy/examples/object-ec.yaml b/deploy/examples/object-ec.yaml index 66743943f..fffa28ddb 100644 --- a/deploy/examples/object-ec.yaml +++ b/deploy/examples/object-ec.yaml @@ -85,7 +85,7 @@ spec: # service endpoint healthcheck healthCheck: # Configure the pod probes for the rgw daemon - livenessProbe: + startupProbe: disabled: false readinessProbe: disabled: false diff --git a/deploy/examples/object-openshift.yaml b/deploy/examples/object-openshift.yaml index 41377c6a8..e85eb176b 100644 --- a/deploy/examples/object-openshift.yaml +++ b/deploy/examples/object-openshift.yaml @@ -99,7 +99,7 @@ spec: # service endpoint healthcheck healthCheck: # Configure the pod probes for the rgw daemon - livenessProbe: + startupProbe: disabled: false readinessProbe: disabled: false diff --git a/deploy/examples/object.yaml b/deploy/examples/object.yaml index 3311ab6ec..dcaf5e1f7 100644 --- a/deploy/examples/object.yaml +++ b/deploy/examples/object.yaml @@ -107,8 +107,6 @@ spec: # Configure the pod probes for the rgw daemon startupProbe: disabled: false - livenessProbe: - disabled: false readinessProbe: disabled: false # security oriented settings diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index 9fcea60a8..2e981faa7 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -1348,8 +1348,9 @@ type ObjectStoreSpec struct { // ObjectHealthCheckSpec represents the health check of an object store type ObjectHealthCheckSpec struct { - // +optional - LivenessProbe *ProbeSpec `json:"livenessProbe,omitempty"` + // livenessProbe field is no longer used + // +kubebuilder:pruning:PreserveUnknownFields + // +optional ReadinessProbe *ProbeSpec `json:"readinessProbe,omitempty"` // +optional diff --git a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go index 7c583cab4..a2832e82c 100644 --- a/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go +++ b/pkg/apis/ceph.rook.io/v1/zz_generated.deepcopy.go @@ -3025,14 +3025,35 @@ func (in *NotificationKeyFilterRule) DeepCopy() *NotificationKeyFilterRule { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ObjectEndpoints) DeepCopyInto(out *ObjectEndpoints) { + *out = *in + if in.Insecure != nil { + in, out := &in.Insecure, &out.Insecure + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.Secure != nil { + in, out := &in.Secure, &out.Secure + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ObjectEndpoints. +func (in *ObjectEndpoints) DeepCopy() *ObjectEndpoints { + if in == nil { + return nil + } + out := new(ObjectEndpoints) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObjectHealthCheckSpec) DeepCopyInto(out *ObjectHealthCheckSpec) { *out = *in - if in.LivenessProbe != nil { - in, out := &in.LivenessProbe, &out.LivenessProbe - *out = new(ProbeSpec) - (*in).DeepCopyInto(*out) - } if in.ReadinessProbe != nil { in, out := &in.ReadinessProbe, &out.ReadinessProbe *out = new(ProbeSpec) @@ -3120,6 +3141,7 @@ func (in *ObjectStoreSpec) DeepCopy() *ObjectStoreSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ObjectStoreStatus) DeepCopyInto(out *ObjectStoreStatus) { *out = *in + in.Endpoints.DeepCopyInto(&out.Endpoints) if in.Info != nil { in, out := &in.Info, &out.Info *out = make(map[string]string, len(*in)) diff --git a/pkg/operator/ceph/config/probes_test.go b/pkg/operator/ceph/config/probes_test.go index 84656811e..e2b7966f5 100644 --- a/pkg/operator/ceph/config/probes_test.go +++ b/pkg/operator/ceph/config/probes_test.go @@ -36,11 +36,9 @@ func TestConfigureLivenessProbe(t *testing.T) { cephv1.KeyRgw, } - store := cephv1.ObjectStoreSpec{} fs := cephv1.CephFilesystem{} healthCheck := cephv1.CephClusterHealthCheckSpec{} livenessProbes := map[cephv1.KeyType]*cephv1.ProbeSpec{ - "rgw": store.HealthCheck.LivenessProbe, "mds": fs.Spec.MetadataServer.LivenessProbe, "mon": healthCheck.LivenessProbe["mon"], "osd": healthCheck.LivenessProbe["osd"], diff --git a/pkg/operator/ceph/object/rgw-probe.sh b/pkg/operator/ceph/object/rgw-probe.sh new file mode 100644 index 000000000..853b430bc --- /dev/null +++ b/pkg/operator/ceph/object/rgw-probe.sh @@ -0,0 +1,79 @@ +#!/usr/bin/env bash + +PROBE_TYPE="{{ .ProbeType }}" +PROBE_PORT="{{ .Port }}" +PROBE_PROTOCOL="{{ .Protocol }}" + +# standard bash codes start at 126 and progress upward. pick error codes from 125 downward for +# script as to allow curl to output new error codes and still return a distinctive number. +USAGE_ERR_CODE=125 +PROBE_ERR_CODE=124 +# curl error codes: 1-123 + +STARTUP_TYPE='startup' +READINESS_TYPE='readiness' + +RGW_URL="$PROBE_PROTOCOL://0.0.0.0:$PROBE_PORT" + +function check() { + local URL="$1" + # --insecure - don't validate ssl if using secure port only + # --silent - don't output progress info + # --output /dev/stderr - output HTML header to stdout (good for debugging) + # --write-out '%{response_code}' - print the HTTP response code to stdout + curl --insecure --silent --output /dev/stderr --write-out '%{response_code}' "$URL" +} + +http_response="$(check "$RGW_URL")" +retcode=$? + +if [[ $retcode -ne 0 ]]; then + # if this is the startup probe, always returning failure. if startup probe passes, all subsequent + # probes can rely on the assumption that the health check was once succeeding without errors. + # if this is the readiness probe, we know that curl was previously working correctly in the + # startup probe, so curl error most likely means some new error with the RGW. + echo "RGW health check failed with error code: $retcode. the RGW likely cannot be reached by clients" >/dev/stderr + exit $retcode +fi + +RGW_RATE_LIMITING_RESPONSE=503 +RGW_MISCONFIGURATION_RESPONSE=500 + +if [[ $http_response -ge 200 ]] && [[ $http_response -lt 400 ]]; then + # 200-399 are successful responses. same behavior as Kubernetes' HTTP probe + exit 0 + +elif [[ $http_response -eq $RGW_RATE_LIMITING_RESPONSE ]]; then + # S3's '503: slow down' code is not an error but an indication that RGW is throttling client + # traffic. failing the readiness check here would only cause an increase in client connections on + # other RGWs and likely cause those to fail also in a cascade. i.e., a special healthy response. + echo "INFO: RGW is rate limiting" 2>/dev/stderr + exit 0 + +elif [[ $http_response -eq $RGW_MISCONFIGURATION_RESPONSE ]]; then + # can't specifically determine if the RGW is running or not. most likely a misconfiguration. + case "$PROBE_TYPE" in + "$STARTUP_TYPE") + # fail until we can accurately get a valid healthy response when runtime starts. + echo 'FAIL: HTTP code 500 suggests an RGW misconfiguration.' >/dev/stderr + exit $PROBE_ERR_CODE + ;; + "$READINESS_TYPE") + # config likely modified at runtime which could result in all RGWs failing this check. + # occasional client failures are still better than total failure, so ignore this + echo 'WARN: HTTP code 500 suggests an RGW misconfiguration' >/dev/stderr + exit 0 + ;; + *) + # prior arg validation means this path should never be activated, but keep to be safe + echo "ERROR: probe type is unknown: $PROBE_TYPE" >/dev/stderr + exit $USAGE_ERR_CODE + ;; + esac + +else + # anything else is a failing response. same behavior as Kubernetes' HTTP probe + echo "FAIL: received an HTTP error code: $http_response" + exit $PROBE_ERR_CODE + +fi diff --git a/pkg/operator/ceph/object/spec.go b/pkg/operator/ceph/object/spec.go index e078c8edb..b61064286 100644 --- a/pkg/operator/ceph/object/spec.go +++ b/pkg/operator/ceph/object/spec.go @@ -17,9 +17,12 @@ limitations under the License. package object import ( + "bytes" + _ "embed" "fmt" "path" "strings" + "text/template" "github.com/hashicorp/vault/api" "github.com/libopenstorage/secrets/vault" @@ -37,7 +40,6 @@ import ( ) const ( - readinessProbePath = "/swift/healthcheck" serviceAccountName = "rook-ceph-rgw" sseKMS = "ssekms" sseS3 = "sses3" @@ -68,8 +70,29 @@ chown --recursive --verbose ceph:ceph $VAULT_TOKEN_NEW_PATH var ( cephVersionMinRGWSSES3 = cephver.CephVersion{Major: 17, Minor: 2, Extra: 3} cephVersionMinRGWSSEKMSTLS = cephver.CephVersion{Major: 16, Minor: 2, Extra: 6} + + //go:embed rgw-probe.sh + rgwProbeScriptTemplate string ) +type ProbeType string +type ProtocolType string + +const ( + StartupProbeType ProbeType = "startup" + ReadinessProbeType ProbeType = "readiness" + + HTTPProtocol ProtocolType = "HTTP" + HTTPSProtocol ProtocolType = "HTTPS" +) + +type rgwProbeConfig struct { + ProbeType ProbeType + + Protocol ProtocolType + Port string +} + func (c *clusterConfig) createDeployment(rgwConfig *rgwConfig) (*apps.Deployment, error) { pod, err := c.makeRGWPodSpec(rgwConfig) if err != nil { @@ -304,6 +327,15 @@ func (c *clusterConfig) makeChownInitContainer(rgwConfig *rgwConfig) v1.Containe func (c *clusterConfig) makeDaemonContainer(rgwConfig *rgwConfig) (v1.Container, error) { // start the rgw daemon in the foreground + startupProbe, err := c.defaultStartupProbe() + if err != nil { + return v1.Container{}, errors.Wrap(err, "failed to generate default startup probe") + } + readinessProbe, err := c.defaultReadinessProbe() + if err != nil { + return v1.Container{}, errors.Wrap(err, "failed to generate default readiness probe") + } + container := v1.Container{ Name: "rgw", Image: c.clusterSpec.CephVersion.Image, @@ -328,17 +360,15 @@ func (c *clusterConfig) makeDaemonContainer(rgwConfig *rgwConfig) (v1.Container, ), Env: controller.DaemonEnvVars(c.clusterSpec.CephVersion.Image), Resources: c.store.Spec.Gateway.Resources, - StartupProbe: c.defaultStartupProbe(), - LivenessProbe: c.defaultLivenessProbe(), - ReadinessProbe: c.defaultReadinessProbe(), + StartupProbe: startupProbe, + LivenessProbe: noLivenessProbe(), + ReadinessProbe: readinessProbe, SecurityContext: controller.PodSecurityContext(), WorkingDir: cephconfig.VarLogCephDir, } // If the startup probe is enabled container = cephconfig.ConfigureStartupProbe(container, c.store.Spec.HealthCheck.StartupProbe) - // If the liveness probe is enabled - container = cephconfig.ConfigureLivenessProbe(container, c.store.Spec.HealthCheck.LivenessProbe) // If the readiness probe is enabled configureReadinessProbe(&container, c.store.Spec.HealthCheck) if c.store.Spec.IsTLSEnabled() { @@ -406,35 +436,76 @@ func configureReadinessProbe(container *v1.Container, healthCheck cephv1.ObjectH } } -func (c *clusterConfig) defaultLivenessProbe() *v1.Probe { - return &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - TCPSocket: &v1.TCPSocketAction{ - Port: c.generateProbePort(), - }, - }, - InitialDelaySeconds: 10, - } +// RGW has internal mechanisms for restarting its processing if it gets stuck. any failures detected +// in a liveness probe are likely to either (1) be resolved by the RGW internally or (2) be a result +// of connection issues to the Ceph cluster. in the first case, restarting is unnecessary. in the +// second case, restarting will only cause more load to the Ceph cluster by causing RGWs to attempt +// to re-connect, potentially causing more issues with the Ceph cluster. forcing a restart of the +// RGW is more likely to cause issues than solve them, so do not implement this probe. +func noLivenessProbe() *v1.Probe { + return nil } -func (c *clusterConfig) defaultReadinessProbe() *v1.Probe { - return &v1.Probe{ +func (c *clusterConfig) defaultReadinessProbe() (*v1.Probe, error) { + proto, port := c.endpointInfo() + cfg := rgwProbeConfig{ + ProbeType: ReadinessProbeType, + Protocol: proto, + Port: port.String(), + } + script, err := renderProbe(cfg) + if err != nil { + return nil, err + } + + probe := &v1.Probe{ ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: readinessProbePath, - Port: c.generateProbePort(), - Scheme: c.generateReadinessProbeScheme(), + Exec: &v1.ExecAction{ + Command: []string{ + "bash", "-c", script, + }, }, }, + TimeoutSeconds: 5, InitialDelaySeconds: 10, + // if RGWs aren't responding reliably, remove them from service routing until they are stable + PeriodSeconds: 10, + FailureThreshold: 3, + SuccessThreshold: 3, // don't re-add too soon to "flappy" RGWs from being rout-able } + + return probe, nil } -func (c *clusterConfig) defaultStartupProbe() *v1.Probe { - probe := c.defaultLivenessProbe() - probe.PeriodSeconds = 10 - probe.FailureThreshold = 18 - return probe +func (c *clusterConfig) defaultStartupProbe() (*v1.Probe, error) { + proto, port := c.endpointInfo() + cfg := rgwProbeConfig{ + ProbeType: StartupProbeType, + Protocol: proto, + Port: port.String(), + } + script, err := renderProbe(cfg) + if err != nil { + return nil, err + } + + probe := &v1.Probe{ + ProbeHandler: v1.ProbeHandler{ + Exec: &v1.ExecAction{ + Command: []string{ + "bash", "-c", script, + }, + }, + }, + TimeoutSeconds: 5, + InitialDelaySeconds: 10, + // RGW's default init timeout is 300 seconds; give an extra margin before the pod should be + // restarted by kubernetes + PeriodSeconds: 10, + FailureThreshold: 33, + } + + return probe, nil } func (c *clusterConfig) generateReadinessProbeScheme() v1.URIScheme { @@ -450,20 +521,25 @@ func (c *clusterConfig) generateReadinessProbeScheme() v1.URIScheme { return uriScheme } -func (c *clusterConfig) generateProbePort() intstr.IntOrString { +func (c *clusterConfig) endpointInfo() (ProtocolType, *intstr.IntOrString) { // The port the liveness probe needs to probe // Assume we run on SDN by default + proto := HTTPProtocol port := intstr.FromInt(int(rgwPortInternalPort)) // If Host Networking is enabled, the port from the spec must be reflected if c.store.Spec.IsHostNetwork(c.clusterSpec) { + proto = HTTPProtocol port = intstr.FromInt(int(c.store.Spec.Gateway.Port)) } if c.store.Spec.Gateway.Port == 0 && c.store.Spec.IsTLSEnabled() { + proto = HTTPSProtocol port = intstr.FromInt(int(c.store.Spec.Gateway.SecurePort)) } - return port + + logger.Debugf("rgw %q probe port is %v", c.store.Namespace+"/"+c.store.Name, port) + return proto, &port } func (c *clusterConfig) generateService(cephObjectStore *cephv1.CephObjectStore) *v1.Service { @@ -482,7 +558,7 @@ func (c *clusterConfig) generateService(cephObjectStore *cephv1.CephObjectStore) svc.Spec.ClusterIP = v1.ClusterIPNone } - destPort := c.generateProbePort() + _, destPort := c.endpointInfo() // When the cluster is external we must use the same one as the gateways are listening on if cephObjectStore.Spec.IsExternal() { @@ -831,3 +907,20 @@ func (c *clusterConfig) sseS3VaultTLSOptions(setOptions bool) []string { } return rgwOptions } + +func renderProbe(cfg rgwProbeConfig) (string, error) { + var writer bytes.Buffer + name := string(cfg.ProbeType) + "-probe" + + t := template.New(name) + t, err := t.Parse(rgwProbeScriptTemplate) + if err != nil { + return "", errors.Wrapf(err, "failed to parse template %q", name) + } + + if err := t.Execute(&writer, cfg); err != nil { + return "", errors.Wrapf(err, "failed to render template %q", name) + } + + return writer.String(), nil +} diff --git a/pkg/operator/ceph/object/spec_test.go b/pkg/operator/ceph/object/spec_test.go index 96a0166fd..cdc14ebfd 100644 --- a/pkg/operator/ceph/object/spec_test.go +++ b/pkg/operator/ceph/object/spec_test.go @@ -36,7 +36,6 @@ import ( k8serrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/util/intstr" ) func configureSSE(t *testing.T, c *clusterConfig, kms bool, s3 bool) { @@ -122,13 +121,12 @@ func TestPodSpecs(t *testing.T) { t.Run(("check rgw ConfigureProbe"), func(t *testing.T) { c.store.Spec.HealthCheck.StartupProbe = &cephv1.ProbeSpec{Disabled: false, Probe: &v1.Probe{InitialDelaySeconds: 1000}} - c.store.Spec.HealthCheck.LivenessProbe = &cephv1.ProbeSpec{Disabled: false, Probe: &v1.Probe{InitialDelaySeconds: 900}} deployment, err := c.makeDaemonContainer(rgwConfig) assert.NoError(t, err) - assert.NotNil(t, c.store.Spec.HealthCheck.LivenessProbe) - assert.NotNil(t, c.store.Spec.HealthCheck.StartupProbe) - assert.Equal(t, int32(900), deployment.LivenessProbe.InitialDelaySeconds) - assert.Equal(t, int32(1000), deployment.StartupProbe.InitialDelaySeconds) + assert.NotNil(t, deployment.StartupProbe) // YES startup probe + assert.Equal(t, int32(1000), deployment.StartupProbe.InitialDelaySeconds) // ^ and modified + assert.NotNil(t, deployment.ReadinessProbe) // YES readiness probe + assert.Nil(t, deployment.LivenessProbe) // NO liveness probe }) } @@ -325,154 +323,94 @@ func TestValidateSpec(t *testing.T) { assert.Nil(t, err) } -func TestDefaultLivenessProbe(t *testing.T) { - store := simpleStore() - c := &clusterConfig{ - store: store, - clusterSpec: &cephv1.ClusterSpec{ - Network: cephv1.NetworkSpec{ - HostNetwork: false, - }, - }, +func TestDefaultProbes(t *testing.T) { + type storeSpec struct { + Port int32 + SecurePort int32 + HostNetwork bool + } + type want struct { + Port int32 + Protocol ProtocolType + } + type test struct { + probeType ProbeType + name string + environment storeSpec + want want } - desiredProbe := &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - TCPSocket: &v1.TCPSocketAction{ - Port: intstr.FromInt(8080), - }, - }, - InitialDelaySeconds: 10, - } - // No SSL - HostNetwork is disabled - using internal port - p := c.defaultLivenessProbe() - assert.Equal(t, desiredProbe, p) - - // No SSL - HostNetwork is enabled - c.store.Spec.Gateway.Port = 123 - c.store.Spec.Gateway.SecurePort = 0 - c.clusterSpec.Network.HostNetwork = true - p = c.defaultLivenessProbe() - desiredProbe.ProbeHandler.TCPSocket.Port = intstr.FromInt(123) - assert.Equal(t, desiredProbe, p) - - // SSL - HostNetwork is enabled - c.store.Spec.Gateway.Port = 0 - c.store.Spec.Gateway.SecurePort = 321 - c.store.Spec.Gateway.SSLCertificateRef = "foo" - p = c.defaultLivenessProbe() - desiredProbe.ProbeHandler.TCPSocket.Port = intstr.FromInt(321) - assert.Equal(t, desiredProbe, p) - - // Both Non-SSL and SSL are enabled - // livenessProbe just on Non-SSL - c.store.Spec.Gateway.Port = 123 - c.store.Spec.Gateway.SecurePort = 321 - p = c.defaultLivenessProbe() - desiredProbe.ProbeHandler.TCPSocket.Port = intstr.FromInt(123) - assert.Equal(t, desiredProbe, p) -} - -func TestDefaultStartupProbe(t *testing.T) { - store := simpleStore() - c := &clusterConfig{ - store: store, - clusterSpec: &cephv1.ClusterSpec{ - Network: cephv1.NetworkSpec{ - HostNetwork: false, - }, - }, + tests := []test{} + // run the same tests for each probe type + // matrix: + // - internal vs host net + // - HTTP vs HTTPS vs both + for _, typ := range []ProbeType{ReadinessProbeType, StartupProbeType} { + tests = append(tests, []test{ + {typ, "internal HTTP", + storeSpec{Port: 80}, + want{Port: 8080, Protocol: HTTPProtocol}}, + {typ, "host net HTTP", + storeSpec{Port: 8088, HostNetwork: true}, + want{Port: 8088, Protocol: HTTPProtocol}}, + {typ, "internal HTTPS", + storeSpec{SecurePort: 8443}, + want{Port: 8443, Protocol: HTTPSProtocol}}, + {typ, "host net HTTPS", + storeSpec{SecurePort: 10443, HostNetwork: true}, + want{Port: 10443, Protocol: HTTPSProtocol}}, + {typ, "internal HTTP+HTTPS", + storeSpec{Port: 80, SecurePort: 443}, + want{Port: 8080, Protocol: HTTPProtocol}}, // uses HTTP + {typ, "host net HTTP+HTTPS", + storeSpec{Port: 10080, SecurePort: 10443, HostNetwork: true}, + want{Port: 10080, Protocol: HTTPProtocol}}, // uses HTTP + }...) } - desiredProbe := &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - TCPSocket: &v1.TCPSocketAction{ - Port: intstr.FromInt(8080), - }, - }, - InitialDelaySeconds: 10, - PeriodSeconds: 10, - FailureThreshold: 18, + for _, tt := range tests { + t.Run(fmt.Sprintf("%s with %s", tt.probeType, tt.name), func(t *testing.T) { + store := simpleStore() + store.Spec.Gateway.Port = tt.environment.Port + store.Spec.Gateway.SecurePort = tt.environment.SecurePort + store.Spec.Gateway.SSLCertificateRef = "foo" + c := &clusterConfig{ + store: store, + clusterSpec: &cephv1.ClusterSpec{ + Network: cephv1.NetworkSpec{ + HostNetwork: tt.environment.HostNetwork, + }, + }, + } + + var got *v1.Probe + var err error + switch tt.probeType { + case StartupProbeType: + got, err = c.defaultStartupProbe() + case ReadinessProbeType: + got, err = c.defaultReadinessProbe() + default: + panic(fmt.Sprintf("unsupported probe type %q", tt.probeType)) + } + + assert.NoError(t, err) + + // must be an exec command, called by bash; then validate core parts of the script. + // no need to validate start/period/timeout timings b/c those are business logic things + // that affect runtime stability & best verified by ci + assert.NotNil(t, got.Exec) + cmd := got.Exec.Command + assert.Equal(t, "bash", cmd[0]) + assert.Equal(t, "-c", cmd[1]) + script := cmd[2] + + // test that script config env vars are set up correctly + assert.Contains(t, script, fmt.Sprintf(`PROBE_TYPE="%s"`, tt.probeType)) + assert.Contains(t, script, fmt.Sprintf(`PROBE_PORT="%d"`, tt.want.Port)) + assert.Contains(t, script, fmt.Sprintf(`PROBE_PROTOCOL="%s"`, tt.want.Protocol)) + }) } - // No SSL - HostNetwork is disabled - using internal port - p := c.defaultStartupProbe() - assert.Equal(t, desiredProbe, p) - - // No SSL - HostNetwork is enabled - c.store.Spec.Gateway.Port = 123 - c.store.Spec.Gateway.SecurePort = 0 - c.clusterSpec.Network.HostNetwork = true - p = c.defaultStartupProbe() - desiredProbe.ProbeHandler.TCPSocket.Port = intstr.FromInt(123) - assert.Equal(t, desiredProbe, p) - - // SSL - HostNetwork is enabled - c.store.Spec.Gateway.Port = 0 - c.store.Spec.Gateway.SecurePort = 321 - c.store.Spec.Gateway.SSLCertificateRef = "foo" - p = c.defaultStartupProbe() - desiredProbe.ProbeHandler.TCPSocket.Port = intstr.FromInt(321) - assert.Equal(t, desiredProbe, p) - - // Both Non-SSL and SSL are enabled - // livenessProbe just on Non-SSL - c.store.Spec.Gateway.Port = 123 - c.store.Spec.Gateway.SecurePort = 321 - p = c.defaultStartupProbe() - desiredProbe.ProbeHandler.TCPSocket.Port = intstr.FromInt(123) - assert.Equal(t, desiredProbe, p) -} - -func TestDefaultReadinessProbe(t *testing.T) { - store := simpleStore() - c := &clusterConfig{ - store: store, - clusterSpec: &cephv1.ClusterSpec{ - Network: cephv1.NetworkSpec{ - HostNetwork: false, - }, - }, - } - - desiredProbe := &v1.Probe{ - ProbeHandler: v1.ProbeHandler{ - HTTPGet: &v1.HTTPGetAction{ - Path: readinessProbePath, - Port: intstr.FromInt(8080), - Scheme: v1.URISchemeHTTP, - }, - }, - InitialDelaySeconds: 10, - } - // No SSL - HostNetwork is disabled - using internal port - p := c.defaultReadinessProbe() - assert.Equal(t, desiredProbe, p) - - // No SSL - HostNetwork is enabled - c.store.Spec.Gateway.Port = 123 - c.store.Spec.Gateway.SecurePort = 0 - c.clusterSpec.Network.HostNetwork = true - p = c.defaultReadinessProbe() - desiredProbe.ProbeHandler.HTTPGet.Port = intstr.FromInt(123) - assert.Equal(t, desiredProbe, p) - - // SSL - HostNetwork is enabled - c.store.Spec.Gateway.Port = 0 - c.store.Spec.Gateway.SecurePort = 321 - c.store.Spec.Gateway.SSLCertificateRef = "foo" - p = c.defaultReadinessProbe() - desiredProbe.ProbeHandler.HTTPGet.Port = intstr.FromInt(321) - desiredProbe.ProbeHandler.HTTPGet.Scheme = v1.URISchemeHTTPS - assert.Equal(t, desiredProbe, p) - - // Both Non-SSL and SSL are enabled - // readinessProbe just on Non-SSL - c.store.Spec.Gateway.Port = 123 - c.store.Spec.Gateway.SecurePort = 321 - p = c.defaultReadinessProbe() - desiredProbe.ProbeHandler.HTTPGet.Port = intstr.FromInt(123) - desiredProbe.ProbeHandler.HTTPGet.Scheme = v1.URISchemeHTTP - assert.Equal(t, desiredProbe, p) } func TestCheckRGWKMS(t *testing.T) {