From 1474d6f5d74b4ce2dd58648030fa85f33a5f8eff Mon Sep 17 00:00:00 2001 From: Fanny Jiang Date: Wed, 22 Nov 2023 17:08:02 -0500 Subject: [PATCH 1/2] Provide default readinessProbe.httpGet if not configured --- controllers/datadogagent/override/container.go | 13 ++++++++++++- controllers/datadogagent/override/container_test.go | 9 +++++++++ go.mod | 1 - go.sum | 2 -- 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/controllers/datadogagent/override/container.go b/controllers/datadogagent/override/container.go index 082e2f15d..1b5d175c0 100644 --- a/controllers/datadogagent/override/container.go +++ b/controllers/datadogagent/override/container.go @@ -15,6 +15,7 @@ import ( "github.com/DataDog/datadog-operator/controllers/datadogagent/feature" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) // Container use to override a corev1.Container with a 2alpha1.DatadogAgentGenericContainer. @@ -121,7 +122,7 @@ func overrideContainer(container *corev1.Container, override *v2alpha1.DatadogAg } if override.ReadinessProbe != nil { - container.ReadinessProbe = override.ReadinessProbe + container.ReadinessProbe = overrideReadinessProbe(override.ReadinessProbe) } if override.LivenessProbe != nil { @@ -206,3 +207,13 @@ func overrideAppArmorProfile(containerName commonv1.AgentContainerName, manager manager.Annotation().AddAnnotation(annotation, *override.AppArmorProfileName) } } + +func overrideReadinessProbe(readinessProbeOverride *corev1.Probe) *corev1.Probe { + // Add default httpGet.path and httpGet.port if not present in readinessProbe override + if readinessProbeOverride.HTTPGet == nil { + readinessProbeOverride.HTTPGet = &corev1.HTTPGetAction{ + Path: common.DefaultReadinessProbeHTTPPath, + Port: intstr.IntOrString{IntVal: common.DefaultAgentHealthPort}} + } + return readinessProbeOverride +} diff --git a/controllers/datadogagent/override/container_test.go b/controllers/datadogagent/override/container_test.go index 068534137..a01859b02 100644 --- a/controllers/datadogagent/override/container_test.go +++ b/controllers/datadogagent/override/container_test.go @@ -10,6 +10,7 @@ import ( "github.com/DataDog/datadog-operator/apis/datadoghq/common" apiutils "github.com/DataDog/datadog-operator/apis/utils" "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/apimachinery/pkg/util/intstr" "reflect" "testing" @@ -295,6 +296,14 @@ func TestContainer(t *testing.T) { assertContainerMatch(t, manager.PodTemplateSpec().Spec.Containers, containerName, func(container corev1.Container) bool { return reflect.DeepEqual( &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/ready", + Port: intstr.IntOrString{ + IntVal: 5555, + }, + }, + }, InitialDelaySeconds: 10, TimeoutSeconds: 5, PeriodSeconds: 30, diff --git a/go.mod b/go.mod index 725e5198b..e27e42323 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5 github.com/onsi/ginkgo v1.16.5 github.com/onsi/gomega v1.18.1 - github.com/openshift/api v3.9.0+incompatible github.com/pkg/errors v0.9.1 github.com/spf13/cobra v1.2.1 github.com/spf13/pflag v1.0.5 diff --git a/go.sum b/go.sum index c89759649..7b4c01807 100644 --- a/go.sum +++ b/go.sum @@ -520,8 +520,6 @@ github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1y github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= -github.com/openshift/api v3.9.0+incompatible h1:fJ/KsefYuZAjmrr3+5U9yZIZbTOpVkDDLDLFresAeYs= -github.com/openshift/api v3.9.0+incompatible/go.mod h1:dh9o4Fs58gpFXGSYfnVxGR9PnV53I8TW84pQaJDdGiY= github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o= github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs= github.com/outcaste-io/ristretto v0.2.1 h1:KCItuNIGJZcursqHr3ghO7fc5ddZLEHspL9UR0cQM64= From b9fe6f9ee4f8baa7c0e94e7377fe307c40167e7c Mon Sep 17 00:00:00 2001 From: Fanny Jiang Date: Wed, 27 Dec 2023 11:26:39 -0500 Subject: [PATCH 2/2] apply review suggestions --- .../datadogagent/override/container.go | 12 +- .../datadogagent/override/container_test.go | 108 +++++++++++++++++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/controllers/datadogagent/override/container.go b/controllers/datadogagent/override/container.go index 1b5d175c0..abd723598 100644 --- a/controllers/datadogagent/override/container.go +++ b/controllers/datadogagent/override/container.go @@ -126,7 +126,7 @@ func overrideContainer(container *corev1.Container, override *v2alpha1.DatadogAg } if override.LivenessProbe != nil { - container.LivenessProbe = override.LivenessProbe + container.LivenessProbe = overrideLivenessProbe(override.LivenessProbe) } if override.SecurityContext != nil { @@ -217,3 +217,13 @@ func overrideReadinessProbe(readinessProbeOverride *corev1.Probe) *corev1.Probe } return readinessProbeOverride } + +func overrideLivenessProbe(livenessProbeOverride *corev1.Probe) *corev1.Probe { + // Add default httpGet.path and httpGet.port if not present in livenessProbe override + if livenessProbeOverride.HTTPGet == nil { + livenessProbeOverride.HTTPGet = &corev1.HTTPGetAction{ + Path: common.DefaultLivenessProbeHTTPPath, + Port: intstr.IntOrString{IntVal: common.DefaultAgentHealthPort}} + } + return livenessProbeOverride +} diff --git a/controllers/datadogagent/override/container_test.go b/controllers/datadogagent/override/container_test.go index a01859b02..b5db482af 100644 --- a/controllers/datadogagent/override/container_test.go +++ b/controllers/datadogagent/override/container_test.go @@ -274,7 +274,7 @@ func TestContainer(t *testing.T) { }, }, { - name: "override readiness probe", + name: "override readiness probe with default HTTPGet", containerName: commonv1.CoreAgentContainerName, existingManager: func() *fake.PodTemplateManagers { return fake.NewPodTemplateManagers(t, corev1.PodTemplateSpec{ @@ -296,6 +296,11 @@ func TestContainer(t *testing.T) { assertContainerMatch(t, manager.PodTemplateSpec().Spec.Containers, containerName, func(container corev1.Container) bool { return reflect.DeepEqual( &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + PeriodSeconds: 30, + SuccessThreshold: 1, + FailureThreshold: 5, ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ Path: "/ready", @@ -304,6 +309,50 @@ func TestContainer(t *testing.T) { }, }, }, + }, + container.ReadinessProbe) + }) + }, + }, + { + name: "override readiness probe", + containerName: commonv1.CoreAgentContainerName, + existingManager: func() *fake.PodTemplateManagers { + return fake.NewPodTemplateManagers(t, corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{*agentContainer}, + }, + }) + }, + override: v2alpha1.DatadogAgentGenericContainer{ + ReadinessProbe: &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + PeriodSeconds: 30, + SuccessThreshold: 1, + FailureThreshold: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/some/path", + Port: intstr.IntOrString{ + IntVal: 1234, + }, + }, + }, + }, + }, + validateManager: func(t *testing.T, manager *fake.PodTemplateManagers, containerName string) { + assertContainerMatch(t, manager.PodTemplateSpec().Spec.Containers, containerName, func(container corev1.Container) bool { + return reflect.DeepEqual( + &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/some/path", + Port: intstr.IntOrString{ + IntVal: 1234, + }, + }, + }, InitialDelaySeconds: 10, TimeoutSeconds: 5, PeriodSeconds: 30, @@ -314,6 +363,47 @@ func TestContainer(t *testing.T) { }) }, }, + { + name: "override liveness probe with default HTTPGet", + containerName: commonv1.CoreAgentContainerName, + existingManager: func() *fake.PodTemplateManagers { + return fake.NewPodTemplateManagers(t, corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{*agentContainer}, + }, + }) + }, + override: v2alpha1.DatadogAgentGenericContainer{ + LivenessProbe: &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + PeriodSeconds: 30, + SuccessThreshold: 1, + FailureThreshold: 5, + }, + }, + validateManager: func(t *testing.T, manager *fake.PodTemplateManagers, containerName string) { + assertContainerMatch(t, manager.PodTemplateSpec().Spec.Containers, containerName, func(container corev1.Container) bool { + return reflect.DeepEqual( + &corev1.Probe{ + InitialDelaySeconds: 10, + TimeoutSeconds: 5, + PeriodSeconds: 30, + SuccessThreshold: 1, + FailureThreshold: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/live", + Port: intstr.IntOrString{ + IntVal: 5555, + }, + }, + }, + }, + container.LivenessProbe) + }) + }, + }, { name: "override liveness probe", containerName: commonv1.CoreAgentContainerName, @@ -331,6 +421,14 @@ func TestContainer(t *testing.T) { PeriodSeconds: 30, SuccessThreshold: 1, FailureThreshold: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/some/path", + Port: intstr.IntOrString{ + IntVal: 1234, + }, + }, + }, }, }, validateManager: func(t *testing.T, manager *fake.PodTemplateManagers, containerName string) { @@ -342,6 +440,14 @@ func TestContainer(t *testing.T) { PeriodSeconds: 30, SuccessThreshold: 1, FailureThreshold: 5, + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/some/path", + Port: intstr.IntOrString{ + IntVal: 1234, + }, + }, + }, }, container.LivenessProbe) })