Add new field ImageDigests to Revision status for multi container support

pkg/apis/config/defaults.go

@@ -23,6 +23,7 @@ import (
 	"io/ioutil"
 	"math"
 	"strconv"
+	"strings"
 	"text/template"
 
 	corev1 "k8s.io/api/core/v1"
@@ -68,6 +69,18 @@ func defaultConfig() *Defaults {
 func NewDefaultsConfigFromMap(data map[string]string) (*Defaults, error) {
 	nc := defaultConfig()
 
+	// Process bool fields.
+	b := struct {
+		key          string
+		field        *bool
+		defaultValue bool
+	}{
+		key:          "enable-multi-container",
+		field:        &nc.EnableMultiContainer,
+		defaultValue: false,
+	}
+	nc.EnableMultiContainer = strings.EqualFold(data[b.key], "true")
+
 	// Process int64 fields
 	for _, i64 := range []struct {
 		key   string
@@ -158,6 +171,9 @@ func NewDefaultsConfigFromConfigMap(config *corev1.ConfigMap) (*Defaults, error)
 
 // Defaults includes the default values to be populated by the webhook.
 type Defaults struct {
+	// Feature flag to enable multi container support
+	EnableMultiContainer bool
+
 	RevisionTimeoutSeconds int64
 	// This is the timeout set for cluster ingress.
 	// RevisionTimeoutSeconds must be less than this value.

pkg/apis/serving/v1/revision_types.go

@@ -136,6 +136,12 @@ type RevisionStatus struct {
 	// may be empty if the image comes from a registry listed to skip resolution.
 	// +optional
 	ImageDigest string `json:"imageDigest,omitempty"`
+	// ImageDigests holds the resolved digest for the image specified
+	// within .Spec.Container.Image. The digest is resolved during the creation
+	// of Revision. ImageDigests holds the digest for all the .Spec.Container.Image both serving and non serving.
+	// ref: http://bit.ly/image-digests
+	// +optional
+	ImageDigests map[string]string `json:"imageDigests,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/apis/serving/v1alpha1/revision_conversion.go

@@ -62,10 +62,8 @@ func (source *RevisionSpec) ConvertTo(ctx context.Context, sink *v1.RevisionSpec
 			Volumes:            source.Volumes,
 			ImagePullSecrets:   source.ImagePullSecrets,
 		}
-	case len(source.Containers) == 1:
+	case len(source.Containers) != 0:
 		sink.PodSpec = source.PodSpec
-	case len(source.Containers) > 1:
-		return apis.ErrMultipleOneOf("containers")
 	default:
 		return apis.ErrMissingOneOf("container", "containers")
 	}
@@ -82,6 +80,7 @@ func (source *RevisionStatus) ConvertTo(ctx context.Context, sink *v1.RevisionSt
 	sink.ServiceName = source.ServiceName
 	sink.LogURL = source.LogURL
 	sink.ImageDigest = source.ImageDigest
+	sink.ImageDigests = source.ImageDigests
 }
 
 // ConvertFrom implements apis.Convertible
@@ -114,4 +113,5 @@ func (sink *RevisionStatus) ConvertFrom(ctx context.Context, source v1.RevisionS
 	sink.ServiceName = source.ServiceName
 	sink.LogURL = source.LogURL
 	sink.ImageDigest = source.ImageDigest
+	sink.ImageDigests = source.ImageDigests
 }

pkg/apis/serving/v1alpha1/revision_conversion_test.go

@@ -231,6 +231,9 @@ func TestRevisionConversionError(t *testing.T) {
 					TimeoutSeconds:       ptr.Int64(18),
 					ContainerConcurrency: ptr.Int64(53),
 				},
+				DeprecatedContainer: &corev1.Container{
+					Image: "busybox",
+				},
 			},
 			Status: RevisionStatus{
 				Status: duckv1.Status{
@@ -244,7 +247,7 @@ func TestRevisionConversionError(t *testing.T) {
 				LogURL:      "http://logger.io",
 			},
 		},
-		want: apis.ErrMultipleOneOf("containers"),
+		want: apis.ErrMultipleOneOf("container, containers"),
 	}, {
 		name: "no containers in podspec",
 		in: &Revision{

pkg/apis/serving/v1alpha1/revision_types.go

@@ -192,6 +192,12 @@ type RevisionStatus struct {
 	// may be empty if the image comes from a registry listed to skip resolution.
 	// +optional
 	ImageDigest string `json:"imageDigest,omitempty"`
+	// ImageDigests holds the resolved digest for the image specified
+	// within .Spec.Container.Image. The digest is resolved during the creation
+	// of Revision. ImageDigests holds the digest for all the .Spec.Container.Image both serving and non serving.
+	// ref: http://bit.ly/image-digests
+	// +optional
+	ImageDigests map[string]string `json:"imageDigests,omitempty"`
 }
 
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object

pkg/reconciler/revision/queueing_test.go

@@ -110,13 +110,36 @@ func testRevision() *v1.Revision {
 	rev.SetDefaults(context.Background())
 	return rev
 }
+func testRevisionForMultipleContainer() *v1.Revision {
+	return &v1.Revision{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test-rev-multi-container",
+			Namespace: testNamespace,
+		},
+		Spec: v1.RevisionSpec{
+			PodSpec: corev1.PodSpec{
+				Containers: []corev1.Container{{
+					Image: "gcr.io/repo/image",
+				}, {
+					Image: "docker.io/repo/image",
+				}},
+			},
+		},
+	}
+}
 
 func getTestDeploymentConfig() *deployment.Config {
 	c, _ := deployment.NewConfigFromConfigMap(getTestDeploymentConfigMap())
 	// ignoring error as test controller is generated
 	return c
 }
 
+func getTestDefaultConfig() *config.Defaults {
+	c, _ := config.NewDefaultsConfigFromConfigMap(getTestDefaultsConfigMap())
+	// ignoring error as test controller is generated
+	return c
+}
+
 func getTestDeploymentConfigMap() *corev1.ConfigMap {
 	return &corev1.ConfigMap{
 		ObjectMeta: metav1.ObjectMeta{

pkg/reconciler/revision/revision.go

@@ -64,7 +64,7 @@ var _ revisionreconciler.Interface = (*Reconciler)(nil)
 
 func (c *Reconciler) reconcileDigest(ctx context.Context, rev *v1.Revision) error {
 	// The image digest has already been resolved.
-	if rev.Status.ImageDigest != "" {
+	if rev.Status.ImageDigest != "" && len(rev.Status.ImageDigests) == len(rev.Spec.Containers) {
 		return nil
 	}
 
@@ -78,17 +78,31 @@ func (c *Reconciler) reconcileDigest(ctx context.Context, rev *v1.Revision) erro
 		ServiceAccountName: rev.Spec.ServiceAccountName,
 		ImagePullSecrets:   imagePullSecrets,
 	}
-	digest, err := c.resolver.Resolve(rev.Spec.GetContainer().Image,
-		opt, cfgs.Deployment.RegistriesSkippingTagResolving)
-	if err != nil {
-		err = fmt.Errorf("failed to resolve image to digest: %w", err)
-		rev.Status.MarkContainerHealthyFalse(v1.ReasonContainerMissing,
-			v1.RevisionContainerMissingMessage(
-				rev.Spec.GetContainer().Image, err.Error()))
-		return err
+	if rev.Status.ImageDigests == nil {
+		rev.Status.ImageDigests = make(map[string]string, len(rev.Spec.Containers))
 	}
+	for _, container := range rev.Spec.Containers {
+		digest, err := c.resolver.Resolve(container.Image,
+			opt, cfgs.Deployment.RegistriesSkippingTagResolving)
+		if err != nil {
+			err = fmt.Errorf("failed to resolve image to digest: %w", err)
+			rev.Status.MarkContainerHealthyFalse(v1.ReasonContainerMissing,
+				v1.RevisionContainerMissingMessage(
+					container.Image, err.Error()))
+			return err
+		}
+		if len(rev.Spec.Containers) == 1 || len(container.Ports) != 0 {
+			rev.Status.ImageDigest = digest
+		}
 
-	rev.Status.ImageDigest = digest
+		// In order to keep consistency with original behavior of single container handled below scenario
+		// revision status should not contains imageDigests field in these scenario
+		// 1. If flag is disabled
+		// 2. If flag enabled but applied revision has single container
+		if cfgs.Defaults.EnableMultiContainer && len(rev.Spec.Containers) > 1 {
+			rev.Status.ImageDigests[container.Name] = digest
+		}
+	}
 
 	return nil
 }

pkg/reconciler/revision/revision_test.go

@@ -338,6 +338,40 @@ func TestUpdateRevWithWithUpdatedLoggingURL(t *testing.T) {
 	}
 }
 
+func TestRevWithImageDigests(t *testing.T) {
+	deploymentConfig := getTestDeploymentConfig()
+	ctx, _, controller, _ := newTestControllerWithConfig(t, deploymentConfig, []*corev1.ConfigMap{{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: system.Namespace(),
+			Name:      config.DefaultsConfigName,
+		},
+		Data: map[string]string{
+			"enable-multi-container": "true",
+		},
+	},
+	})
+	// Flag is enabled and spec contains single container then rev status should not have image digests field
+	createdRev := getCreatedRev(ctx, t, controller, testRevision())
+	if len(createdRev.Status.ImageDigests) != 0 {
+		t.Errorf("Revision status does not have imageDigests")
+	}
+	// Flag is enabled and spec contains multiple container so rev status should contain image digests field
+	createdRev = getCreatedRev(ctx, t, controller, testRevisionForMultipleContainer())
+	if len(createdRev.Status.ImageDigests) == 0 {
+		t.Errorf("Revision status does not have imageDigests")
+	}
+}
+
+func getCreatedRev(ctx context.Context, t *testing.T, controller *controller.Impl, rev *v1.Revision) *v1.Revision {
+	revClient := fakeservingclient.Get(ctx).ServingV1().Revisions(testNamespace)
+	createRevision(t, ctx, controller, rev)
+	createdRev, err := revClient.Get(rev.Name, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Couldn't get revision: %v", err)
+	}
+	return createdRev
+}
+
 // TODO(mattmoor): Remove when we have coverage of EnqueueEndpointsRevision
 func TestMarkRevReadyUponEndpointBecomesReady(t *testing.T) {
 	ctx, cancel, _, ctl, _ := newTestController(t)

pkg/reconciler/revision/table_test.go

@@ -747,5 +747,6 @@ func ReconcilerTestConfig() *config.Config {
 		Logging:    &logging.Config{},
 		Tracing:    &tracingconfig.Config{},
 		Autoscaler: &autoscalerconfig.Config{},
+		Defaults:   getTestDefaultConfig(),
 	}
 }