diff --git a/apis/v1alpha1/convert.go b/apis/v1alpha1/convert.go index e38a1d6fed..aa67903cb9 100644 --- a/apis/v1alpha1/convert.go +++ b/apis/v1alpha1/convert.go @@ -25,7 +25,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/conversion" "github.com/open-telemetry/opentelemetry-operator/apis/v1beta1" - v1 "k8s.io/api/core/v1" ) var _ conversion.Convertible = &OpenTelemetryCollector{} @@ -130,15 +129,7 @@ func tov1beta1(in OpenTelemetryCollector) (v1beta1.OpenTelemetryCollector, error }, }, LivenessProbe: tov1beta1Probe(copy.Spec.LivenessProbe), - ReadinessProbe: &v1.Probe{ - InitialDelaySeconds: copy.Spec.ReadinessProbe.InitialDelaySeconds, - TimeoutSeconds: copy.Spec.ReadinessProbe.TimeoutSeconds, - PeriodSeconds: copy.Spec.ReadinessProbe.PeriodSeconds, - SuccessThreshold: copy.Spec.ReadinessProbe.SuccessThreshold, - FailureThreshold: copy.Spec.ReadinessProbe.FailureThreshold, - TerminationGracePeriodSeconds: copy.Spec.ReadinessProbe.TerminationGracePeriodSeconds, - ProbeHandler: *copy.Spec.ReadinessProbe.ProbeHandler.DeepCopy(), - }, + ReadinessProbe: tov1beta1Probe(copy.Spec.ReadinessProbe), Observability: v1beta1.ObservabilitySpec{ Metrics: v1beta1.MetricsConfigSpec{ EnableMetrics: copy.Spec.Observability.Metrics.EnableMetrics, @@ -362,17 +353,9 @@ func tov1alpha1(in v1beta1.OpenTelemetryCollector) (*OpenTelemetryCollector, err Lifecycle: copy.Spec.Lifecycle, TerminationGracePeriodSeconds: copy.Spec.TerminationGracePeriodSeconds, LivenessProbe: tov1alpha1Probe(copy.Spec.LivenessProbe), - ReadinessProbe: &v1.Probe{ - InitialDelaySeconds: copy.Spec.ReadinessProbe.InitialDelaySeconds, - TimeoutSeconds: copy.Spec.ReadinessProbe.TimeoutSeconds, - PeriodSeconds: copy.Spec.ReadinessProbe.PeriodSeconds, - SuccessThreshold: copy.Spec.ReadinessProbe.SuccessThreshold, - FailureThreshold: copy.Spec.ReadinessProbe.FailureThreshold, - TerminationGracePeriodSeconds: copy.Spec.ReadinessProbe.TerminationGracePeriodSeconds, - ProbeHandler: *copy.Spec.ReadinessProbe.ProbeHandler.DeepCopy(), - }, - InitContainers: copy.Spec.InitContainers, - AdditionalContainers: copy.Spec.AdditionalContainers, + ReadinessProbe: tov1alpha1Probe(copy.Spec.ReadinessProbe), + InitContainers: copy.Spec.InitContainers, + AdditionalContainers: copy.Spec.AdditionalContainers, Observability: ObservabilitySpec{ Metrics: MetricsConfigSpec{ EnableMetrics: copy.Spec.Observability.Metrics.EnableMetrics, diff --git a/apis/v1alpha1/convert_test.go b/apis/v1alpha1/convert_test.go index ba8bb063d0..0510fcea60 100644 --- a/apis/v1alpha1/convert_test.go +++ b/apis/v1alpha1/convert_test.go @@ -266,6 +266,9 @@ func Test_tov1beta1AndBack(t *testing.T) { LivenessProbe: &Probe{ PeriodSeconds: &one, }, + ReadinessProbe: &Probe{ + PeriodSeconds: &one, + }, InitContainers: []v1.Container{ { Name: "init", diff --git a/apis/v1alpha1/opentelemetrycollector_types.go b/apis/v1alpha1/opentelemetrycollector_types.go index f058a2e841..8ee67cb5de 100644 --- a/apis/v1alpha1/opentelemetrycollector_types.go +++ b/apis/v1alpha1/opentelemetrycollector_types.go @@ -236,8 +236,10 @@ type OpenTelemetryCollectorSpec struct { // It is only effective when healthcheckextension is configured in the OpenTelemetry Collector pipeline. // +optional LivenessProbe *Probe `json:"livenessProbe,omitempty"` + // Readiness config for the OpenTelemetry Collector except the probe handler which is auto generated from the health extension of the collector. + // It is only effective when healthcheckextension is configured in the OpenTelemetry Collector pipeline. // +optional - ReadinessProbe *v1.Probe `json:"readinessProbe,omitempty"` + ReadinessProbe *Probe `json:"readinessProbe,omitempty"` // InitContainers allows injecting initContainers to the Collector's pod definition. // These init containers can be used to fetch secrets for injection into the // configuration from external sources, run added checks, etc. Any errors during the execution of @@ -553,7 +555,7 @@ type ObservabilitySpec struct { Metrics MetricsConfigSpec `json:"metrics,omitempty"` } -// Probe defines the OpenTelemetry's pod probe config. Only Liveness probe is supported currently. +// Probe defines the OpenTelemetry's pod probe config. type Probe struct { // Number of seconds after the container has started before liveness probes are initiated. // Defaults to 0 seconds. Minimum value is 0. diff --git a/apis/v1beta1/collector_webhook.go b/apis/v1beta1/collector_webhook.go index 8780ffa7e3..941c0de3f3 100644 --- a/apis/v1beta1/collector_webhook.go +++ b/apis/v1beta1/collector_webhook.go @@ -309,6 +309,26 @@ func (c CollectorWebhook) validate(ctx context.Context, r *OpenTelemetryCollecto return warnings, fmt.Errorf("the OpenTelemetry Spec LivenessProbe TerminationGracePeriodSeconds configuration is incorrect. TerminationGracePeriodSeconds should be greater than or equal to 1") } } + if r.Spec.ReadinessProbe != nil { + if r.Spec.ReadinessProbe.InitialDelaySeconds != nil && *r.Spec.ReadinessProbe.InitialDelaySeconds < 0 { + return warnings, fmt.Errorf("the OpenTelemetry Spec ReadinessProbe InitialDelaySeconds configuration is incorrect. InitialDelaySeconds should be greater than or equal to 0") + } + if r.Spec.ReadinessProbe.PeriodSeconds != nil && *r.Spec.ReadinessProbe.PeriodSeconds < 1 { + return warnings, fmt.Errorf("the OpenTelemetry Spec ReadinessProbe PeriodSeconds configuration is incorrect. PeriodSeconds should be greater than or equal to 1") + } + if r.Spec.ReadinessProbe.TimeoutSeconds != nil && *r.Spec.ReadinessProbe.TimeoutSeconds < 1 { + return warnings, fmt.Errorf("the OpenTelemetry Spec ReadinessProbe TimeoutSeconds configuration is incorrect. TimeoutSeconds should be greater than or equal to 1") + } + if r.Spec.ReadinessProbe.SuccessThreshold != nil && *r.Spec.ReadinessProbe.SuccessThreshold < 1 { + return warnings, fmt.Errorf("the OpenTelemetry Spec ReadinessProbe SuccessThreshold configuration is incorrect. SuccessThreshold should be greater than or equal to 1") + } + if r.Spec.ReadinessProbe.FailureThreshold != nil && *r.Spec.ReadinessProbe.FailureThreshold < 1 { + return warnings, fmt.Errorf("the OpenTelemetry Spec ReadinessProbe FailureThreshold configuration is incorrect. FailureThreshold should be greater than or equal to 1") + } + if r.Spec.ReadinessProbe.TerminationGracePeriodSeconds != nil && *r.Spec.ReadinessProbe.TerminationGracePeriodSeconds < 1 { + return warnings, fmt.Errorf("the OpenTelemetry Spec ReadinessProbe TerminationGracePeriodSeconds configuration is incorrect. TerminationGracePeriodSeconds should be greater than or equal to 1") + } + } // validate updateStrategy for DaemonSet if r.Spec.Mode != ModeDaemonSet && len(r.Spec.DaemonSetUpdateStrategy.Type) > 0 { diff --git a/apis/v1beta1/collector_webhook_test.go b/apis/v1beta1/collector_webhook_test.go index 99d127b3ef..b3b8ade472 100644 --- a/apis/v1beta1/collector_webhook_test.go +++ b/apis/v1beta1/collector_webhook_test.go @@ -1026,6 +1026,17 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Spec LivenessProbe InitialDelaySeconds configuration is incorrect", }, + { + name: "invalid InitialDelaySeconds readiness", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + ReadinessProbe: &Probe{ + InitialDelaySeconds: &minusOne, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec ReadinessProbe InitialDelaySeconds configuration is incorrect", + }, { name: "invalid PeriodSeconds", otelcol: OpenTelemetryCollector{ @@ -1037,6 +1048,17 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Spec LivenessProbe PeriodSeconds configuration is incorrect", }, + { + name: "invalid PeriodSeconds readiness", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + ReadinessProbe: &Probe{ + PeriodSeconds: &zero, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec ReadinessProbe PeriodSeconds configuration is incorrect", + }, { name: "invalid TimeoutSeconds", otelcol: OpenTelemetryCollector{ @@ -1048,6 +1070,17 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Spec LivenessProbe TimeoutSeconds configuration is incorrect", }, + { + name: "invalid TimeoutSeconds readiness", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + ReadinessProbe: &Probe{ + TimeoutSeconds: &zero, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec ReadinessProbe TimeoutSeconds configuration is incorrect", + }, { name: "invalid SuccessThreshold", otelcol: OpenTelemetryCollector{ @@ -1059,6 +1092,17 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Spec LivenessProbe SuccessThreshold configuration is incorrect", }, + { + name: "invalid SuccessThreshold readiness", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + ReadinessProbe: &Probe{ + SuccessThreshold: &zero, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec ReadinessProbe SuccessThreshold configuration is incorrect", + }, { name: "invalid FailureThreshold", otelcol: OpenTelemetryCollector{ @@ -1070,6 +1114,17 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Spec LivenessProbe FailureThreshold configuration is incorrect", }, + { + name: "invalid FailureThreshold readiness", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + ReadinessProbe: &Probe{ + FailureThreshold: &zero, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec ReadinessProbe FailureThreshold configuration is incorrect", + }, { name: "invalid TerminationGracePeriodSeconds", otelcol: OpenTelemetryCollector{ @@ -1081,6 +1136,17 @@ func TestOTELColValidatingWebhook(t *testing.T) { }, expectedErr: "the OpenTelemetry Spec LivenessProbe TerminationGracePeriodSeconds configuration is incorrect", }, + { + name: "invalid TerminationGracePeriodSeconds readiness", + otelcol: OpenTelemetryCollector{ + Spec: OpenTelemetryCollectorSpec{ + ReadinessProbe: &Probe{ + TerminationGracePeriodSeconds: &zero64, + }, + }, + }, + expectedErr: "the OpenTelemetry Spec ReadinessProbe TerminationGracePeriodSeconds configuration is incorrect", + }, { name: "invalid AdditionalContainers", otelcol: OpenTelemetryCollector{ diff --git a/apis/v1beta1/opentelemetrycollector_types.go b/apis/v1beta1/opentelemetrycollector_types.go index 7ad6c6fa94..141178895f 100644 --- a/apis/v1beta1/opentelemetrycollector_types.go +++ b/apis/v1beta1/opentelemetrycollector_types.go @@ -103,8 +103,10 @@ type OpenTelemetryCollectorSpec struct { // It is only effective when healthcheckextension is configured in the OpenTelemetry Collector pipeline. // +optional LivenessProbe *Probe `json:"livenessProbe,omitempty"` + // Readiness config for the OpenTelemetry Collector except the probe handler which is auto generated from the health extension of the collector. + // It is only effective when healthcheckextension is configured in the OpenTelemetry Collector pipeline. // +optional - ReadinessProbe *v1.Probe `json:"readinessProbe,omitempty"` + ReadinessProbe *Probe `json:"readinessProbe,omitempty"` // ObservabilitySpec defines how telemetry data gets handled. // @@ -208,7 +210,7 @@ type TargetAllocatorEmbedded struct { PodDisruptionBudget *PodDisruptionBudgetSpec `json:"podDisruptionBudget,omitempty"` } -// Probe defines the OpenTelemetry's pod probe config. Only Liveness probe is supported currently. +// Probe defines the OpenTelemetry's pod probe config. type Probe struct { // Number of seconds after the container has started before liveness probes are initiated. // Defaults to 0 seconds. Minimum value is 0. diff --git a/internal/manifests/collector/container.go b/internal/manifests/collector/container.go index 3df4040ad8..193596f038 100644 --- a/internal/manifests/collector/container.go +++ b/internal/manifests/collector/container.go @@ -140,8 +140,9 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme } var livenessProbe *corev1.Probe + var readinessProbe *corev1.Probe if configFromString, err := adapters.ConfigFromString(configYaml); err == nil { - if probe, err := getLivenessProbe(configFromString, otelcol.Spec.LivenessProbe); err == nil { + if probe, err := getProbe(configFromString, otelcol.Spec.LivenessProbe); err == nil { livenessProbe = probe } else if errors.Is(err, adapters.ErrNoServiceExtensions) { logger.V(4).Info("extensions not configured, skipping liveness probe creation") @@ -150,9 +151,17 @@ func Container(cfg config.Config, logger logr.Logger, otelcol v1beta1.OpenTeleme } else { logger.Error(err, "cannot create liveness probe.") } - } - var readinessProbe = otelcol.Spec.ReadinessProbe + if probe, err := getProbe(configFromString, otelcol.Spec.ReadinessProbe); err == nil { + readinessProbe = probe + } else if errors.Is(err, adapters.ErrNoServiceExtensions) { + logger.V(4).Info("extensions not configured, skipping readiness probe creation") + } else if errors.Is(err, adapters.ErrNoServiceExtensionHealthCheck) { + logger.V(4).Info("healthcheck extension not configured, skipping readiness probe creation") + } else { + logger.Error(err, "cannot create readiness probe.") + } + } envVars = append(envVars, proxy.ReadProxyVarsFromEnv()...) return corev1.Container{ @@ -230,7 +239,7 @@ func portMapToList(portMap map[string]corev1.ContainerPort) []corev1.ContainerPo return ports } -func getLivenessProbe(config map[interface{}]interface{}, probeConfig *v1beta1.Probe) (*corev1.Probe, error) { +func getProbe(config map[interface{}]interface{}, probeConfig *v1beta1.Probe) (*corev1.Probe, error) { probe, err := adapters.ConfigToContainerProbe(config) if err != nil { return nil, err diff --git a/internal/manifests/collector/container_test.go b/internal/manifests/collector/container_test.go index e2cd24639d..5b6be02367 100644 --- a/internal/manifests/collector/container_test.go +++ b/internal/manifests/collector/container_test.go @@ -728,6 +728,14 @@ service: FailureThreshold: &failureThreshold, TerminationGracePeriodSeconds: &terminationGracePeriodSeconds, }, + ReadinessProbe: &v1beta1.Probe{ + InitialDelaySeconds: &initialDelaySeconds, + TimeoutSeconds: &timeoutSeconds, + PeriodSeconds: &periodSeconds, + SuccessThreshold: &successThreshold, + FailureThreshold: &failureThreshold, + TerminationGracePeriodSeconds: &terminationGracePeriodSeconds, + }, }, } cfg := config.New() @@ -736,6 +744,7 @@ service: c := Container(cfg, logger, otelcol, true) // verify + // liveness assert.Equal(t, "/", c.LivenessProbe.HTTPGet.Path) assert.Equal(t, int32(13133), c.LivenessProbe.HTTPGet.Port.IntVal) assert.Equal(t, "", c.LivenessProbe.HTTPGet.Host) @@ -746,6 +755,18 @@ service: assert.Equal(t, successThreshold, c.LivenessProbe.SuccessThreshold) assert.Equal(t, failureThreshold, c.LivenessProbe.FailureThreshold) assert.Equal(t, terminationGracePeriodSeconds, *c.LivenessProbe.TerminationGracePeriodSeconds) + + // rediness + assert.Equal(t, "/", c.ReadinessProbe.HTTPGet.Path) + assert.Equal(t, int32(13133), c.ReadinessProbe.HTTPGet.Port.IntVal) + assert.Equal(t, "", c.ReadinessProbe.HTTPGet.Host) + + assert.Equal(t, initialDelaySeconds, c.ReadinessProbe.InitialDelaySeconds) + assert.Equal(t, timeoutSeconds, c.ReadinessProbe.TimeoutSeconds) + assert.Equal(t, periodSeconds, c.ReadinessProbe.PeriodSeconds) + assert.Equal(t, successThreshold, c.ReadinessProbe.SuccessThreshold) + assert.Equal(t, failureThreshold, c.ReadinessProbe.FailureThreshold) + assert.Equal(t, terminationGracePeriodSeconds, *c.ReadinessProbe.TerminationGracePeriodSeconds) } func TestContainerProbeEmptyConfig(t *testing.T) { @@ -758,6 +779,7 @@ func TestContainerProbeEmptyConfig(t *testing.T) { service: extensions: [health_check]`), LivenessProbe: &v1beta1.Probe{}, + ReadinessProbe: &v1beta1.Probe{}, }, } cfg := config.New() @@ -766,9 +788,14 @@ service: c := Container(cfg, logger, otelcol, true) // verify + // liveness assert.Equal(t, "/", c.LivenessProbe.HTTPGet.Path) assert.Equal(t, int32(13133), c.LivenessProbe.HTTPGet.Port.IntVal) assert.Equal(t, "", c.LivenessProbe.HTTPGet.Host) + // readiness + assert.Equal(t, "/", c.ReadinessProbe.HTTPGet.Path) + assert.Equal(t, int32(13133), c.ReadinessProbe.HTTPGet.Port.IntVal) + assert.Equal(t, "", c.ReadinessProbe.HTTPGet.Host) } func TestContainerProbeNoConfig(t *testing.T) {