Adjusted code to be similar to Liveness logic

Signed-off-by: Janario Oliveira <[email protected]>

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,

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",

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.

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 {

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{

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.

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

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) {