Propagate ImagePullBackoff

pkg/reconciler/revision/reconcile_resources.go

@@ -20,15 +20,13 @@ import (
 	"context"
 	"fmt"
 
-	"go.uber.org/zap"
-
 	"github.com/knative/pkg/logging"
 	"github.com/knative/pkg/logging/logkey"
 	kpav1alpha1 "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
 	"github.com/knative/serving/pkg/reconciler/revision/resources"
 	resourcenames "github.com/knative/serving/pkg/reconciler/revision/resources/names"
-
+	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	apierrs "k8s.io/apimachinery/pkg/api/errors"
@@ -89,6 +87,9 @@ func (c *Reconciler) reconcileDeployment(ctx context.Context, rev *v1alpha1.Revi
 					if t := status.LastTerminationState.Terminated; t != nil {
 						logger.Infof("%s marking exiting with: %d/%s", rev.Name, t.ExitCode, t.Message)
 						rev.Status.MarkContainerExiting(t.ExitCode, t.Message)
+					} else if w := status.State.Waiting; w != nil && hasDeploymentTimedOut(deployment) {
+						logger.Infof("%s marking resources unavailable with: %s: %s", rev.Name, w.Reason, w.Message)
+						rev.Status.MarkResourcesUnavailable(w.Reason, w.Message)
 					}
 					break
 				}

pkg/reconciler/revision/table_test.go

@@ -25,6 +25,7 @@ import (
 	"github.com/knative/pkg/controller"
 	"github.com/knative/pkg/logging"
 	logtesting "github.com/knative/pkg/logging/testing"
+	. "github.com/knative/pkg/reconciler/testing"
 	autoscalingv1alpha1 "github.com/knative/serving/pkg/apis/autoscaling/v1alpha1"
 	"github.com/knative/serving/pkg/apis/networking"
 	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
@@ -35,14 +36,12 @@ import (
 	"github.com/knative/serving/pkg/reconciler"
 	"github.com/knative/serving/pkg/reconciler/revision/config"
 	"github.com/knative/serving/pkg/reconciler/revision/resources"
+	. "github.com/knative/serving/pkg/reconciler/testing"
 	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	clientgotesting "k8s.io/client-go/testing"
-
-	. "github.com/knative/pkg/reconciler/testing"
-	. "github.com/knative/serving/pkg/reconciler/testing"
 )
 
 // This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method.
@@ -432,6 +431,23 @@ func TestReconcile(t *testing.T) {
 				"deploy-timeout"),
 		},
 		Key: "foo/deploy-timeout",
+	}, {
+		Name: "surface ImagePullBackoff",
+		// Test the propagation of ImagePullBackoff from user container.
+		Objects: []runtime.Object{
+			rev("foo", "pull-backoff",
+				withK8sServiceName("the-taxman"), WithLogURL, MarkActivating("Deploying", "")),
+			kpa("foo", "pull-backoff"), // KPA can't be ready since deployment times out.
+			pod("foo", "pull-backoff", WithWaitingContainer("user-container", "ImagePullBackoff", "can't pull it")),
+			timeoutDeploy(deploy("foo", "pull-backoff")),
+			image("foo", "pull-backoff"),
+		},
+		WantStatusUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: rev("foo", "pull-backoff",
+				WithLogURL, AllUnknownConditions,
+				MarkResourcesUnavailable("ImagePullBackoff", "can't pull it")),
+		}},
+		Key: "foo/pull-backoff",
 	}, {
 		Name: "surface pod errors",
 		// Test the propagation of the termination state of a Pod into the revision.

pkg/reconciler/testing/functional.go

@@ -1011,7 +1011,7 @@ func WithFailingContainer(name string, exitCode int, message string) PodOption {
 	}
 }
 
