TestLivenessProbeAwareOfStartupProbe

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

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
-}

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")
+		}
+	}
+}

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
+}