diff --git a/pkg/testing/v1/service.go b/pkg/testing/v1/service.go index 8d616c0debf9..c84ea5298ac0 100644 --- a/pkg/testing/v1/service.go +++ b/pkg/testing/v1/service.go @@ -437,6 +437,14 @@ func WithLivenessProbe(p *corev1.Probe) ServiceOption { } } +// WithStartupProbe sets the provided probe to be the startup +// probe on the service. +func WithStartupProbe(p *corev1.Probe) ServiceOption { + return func(s *v1.Service) { + s.Spec.Template.Spec.Containers[0].StartupProbe = p + } +} + // MarkConfigurationNotReconciled calls the function of the same name on the Service's status. func MarkConfigurationNotReconciled(service *v1.Service) { service.Status.MarkConfigurationNotReconciled() diff --git a/test/conformance/runtime/liveness_probe_test.go b/test/conformance/runtime/liveness_probe_test.go index 611b64e9b5a5..6ac0a0013a14 100644 --- a/test/conformance/runtime/liveness_probe_test.go +++ b/test/conformance/runtime/liveness_probe_test.go @@ -31,7 +31,6 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" pkgtest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" - "knative.dev/serving/pkg/apis/config" resourcenames "knative.dev/serving/pkg/reconciler/revision/resources/names" v1opts "knative.dev/serving/pkg/testing/v1" "knative.dev/serving/test" @@ -99,7 +98,7 @@ func TestLivenessWithFail(t *testing.T) { } for i := range podList.Items { pod := &podList.Items[i] - if strings.Contains(pod.Name, deploymentName) && userContainerRestarted(pod) { + if strings.Contains(pod.Name, deploymentName) && test.UserContainerRestarted(pod) { t.Fatal("User container unexpectedly restarted") } } @@ -131,7 +130,7 @@ func TestLivenessWithFail(t *testing.T) { func(p *corev1.PodList) (bool, error) { for i := range p.Items { pod := &p.Items[i] - if strings.Contains(pod.Name, deploymentName) && userContainerRestarted(pod) { + if strings.Contains(pod.Name, deploymentName) && test.UserContainerRestarted(pod) { return true, nil } } @@ -170,12 +169,3 @@ func atLeastNumLivenessChecks(t *testing.T, expectedChecks int) spoof.ResponseCh return false, nil } } - -func userContainerRestarted(pod *corev1.Pod) bool { - for _, status := range pod.Status.ContainerStatuses { - if status.Name == config.DefaultUserContainerName && status.RestartCount > 0 { - return true - } - } - return false -} diff --git a/test/e2e/multicontainerprobing/multicontainer_readiness_test.go b/test/e2e/multicontainerprobing/multicontainer_readiness_test.go index 75cc56d7e985..9d497105b5fd 100644 --- a/test/e2e/multicontainerprobing/multicontainer_readiness_test.go +++ b/test/e2e/multicontainerprobing/multicontainer_readiness_test.go @@ -171,11 +171,18 @@ func TestMultiContainerReadinessDifferentProbeTypes(t *testing.T) { Ports: []corev1.ContainerPort{{ ContainerPort: 8080, }}, - }, { // Sidecar with TCPSocket readiness and liveness probes. + }, { // Sidecar with TCPSocket startup, readiness and liveness probes. Image: pkgTest.ImagePath(names.Sidecars[0]), Env: []corev1.EnvVar{ {Name: "PORT", Value: "8881"}, }, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + TCPSocket: &corev1.TCPSocketAction{ + Port: intstr.FromInt32(8881), + }, + }, + }, ReadinessProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ TCPSocket: &corev1.TCPSocketAction{ @@ -190,11 +197,19 @@ func TestMultiContainerReadinessDifferentProbeTypes(t *testing.T) { }, }, }, - }, { // Sidecar with HTTPGet readiness and Exec liveness probes. + }, { // Sidecar with HTTPGet startup, HTTPGet readiness and Exec liveness probes. Image: pkgTest.ImagePath(names.Sidecars[1]), Env: []corev1.EnvVar{ {Name: "PORT", Value: "8882"}, }, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "/healthz/readiness", + Port: intstr.FromInt32(8882), + }, + }, + }, ReadinessProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ HTTPGet: &corev1.HTTPGetAction{ @@ -210,11 +225,18 @@ func TestMultiContainerReadinessDifferentProbeTypes(t *testing.T) { }, }, }, - }, { // Sidecar with GRPC readiness and liveness probes. + }, { // Sidecar with GRPC startup, readiness and liveness probes. Image: pkgTest.ImagePath(names.Sidecars[2]), Env: []corev1.EnvVar{ {Name: "PORT", Value: "8883"}, }, + StartupProbe: &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + GRPC: &corev1.GRPCAction{ + Port: 8883, + }, + }, + }, ReadinessProbe: &corev1.Probe{ ProbeHandler: corev1.ProbeHandler{ GRPC: &corev1.GRPCAction{ diff --git a/test/e2e/readiness_test.go b/test/e2e/readiness_test.go index 27b5f8111bec..b6a356a051a0 100644 --- a/test/e2e/readiness_test.go +++ b/test/e2e/readiness_test.go @@ -21,18 +21,26 @@ package e2e import ( "context" + "strings" "testing" corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" pkgTest "knative.dev/pkg/test" "knative.dev/pkg/test/spoof" v1 "knative.dev/serving/pkg/apis/serving/v1" + resourcenames "knative.dev/serving/pkg/reconciler/revision/resources/names" v1opts "knative.dev/serving/pkg/testing/v1" "knative.dev/serving/test" v1test "knative.dev/serving/test/v1" ) +const ( + livenessPath = "/healthz/liveness" + readinessPath = "/healthz/readiness" +) + func TestReadinessAlternatePort(t *testing.T) { t.Parallel() @@ -166,3 +174,82 @@ func TestReadinessGRPCProbe(t *testing.T) { t.Fatalf("The endpoint %s for Route %s didn't return success: %v", url, names.Route, err) } } + +// TestLivenessProbeAwareOfStartupProbe verifies that liveness probes will only start after startup +// probes finished. Having a long startup probe shouldn't cause the Kubelet to restart the container +// too early due to a liveness check failing. +func TestLivenessProbeAwareOfStartupProbe(t *testing.T) { + t.Parallel() + + clients := test.Setup(t) + + names := test.ResourceNames{ + Service: test.ObjectNameForTest(t), + Image: test.Readiness, + } + + test.EnsureTearDown(t, clients, &names) + + t.Log("Creating a new Service") + resources, err := v1test.CreateServiceReady(t, clients, &names, + v1opts.WithEnv(corev1.EnvVar{ + Name: "READY_DELAY", + Value: "10s", + }), + v1opts.WithStartupProbe( + &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: readinessPath, + Port: intstr.FromInt32(8080), + }, + }, + PeriodSeconds: 1, + // Must be longer than READY_DELAY otherwise Kubelet will restart the container. + FailureThreshold: 20, + }), + v1opts.WithLivenessProbe( + &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: livenessPath, + Port: intstr.FromInt32(8080), + }, + }, + PeriodSeconds: 1, + // Intentionally shorter than READY_DELAY. + FailureThreshold: 3, + }), + ) + + if err != nil { + t.Fatalf("Failed to create initial Service: %v: %v", names.Service, err) + } + + url := resources.Route.Status.URL.URL() + if _, err := pkgTest.CheckEndpointState( + context.Background(), + clients.KubeClient, + t.Logf, + url, + spoof.MatchesAllOf(spoof.IsStatusOK, spoof.MatchesBody(test.HelloWorldText)), + "containerServesExpectedText", + test.ServingFlags.ResolvableDomain, + test.AddRootCAtoTransport(context.Background(), t.Logf, clients, test.ServingFlags.HTTPS), + ); err != nil { + t.Fatalf("The endpoint %s for Route %s didn't serve the expected text %q: %v", url, names.Route, test.HelloWorldText, err) + } + + // Check that user-container hasn't been restarted. + deploymentName := resourcenames.Deployment(resources.Revision) + podList, err := clients.KubeClient.CoreV1().Pods(test.ServingFlags.TestNamespace).List(context.Background(), metav1.ListOptions{}) + if err != nil { + t.Fatal("Unable to get pod list: ", err) + } + for i := range podList.Items { + pod := &podList.Items[i] + if strings.Contains(pod.Name, deploymentName) && test.UserContainerRestarted(pod) { + t.Fatal("User container unexpectedly restarted") + } + } +} diff --git a/test/util.go b/test/util.go index d5c03ca7cba4..2d72b9ad3937 100644 --- a/test/util.go +++ b/test/util.go @@ -31,6 +31,7 @@ import ( "knative.dev/pkg/signals" "knative.dev/pkg/test/logging" "knative.dev/pkg/test/spoof" + "knative.dev/serving/pkg/apis/config" ) const ( @@ -106,3 +107,13 @@ func AddTestAnnotation(t testing.TB, m metav1.ObjectMeta) { testAnnotation: t.Name(), }) } + +// UserContainerRestarted checks if the container was restarted. +func UserContainerRestarted(pod *corev1.Pod) bool { + for _, status := range pod.Status.ContainerStatuses { + if status.Name == config.DefaultUserContainerName && status.RestartCount > 0 { + return true + } + } + return false +}