-// WithUnschedulableContainer sets the .Status.Conditionss on the pod to
+// WithUnschedulableContainer sets the .Status.Conditions on the pod to
 // include `PodScheduled` status to `False` with the given message and reason.
 func WithUnschedulableContainer(reason, message string) PodOption {
 	return func(pod *corev1.Pod) {
@@ -1024,6 +1024,22 @@ func WithUnschedulableContainer(reason, message string) PodOption {
 	}
 }
 
+// WithWaitingContainer sets the .Status.ContainerStatuses on the pod to
+// include a container named accordingly to wait with the given state.
+func WithWaitingContainer(name, reason, message string) PodOption {
+	return func(pod *corev1.Pod) {
+		pod.Status.ContainerStatuses = []corev1.ContainerStatus{{
+			Name: name,
+			State: corev1.ContainerState{
+				Waiting: &corev1.ContainerStateWaiting{
+					Reason:  reason,
+					Message: message,
+				},
+			},
+		}}
+	}
+}
+
 // ClusterIngressOption enables further configuration of the Cluster Ingress.
 type ClusterIngressOption func(*netv1alpha1.ClusterIngress)
 

test/e2e/image_pull_error_test.go

@@ -0,0 +1,110 @@
+// +build e2e
+
+/*
+Copyright 2019 The Knative Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package e2e
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/knative/serving/pkg/apis/serving/v1alpha1"
+	serviceresourcenames "github.com/knative/serving/pkg/reconciler/service/resources/names"
+	v1alpha1testing "github.com/knative/serving/pkg/reconciler/testing"
+	"github.com/knative/serving/test"
+)
+
+func TestImagePullError(t *testing.T) {
+	clients := Setup(t)
+	const (
+		backoffMsg    = "Back-off pulling image"
+		backoffReason = "ImagePullBackOff"
+		daemonMsg     = "Error response from daemon: manifest for"
+		daemonReason  = "ErrImagePull"
+	)
+	names := test.ResourceNames{
+		Service: test.ObjectNameForTest(t),
+		// TODO: Replace this when sha256 is broken.
+		Image: "ubuntu@sha256:0000000000000000000000000000000000000000000000000000000000000000",
+	}
+
+	defer test.TearDown(clients, names)
+	test.CleanupOnInterrupt(func() { test.TearDown(clients, names) })
+
+	t.Logf("Creating a new Service %s", names.Image)
+	var (
+		svc *v1alpha1.Service
+		err error
+	)
+	if svc, err = createLatestService(t, clients, names); err != nil {
+		t.Fatalf("Failed to create Service %s: %v", names.Service, err)
+	}
+
+	names.Config = serviceresourcenames.Configuration(svc)
+
+	err = test.WaitForServiceState(clients.ServingClient, names.Service, func(r *v1alpha1.Service) (bool, error) {
+		cond := r.Status.GetCondition(v1alpha1.ConfigurationConditionReady)
+		if cond != nil && !cond.IsUnknown() {
+			if cond.IsFalse() {
+				if strings.Contains(cond.Message, backoffMsg) ||
+					strings.Contains(cond.Message, daemonMsg) {
+					return true, nil
+				}
+			}
+			t.Logf("Reason: %s ; Message: %s ; Status: %s", cond.Reason, cond.Message, cond.Status)
+			return true, fmt.Errorf("the service %s was not marked with expected error condition, but with (Reason=\"%s\", Message=\"%s\", Status=\"%s\")",
+				names.Service, cond.Reason, cond.Message, cond.Status)
+		}
+		return false, nil
+	}, "ContainerUnpullable")
+
+	if err != nil {
+		t.Fatalf("Failed to validate service state: %s", err)
+	}
+
+	revisionName, err := revisionFromConfiguration(clients, names.Config)
+	if err != nil {
+		t.Fatalf("Failed to get revision from configuration %s: %v", names.Config, err)
+	}
+
+	t.Log("When the images are not pulled, the revision should have error status.")
+	err = test.CheckRevisionState(clients.ServingClient, revisionName, func(r *v1alpha1.Revision) (bool, error) {
+		cond := r.Status.GetCondition(v1alpha1.RevisionConditionReady)
+		if cond != nil {
+			if (cond.Reason == backoffReason && strings.Contains(cond.Message, backoffMsg)) ||
+				(cond.Reason == daemonReason && strings.Contains(cond.Message, daemonMsg)) {
+				return true, nil
+			}
+			return true, fmt.Errorf("the revision %s was not marked with expected error condition, but with (Reason=%q, Message=%q)",
+				revisionName, cond.Reason, cond.Message)
+		}
+		return false, nil
+	})
+
+	if err != nil {
+		t.Fatalf("Failed to validate revision state: %s", err)
+	}
+}
+
+// Wrote our own thing so that we can pass in an image by digest.
+// knative/pkg/test.ImagePath currently assumes there's a tag, which fails to parse.
+func createLatestService(t *testing.T, clients *test.Clients, names test.ResourceNames) (*v1alpha1.Service, error) {
+	opt := v1alpha1testing.WithInlineConfigSpec(*test.ConfigurationSpec(names.Image, &test.Options{}))
+	service := v1alpha1testing.ServiceWithoutNamespace(names.Service, opt)
+	test.LogResourceObject(t, test.ResourceObjects{Service: service})
+	return clients.ServingClient.Services.Create(service)
+}