diff --git a/pkg/apis/serving/v1/revision_types.go b/pkg/apis/serving/v1/revision_types.go index 7b5765a251a1..1fa3f7780781 100644 --- a/pkg/apis/serving/v1/revision_types.go +++ b/pkg/apis/serving/v1/revision_types.go @@ -129,12 +129,32 @@ type RevisionStatus struct { // +optional LogURL string `json:"logUrl,omitempty"` - // ImageDigest holds the resolved digest for the image specified + // DeprecatedImageDigest holds the resolved digest for the image specified // within .Spec.Container.Image. The digest is resolved during the creation // of Revision. This field holds the digest value regardless of whether // a tag or digest was originally specified in the Container object. It // may be empty if the image comes from a registry listed to skip resolution. + // If multiple containers specified then DeprecatedImageDigest holds the digest + // for serving container. + // DEPRECATED Use ImageDigests instead. + // TODO(savitaashture) Remove deprecatedImageDigest. + // ref https://kubernetes.io/docs/reference/using-api/deprecation-policy for deprecation. // +optional + DeprecatedImageDigest string `json:"imageDigest,omitempty"` + + // ContainerStatuses is a slice of images present in .Spec.Container[*].Image + // to their respective digests and their container name. + // The digests are resolved during the creation of Revision. + // ContainerStatuses holds the container name and image digests + // for both serving and non serving containers. + // ref: http://bit.ly/image-digests + // +optional + ContainerStatuses []ContainerStatuses `json:"containerStatuses,omitempty"` +} + +// ContainerStatuses holds the information of container name and image digest value +type ContainerStatuses struct { + Name string `json:"name,omitempty"` ImageDigest string `json:"imageDigest,omitempty"` } diff --git a/pkg/apis/serving/v1/zz_generated.deepcopy.go b/pkg/apis/serving/v1/zz_generated.deepcopy.go index bfff38f19787..a8d09cb102bd 100644 --- a/pkg/apis/serving/v1/zz_generated.deepcopy.go +++ b/pkg/apis/serving/v1/zz_generated.deepcopy.go @@ -138,6 +138,22 @@ func (in *ConfigurationStatusFields) DeepCopy() *ConfigurationStatusFields { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ContainerStatuses) DeepCopyInto(out *ContainerStatuses) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ContainerStatuses. +func (in *ContainerStatuses) DeepCopy() *ContainerStatuses { + if in == nil { + return nil + } + out := new(ContainerStatuses) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Revision) DeepCopyInto(out *Revision) { *out = *in @@ -230,6 +246,11 @@ func (in *RevisionSpec) DeepCopy() *RevisionSpec { func (in *RevisionStatus) DeepCopyInto(out *RevisionStatus) { *out = *in in.Status.DeepCopyInto(&out.Status) + if in.ContainerStatuses != nil { + in, out := &in.ContainerStatuses, &out.ContainerStatuses + *out = make([]ContainerStatuses, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/apis/serving/v1alpha1/revision_conversion.go b/pkg/apis/serving/v1alpha1/revision_conversion.go index e54ebfeeaa5e..9487885a7d32 100644 --- a/pkg/apis/serving/v1alpha1/revision_conversion.go +++ b/pkg/apis/serving/v1alpha1/revision_conversion.go @@ -79,6 +79,16 @@ func (source *RevisionStatus) ConvertTo(ctx context.Context, sink *v1.RevisionSt source.Status.ConvertTo(ctx, &sink.Status, v1.IsRevisionCondition) sink.ServiceName = source.ServiceName sink.LogURL = source.LogURL + sink.DeprecatedImageDigest = source.DeprecatedImageDigest + sink.ContainerStatuses = make([]v1.ContainerStatuses, len(source.ContainerStatuses)) + for i := range source.ContainerStatuses { + source.ContainerStatuses[i].ConvertTo(ctx, &sink.ContainerStatuses[i]) + } +} + +// ConvertTo helps implement apis.Convertible +func (source *ContainerStatuses) ConvertTo(ctx context.Context, sink *v1.ContainerStatuses) { + sink.Name = source.Name sink.ImageDigest = source.ImageDigest } @@ -111,5 +121,15 @@ func (sink *RevisionStatus) ConvertFrom(ctx context.Context, source v1.RevisionS source.Status.ConvertTo(ctx, &sink.Status, v1.IsRevisionCondition) sink.ServiceName = source.ServiceName sink.LogURL = source.LogURL + sink.DeprecatedImageDigest = source.DeprecatedImageDigest + sink.ContainerStatuses = make([]ContainerStatuses, len(source.ContainerStatuses)) + for i := range sink.ContainerStatuses { + sink.ContainerStatuses[i].ConvertFrom(ctx, &source.ContainerStatuses[i]) + } +} + +// ConvertFrom helps implement apis.Convertible +func (sink *ContainerStatuses) ConvertFrom(ctx context.Context, source *v1.ContainerStatuses) { + sink.Name = source.Name sink.ImageDigest = source.ImageDigest } diff --git a/pkg/apis/serving/v1alpha1/revision_conversion_test.go b/pkg/apis/serving/v1alpha1/revision_conversion_test.go index 5e724255c5e6..0cca6275451d 100644 --- a/pkg/apis/serving/v1alpha1/revision_conversion_test.go +++ b/pkg/apis/serving/v1alpha1/revision_conversion_test.go @@ -89,8 +89,9 @@ func TestRevisionConversion(t *testing.T) { Status: "True", }}, }, - ServiceName: "foo-bar", - LogURL: "http://logger.io", + ServiceName: "foo-bar", + LogURL: "http://logger.io", + ContainerStatuses: []ContainerStatuses{}, }, }, }, { @@ -232,8 +233,9 @@ func TestRevisionConversionForMultiContainer(t *testing.T) { Status: "True", }}, }, - ServiceName: "foo-bar", - LogURL: "http://logger.io", + ServiceName: "foo-bar", + LogURL: "http://logger.io", + ContainerStatuses: []ContainerStatuses{}, }, } beta := &v1beta1.Revision{} diff --git a/pkg/apis/serving/v1alpha1/revision_types.go b/pkg/apis/serving/v1alpha1/revision_types.go index e3c4ce8f943e..b78d2704bbf2 100644 --- a/pkg/apis/serving/v1alpha1/revision_types.go +++ b/pkg/apis/serving/v1alpha1/revision_types.go @@ -185,12 +185,32 @@ type RevisionStatus struct { // +optional LogURL string `json:"logUrl,omitempty"` - // ImageDigest holds the resolved digest for the image specified + // DeprecatedImageDigest holds the resolved digest for the image specified // within .Spec.Container.Image. The digest is resolved during the creation // of Revision. This field holds the digest value regardless of whether // a tag or digest was originally specified in the Container object. It // may be empty if the image comes from a registry listed to skip resolution. + // If multiple containers specified then DeprecatedImageDigest holds the digest + // for serving container. + // DEPRECATED Use ImageDigests instead. + // TODO(savitaashture) Remove deprecatedImageDigest. + // ref https://kubernetes.io/docs/reference/using-api/deprecation-policy for deprecation. // +optional + DeprecatedImageDigest string `json:"imageDigest,omitempty"` + + // ContainerStatuses is a slice of images present in .Spec.Container[*].Image + // to their respective digests and their container name. + // The digests are resolved during the creation of Revision. + // ContainerStatuses holds the container name and image digests + // for both serving and non serving containers. + // ref: http://bit.ly/image-digests + // +optional + ContainerStatuses []ContainerStatuses `json:"containerStatuses,omitempty"` +} + +// ContainerStatuses holds the information of container name and image digest value +type ContainerStatuses struct { + Name string `json:"name,omitempty"` ImageDigest string `json:"imageDigest,omitempty"` } diff --git a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go index 8d347d821b62..cb524583d384 100644 --- a/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/serving/v1alpha1/zz_generated.deepcopy.go @@ -169,6 +169,22 @@ func (in *ConfigurationStatusFields) DeepCopy() *ConfigurationStatusFields { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ContainerStatuses) DeepCopyInto(out *ContainerStatuses) { + *out = *in + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ContainerStatuses. +func (in *ContainerStatuses) DeepCopy() *ContainerStatuses { + if in == nil { + return nil + } + out := new(ContainerStatuses) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ManualType) DeepCopyInto(out *ManualType) { *out = *in @@ -316,6 +332,11 @@ func (in *RevisionSpec) DeepCopy() *RevisionSpec { func (in *RevisionStatus) DeepCopyInto(out *RevisionStatus) { *out = *in in.Status.DeepCopyInto(&out.Status) + if in.ContainerStatuses != nil { + in, out := &in.ContainerStatuses, &out.ContainerStatuses + *out = make([]ContainerStatuses, len(*in)) + copy(*out, *in) + } return } diff --git a/pkg/reconciler/revision/queueing_test.go b/pkg/reconciler/revision/queueing_test.go index 7a10c480080e..2dc8cd495c23 100644 --- a/pkg/reconciler/revision/queueing_test.go +++ b/pkg/reconciler/revision/queueing_test.go @@ -60,7 +60,37 @@ const ( testQueueImage = "queueImage" ) -func testRevision() *v1.Revision { +func getPodSpec() corev1.PodSpec { + return corev1.PodSpec{ + // corev1.Container has a lot of setting. We try to pass many + // of them here to verify that we pass through the settings to + // derived objects. + Containers: []corev1.Container{{ + Image: "gcr.io/repo/image", + Command: []string{"echo"}, + Args: []string{"hello", "world"}, + WorkingDir: "/tmp", + Env: []corev1.EnvVar{{ + Name: "EDITOR", + Value: "emacs", + }}, + LivenessProbe: &corev1.Probe{ + TimeoutSeconds: 42, + }, + ReadinessProbe: &corev1.Probe{ + Handler: corev1.Handler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: "health", + }, + }, + TimeoutSeconds: 43, + }, + TerminationMessagePath: "/dev/null", + }}, + } +} + +func testRevision(podSpec corev1.PodSpec) *v1.Revision { rev := &v1.Revision{ ObjectMeta: metav1.ObjectMeta{ SelfLink: "/apis/serving/v1/namespaces/test/revisions/test-rev", @@ -77,33 +107,7 @@ func testRevision() *v1.Revision { UID: "test-rev-uid", }, Spec: v1.RevisionSpec{ - PodSpec: corev1.PodSpec{ - // corev1.Container has a lot of setting. We try to pass many - // of them here to verify that we pass through the settings to - // derived objects. - Containers: []corev1.Container{{ - Image: "gcr.io/repo/image", - Command: []string{"echo"}, - Args: []string{"hello", "world"}, - WorkingDir: "/tmp", - Env: []corev1.EnvVar{{ - Name: "EDITOR", - Value: "emacs", - }}, - LivenessProbe: &corev1.Probe{ - TimeoutSeconds: 42, - }, - ReadinessProbe: &corev1.Probe{ - Handler: corev1.Handler{ - HTTPGet: &corev1.HTTPGetAction{ - Path: "health", - }, - }, - TimeoutSeconds: 43, - }, - TerminationMessagePath: "/dev/null", - }}, - }, + PodSpec: podSpec, TimeoutSeconds: ptr.Int64(60), }, } @@ -230,7 +234,7 @@ func TestNewRevisionCallsSyncHandler(t *testing.T) { eg := errgroup.Group{} - rev := testRevision() + rev := testRevision(getPodSpec()) servingClient := fakeservingclient.Get(ctx) h := NewHooks() diff --git a/pkg/reconciler/revision/resources/deploy.go b/pkg/reconciler/revision/resources/deploy.go index e56f3b564abe..5c190c4b5e3b 100644 --- a/pkg/reconciler/revision/resources/deploy.go +++ b/pkg/reconciler/revision/resources/deploy.go @@ -159,8 +159,8 @@ func makePodSpec(rev *v1.Revision, loggingConfig *logging.Config, tracingConfig userContainer.TTY = false // Prefer imageDigest from revision if available - if rev.Status.ImageDigest != "" { - userContainer.Image = rev.Status.ImageDigest + if rev.Status.DeprecatedImageDigest != "" { + userContainer.Image = rev.Status.DeprecatedImageDigest } if userContainer.TerminationMessagePolicy == "" { diff --git a/pkg/reconciler/revision/resources/deploy_test.go b/pkg/reconciler/revision/resources/deploy_test.go index c2c1464719e4..ac7f9b56e184 100644 --- a/pkg/reconciler/revision/resources/deploy_test.go +++ b/pkg/reconciler/revision/resources/deploy_test.go @@ -495,7 +495,7 @@ func TestMakePodSpec(t *testing.T) { withContainerConcurrency(1), func(revision *v1.Revision) { revision.Status = v1.RevisionStatus{ - ImageDigest: "busybox@sha256:deadbeef", + DeprecatedImageDigest: "busybox@sha256:deadbeef", } container(revision.Spec.GetContainer(), withTCPReadinessProbe(), diff --git a/pkg/reconciler/revision/resources/imagecache.go b/pkg/reconciler/revision/resources/imagecache.go index 6080721697ee..d2816b89fb73 100644 --- a/pkg/reconciler/revision/resources/imagecache.go +++ b/pkg/reconciler/revision/resources/imagecache.go @@ -28,7 +28,7 @@ import ( // MakeImageCache makes an caching.Image resources from a revision. func MakeImageCache(rev *v1.Revision) *caching.Image { - image := rev.Status.ImageDigest + image := rev.Status.DeprecatedImageDigest if image == "" { image = rev.Spec.GetContainer().Image } diff --git a/pkg/reconciler/revision/resources/imagecache_test.go b/pkg/reconciler/revision/resources/imagecache_test.go index 966a23959974..7015476718cf 100644 --- a/pkg/reconciler/revision/resources/imagecache_test.go +++ b/pkg/reconciler/revision/resources/imagecache_test.go @@ -55,7 +55,7 @@ func TestMakeImageCache(t *testing.T) { }, }, Status: v1.RevisionStatus{ - ImageDigest: "busybox@sha256:deadbeef", + DeprecatedImageDigest: "busybox@sha256:deadbeef", }, }, want: &caching.Image{ diff --git a/pkg/reconciler/revision/revision.go b/pkg/reconciler/revision/revision.go index 73bc952a45ba..1b64ff5e25b5 100644 --- a/pkg/reconciler/revision/revision.go +++ b/pkg/reconciler/revision/revision.go @@ -22,6 +22,7 @@ import ( "strings" "github.com/google/go-containerregistry/pkg/authn/k8schain" + "golang.org/x/sync/errgroup" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -63,8 +64,23 @@ type Reconciler struct { var _ revisionreconciler.Interface = (*Reconciler)(nil) func (c *Reconciler) reconcileDigest(ctx context.Context, rev *v1.Revision) error { + if rev.Status.ContainerStatuses == nil { + rev.Status.ContainerStatuses = make([]v1.ContainerStatuses, 0, len(rev.Spec.Containers)) + } + + if rev.Status.DeprecatedImageDigest != "" { + // Default old revisions to have ContainerStatuses filled in. + // This path should only be taken by "old" revisions that have exactly one container. + if len(rev.Status.ContainerStatuses) == 0 { + rev.Status.ContainerStatuses = append(rev.Status.ContainerStatuses, v1.ContainerStatuses{ + Name: rev.Spec.Containers[0].Name, + ImageDigest: rev.Status.DeprecatedImageDigest, + }) + } + } + // The image digest has already been resolved. - if rev.Status.ImageDigest != "" { + if len(rev.Status.ContainerStatuses) == len(rev.Spec.Containers) { return nil } @@ -78,18 +94,55 @@ 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 - } - rev.Status.ImageDigest = digest + var digestGrp errgroup.Group + type digestData struct { + digestValue string + containerName string + isServingContainer bool + image string + digestError error + } + digests := make(chan digestData, len(rev.Spec.Containers)) + for _, container := range rev.Spec.Containers { + container := container // Standard Go concurrency pattern. + digestGrp.Go(func() error { + 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) + digests <- digestData{ + image: container.Image, + digestError: err, + } + } else { + digests <- digestData{ + digestValue: digest, + containerName: container.Name, + isServingContainer: len(rev.Spec.Containers) == 1 || len(container.Ports) != 0, + } + } + return nil + }) + } + digestGrp.Wait() + close(digests) + for v := range digests { + if v.digestError != nil { + rev.Status.MarkContainerHealthyFalse(v1.ReasonContainerMissing, + v1.RevisionContainerMissingMessage( + v.image, v.digestError.Error())) + return v.digestError + } + if v.isServingContainer { + rev.Status.DeprecatedImageDigest = v.digestValue + } + rev.Status.ContainerStatuses = append(rev.Status.ContainerStatuses, v1.ContainerStatuses{ + Name: v.containerName, + ImageDigest: v.digestValue, + }) + } return nil } diff --git a/pkg/reconciler/revision/revision_test.go b/pkg/reconciler/revision/revision_test.go index 78af24c3af35..934d4194e875 100644 --- a/pkg/reconciler/revision/revision_test.go +++ b/pkg/reconciler/revision/revision_test.go @@ -268,7 +268,7 @@ func TestResolutionFailed(t *testing.T) { }) defer cancel() - rev := testRevision() + rev := testRevision(getPodSpec()) config := testConfiguration() rev.OwnerReferences = append(rev.OwnerReferences, *kmeta.NewControllerRef(config)) @@ -313,7 +313,7 @@ func TestUpdateRevWithWithUpdatedLoggingURL(t *testing.T) { }) revClient := fakeservingclient.Get(ctx).ServingV1().Revisions(testNamespace) - rev := testRevision() + rev := testRevision(getPodSpec()) createRevision(t, ctx, controller, rev) // Update controllers logging URL @@ -340,11 +340,55 @@ func TestUpdateRevWithWithUpdatedLoggingURL(t *testing.T) { } } +func TestRevWithImageDigests(t *testing.T) { + deploymentConfig := getTestDeploymentConfig() + ctx, _, controller, _ := newTestControllerWithConfig(t, deploymentConfig, []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{ + Name: config.DefaultsConfigName, + Namespace: system.Namespace(), + }, + Data: map[string]string{ + "container-name-template": "user-container", + }, + }}) + + rev := testRevision(corev1.PodSpec{ + Containers: []corev1.Container{{ + Image: "gcr.io/repo/image", + Ports: []corev1.ContainerPort{{ + ContainerPort: 8888, + }}, + }, { + Image: "docker.io/repo/image", + }}, + }) + createRevision(t, ctx, controller, rev) + revClient := fakeservingclient.Get(ctx).ServingV1().Revisions(testNamespace) + rev, err := revClient.Get(rev.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get revision: %v", err) + } + if len(rev.Status.ContainerStatuses) < 2 { + t.Error("Revision status does not have imageDigests") + } + + rev.Status.DeprecatedImageDigest = "gcr.io/repo/image" + updateRevision(t, ctx, controller, rev) + if len(rev.Spec.Containers) != len(rev.Status.ContainerStatuses) { + t.Error("Image digests does not match with the provided containers") + } + rev.Status.ContainerStatuses = []v1.ContainerStatuses{} + updateRevision(t, ctx, controller, rev) + if len(rev.Status.ContainerStatuses) != 0 { + t.Error("Failed to update revision") + } +} + // TODO(mattmoor): Remove when we have coverage of EnqueueEndpointsRevision func TestMarkRevReadyUponEndpointBecomesReady(t *testing.T) { ctx, cancel, _, ctl, _ := newTestController(t) defer cancel() - rev := testRevision() + rev := testRevision(getPodSpec()) fakeRecorder := controller.GetEventRecorder(ctx).(*record.FakeRecorder) @@ -410,7 +454,7 @@ func TestNoQueueSidecarImageUpdateFail(t *testing.T) { ctx, cancel, _, controller, watcher := newTestController(t) defer cancel() - rev := testRevision() + rev := testRevision(getPodSpec()) config := testConfiguration() rev.OwnerReferences = append( rev.OwnerReferences, @@ -505,7 +549,7 @@ func TestGlobalResyncOnConfigMapUpdateRevision(t *testing.T) { servingClient := fakeservingclient.Get(ctx) - rev := testRevision() + rev := testRevision(getPodSpec()) revClient := servingClient.ServingV1().Revisions(rev.Namespace) h := NewHooks() @@ -645,7 +689,7 @@ func TestGlobalResyncOnConfigMapUpdateDeployment(t *testing.T) { kubeClient := fakekubeclient.Get(ctx) - rev := testRevision() + rev := testRevision(getPodSpec()) revClient := fakeservingclient.Get(ctx).ServingV1().Revisions(rev.Namespace) h := NewHooks() h.OnUpdate(&kubeClient.Fake, "deployments", test.callback(t)) diff --git a/pkg/reconciler/revision/table_test.go b/pkg/reconciler/revision/table_test.go index 9655b7abf344..077f60572198 100644 --- a/pkg/reconciler/revision/table_test.go +++ b/pkg/reconciler/revision/table_test.go @@ -36,6 +36,7 @@ import ( pkgreconciler "knative.dev/pkg/reconciler" tracingconfig "knative.dev/pkg/tracing/config" asv1a1 "knative.dev/serving/pkg/apis/autoscaling/v1alpha1" + defaultconfig "knative.dev/serving/pkg/apis/config" "knative.dev/serving/pkg/apis/networking" v1 "knative.dev/serving/pkg/apis/serving/v1" autoscalerconfig "knative.dev/serving/pkg/autoscaler/config" @@ -84,7 +85,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "first-reconcile", // The first reconciliation Populates the following status properties. - WithLogURL, AllUnknownConditions, MarkDeploying("Deploying")), + WithLogURL, AllUnknownConditions, MarkDeploying("Deploying"), WithImageDigests), }}, Key: "foo/first-reconcile", }, { @@ -107,7 +108,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "update-status-failure", // Despite failure, the following status properties are set. - WithLogURL, AllUnknownConditions, MarkDeploying("Deploying")), + WithLogURL, AllUnknownConditions, MarkDeploying("Deploying"), WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "UpdateFailed", "Failed to update status for %q: %v", @@ -135,7 +136,7 @@ func TestReconcile(t *testing.T) { Object: rev("foo", "create-pa-failure", // Despite failure, the following status properties are set. WithLogURL, WithInitRevConditions, - MarkDeploying("Deploying")), + MarkDeploying("Deploying"), WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", `failed to create PA "create-pa-failure": inducing failure for create podautoscalers`), @@ -161,7 +162,7 @@ func TestReconcile(t *testing.T) { Object: rev("foo", "create-user-deploy-failure", // Despite failure, the following status properties are set. WithLogURL, WithInitRevConditions, - MarkDeploying("Deploying")), + MarkDeploying("Deploying"), WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", @@ -175,7 +176,7 @@ func TestReconcile(t *testing.T) { // state (immediately post-creation), and verify that no changes // are necessary. Objects: []runtime.Object{ - rev("foo", "stable-reconcile", WithLogURL, AllUnknownConditions), + rev("foo", "stable-reconcile", WithLogURL, AllUnknownConditions, WithImageDigests), pa("foo", "stable-reconcile", WithReachability(asv1a1.ReachabilityUnknown)), deploy(t, "foo", "stable-reconcile"), @@ -189,7 +190,7 @@ func TestReconcile(t *testing.T) { // with our desired spec. Objects: []runtime.Object{ rev("foo", "fix-containers", - WithLogURL, AllUnknownConditions), + WithLogURL, AllUnknownConditions, WithImageDigests), pa("foo", "fix-containers", WithReachability(asv1a1.ReachabilityUnknown)), changeContainers(deploy(t, "foo", "fix-containers")), image("foo", "fix-containers"), @@ -207,7 +208,7 @@ func TestReconcile(t *testing.T) { }, Objects: []runtime.Object{ rev("foo", "failure-update-deploy", - withK8sServiceName("whateves"), WithLogURL, AllUnknownConditions), + withK8sServiceName("whateves"), WithLogURL, AllUnknownConditions, WithImageDigests), pa("foo", "failure-update-deploy"), changeContainers(deploy(t, "foo", "failure-update-deploy")), image("foo", "failure-update-deploy"), @@ -228,7 +229,7 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ rev("foo", "stable-deactivation", WithLogURL, MarkRevisionReady, - MarkInactive("NoTraffic", "This thing is inactive.")), + MarkInactive("NoTraffic", "This thing is inactive."), WithImageDigests), pa("foo", "stable-deactivation", WithNoTraffic("NoTraffic", "This thing is inactive.")), deploy(t, "foo", "stable-deactivation"), @@ -249,7 +250,7 @@ func TestReconcile(t *testing.T) { WithLogURL, // When the endpoint and pa are ready, then we will see the // Revision become ready. - MarkRevisionReady), + MarkRevisionReady, WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "RevisionReady", "Revision becomes ready upon all resources being ready"), @@ -270,7 +271,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "pa-not-ready", - WithLogURL, MarkRevisionReady, + WithLogURL, MarkRevisionReady, WithImageDigests, withK8sServiceName("its-not-confidential"), // When we reconcile a ready state and our pa is in an activating // state, we should see the following mutation. @@ -291,7 +292,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "pa-inactive", - WithLogURL, MarkRevisionReady, + WithLogURL, MarkRevisionReady, WithImageDigests, // When we reconcile an "all ready" revision when the PA // is inactive, we should see the following change. MarkInactive("NoTraffic", "This thing is inactive.")), @@ -316,7 +317,7 @@ func TestReconcile(t *testing.T) { withK8sServiceName("pa-inactive-svc"), // When we reconcile an "all ready" revision when the PA // is inactive, we should see the following change. - MarkInactive("NoTraffic", "This thing is inactive.")), + MarkInactive("NoTraffic", "This thing is inactive."), WithImageDigests), }}, Key: "foo/pa-inactive", }, { @@ -337,7 +338,7 @@ func TestReconcile(t *testing.T) { WithLogURL, AllUnknownConditions, // When our reconciliation has to change the service // we should see the following mutations to status. - withK8sServiceName("fix-mutated-pa"), WithLogURL, MarkRevisionReady), + withK8sServiceName("fix-mutated-pa"), WithLogURL, MarkRevisionReady, WithImageDigests), }}, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: pa("foo", "fix-mutated-pa", WithTraffic, @@ -350,7 +351,7 @@ func TestReconcile(t *testing.T) { Objects: []runtime.Object{ rev("foo", "fix-mutated-pa-fail", withK8sServiceName("some-old-stuff"), - WithLogURL, AllUnknownConditions), + WithLogURL, AllUnknownConditions, WithImageDigests), pa("foo", "fix-mutated-pa-fail", WithProtocolType(networking.ProtocolH2C), WithReachability(asv1a1.ReachabilityUnknown)), deploy(t, "foo", "fix-mutated-pa-fail"), image("foo", "fix-mutated-pa-fail"), @@ -385,7 +386,7 @@ func TestReconcile(t *testing.T) { WithLogURL, AllUnknownConditions, // When the revision is reconciled after a Deployment has // timed out, we should see it marked with the PDE state. - MarkProgressDeadlineExceeded("I timed out!")), + MarkProgressDeadlineExceeded("I timed out!"), WithImageDigests), }}, Key: "foo/deploy-timeout", }, { @@ -406,7 +407,7 @@ func TestReconcile(t *testing.T) { WithLogURL, AllUnknownConditions, // When the revision is reconciled after a Deployment has // timed out, we should see it marked with the FailedCreate state. - MarkResourcesUnavailable("FailedCreate", "I replica failed!")), + MarkResourcesUnavailable("FailedCreate", "I replica failed!"), WithImageDigests), }}, Key: "foo/deploy-replica-failure", }, { @@ -423,7 +424,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "pull-backoff", WithLogURL, AllUnknownConditions, - MarkResourcesUnavailable("ImagePullBackoff", "can't pull it")), + MarkResourcesUnavailable("ImagePullBackoff", "can't pull it"), WithImageDigests), }}, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: pa("foo", "pull-backoff", WithReachability(asv1a1.ReachabilityUnreachable)), @@ -445,7 +446,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "pod-error", - WithLogURL, AllUnknownConditions, MarkContainerExiting(5, v1.RevisionContainerExitingMessage("I failed man!"))), + WithLogURL, AllUnknownConditions, MarkContainerExiting(5, v1.RevisionContainerExitingMessage("I failed man!")), WithImageDigests), }}, Key: "foo/pod-error", }, { @@ -463,7 +464,7 @@ func TestReconcile(t *testing.T) { }, WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "pod-schedule-error", - WithLogURL, AllUnknownConditions, MarkResourcesUnavailable("Insufficient energy", "Unschedulable")), + WithLogURL, AllUnknownConditions, MarkResourcesUnavailable("Insufficient energy", "Unschedulable"), WithImageDigests), }}, Key: "foo/pod-schedule-error", }, { @@ -483,7 +484,7 @@ func TestReconcile(t *testing.T) { Object: rev("foo", "steady-ready", withK8sServiceName("steadier-even"), WithLogURL, // All resources are ready to go, we should see the revision being // marked ready - MarkRevisionReady), + MarkRevisionReady, WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeNormal, "RevisionReady", "Revision becomes ready upon all resources being ready"), @@ -503,7 +504,7 @@ func TestReconcile(t *testing.T) { Object: rev("foo", "missing-owners", withK8sServiceName("lesser-revision"), WithLogURL, MarkRevisionReady, // When we're missing the OwnerRef for PodAutoscaler we see this update. - MarkResourceNotOwned("PodAutoscaler", "missing-owners")), + MarkResourceNotOwned("PodAutoscaler", "missing-owners"), WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", `revision: "missing-owners" does not own PodAutoscaler: "missing-owners"`), @@ -514,7 +515,7 @@ func TestReconcile(t *testing.T) { WantErr: true, Objects: []runtime.Object{ rev("foo", "missing-owners", withK8sServiceName("youre-gonna-lose"), WithLogURL, - MarkRevisionReady), + MarkRevisionReady, WithImageDigests), pa("foo", "missing-owners", WithTraffic), noOwner(deploy(t, "foo", "missing-owners")), image("foo", "missing-owners"), @@ -523,7 +524,7 @@ func TestReconcile(t *testing.T) { Object: rev("foo", "missing-owners", withK8sServiceName("youre-gonna-lose"), WithLogURL, MarkRevisionReady, // When we're missing the OwnerRef for Deployment we see this update. - MarkResourceNotOwned("Deployment", "missing-owners-deployment")), + MarkResourceNotOwned("Deployment", "missing-owners-deployment"), WithImageDigests), }}, WantEvents: []string{ Eventf(corev1.EventTypeWarning, "InternalError", `revision: "missing-owners" does not own Deployment: "missing-owners-deployment"`), @@ -543,7 +544,7 @@ func TestReconcile(t *testing.T) { WantStatusUpdates: []clientgotesting.UpdateActionImpl{{ Object: rev("foo", "image-pull-secrets", WithImagePullSecrets("foo-secret"), - WithLogURL, AllUnknownConditions, MarkDeploying("Deploying")), + WithLogURL, AllUnknownConditions, MarkDeploying("Deploying"), WithImageDigests), }}, Key: "foo/image-pull-secrets", }} @@ -747,5 +748,6 @@ func ReconcilerTestConfig() *config.Config { Logging: &logging.Config{}, Tracing: &tracingconfig.Config{}, Autoscaler: &autoscalerconfig.Config{}, + Defaults: &defaultconfig.Defaults{}, } } diff --git a/pkg/testing/v1/revision.go b/pkg/testing/v1/revision.go index 40a5594704f4..ead895150a33 100644 --- a/pkg/testing/v1/revision.go +++ b/pkg/testing/v1/revision.go @@ -182,3 +182,10 @@ func WithRevisionLabel(key, value string) RevisionOption { config.Labels[key] = value } } + +// WithImageDigests sets the .Status.ImageDigests to the Revision. +func WithImageDigests(r *v1.Revision) { + r.Status.ContainerStatuses = []v1.ContainerStatuses{{ + Name: "user-container", + }} +} diff --git a/test/conformance/api/v1/util.go b/test/conformance/api/v1/util.go index 5efc7ad19c8b..1c2a7f126cc3 100644 --- a/test/conformance/api/v1/util.go +++ b/test/conformance/api/v1/util.go @@ -222,11 +222,11 @@ func validateControlPlane(t pkgTest.T, clients *test.Clients, names test.Resourc if ready, err := v1test.IsRevisionReady(r); !ready { return false, fmt.Errorf("revision %s did not become ready to serve traffic: %w", names.Revision, err) } - if r.Status.ImageDigest == "" { + if r.Status.DeprecatedImageDigest == "" { return false, fmt.Errorf("imageDigest not present for revision %s", names.Revision) } - if validDigest, err := validateImageDigest(names.Image, r.Status.ImageDigest); !validDigest { - return false, fmt.Errorf("imageDigest %s is not valid for imageName %s: %w", r.Status.ImageDigest, names.Image, err) + if validDigest, err := validateImageDigest(names.Image, r.Status.DeprecatedImageDigest); !validDigest { + return false, fmt.Errorf("imageDigest %s is not valid for imageName %s: %w", r.Status.DeprecatedImageDigest, names.Image, err) } return true, nil }) diff --git a/test/conformance/api/v1alpha1/util.go b/test/conformance/api/v1alpha1/util.go index 138980e64017..a1ea731249a2 100644 --- a/test/conformance/api/v1alpha1/util.go +++ b/test/conformance/api/v1alpha1/util.go @@ -222,11 +222,11 @@ func validateRunLatestControlPlane(t pkgTest.T, clients *test.Clients, names tes if ready, err := v1a1test.IsRevisionReady(r); !ready { return false, fmt.Errorf("revision %s did not become ready to serve traffic: %w", names.Revision, err) } - if r.Status.ImageDigest == "" { + if r.Status.DeprecatedImageDigest == "" { return false, fmt.Errorf("imageDigest not present for revision %s", names.Revision) } - if validDigest, err := validateImageDigest(names.Image, r.Status.ImageDigest); !validDigest { - return false, fmt.Errorf("imageDigest %s is not valid for imageName %s: %w", r.Status.ImageDigest, names.Image, err) + if validDigest, err := validateImageDigest(names.Image, r.Status.DeprecatedImageDigest); !validDigest { + return false, fmt.Errorf("imageDigest %s is not valid for imageName %s: %w", r.Status.DeprecatedImageDigest, names.Image, err) } return true, nil }) diff --git a/test/conformance/api/v1beta1/util.go b/test/conformance/api/v1beta1/util.go index 923c211ae47f..6add838179c3 100644 --- a/test/conformance/api/v1beta1/util.go +++ b/test/conformance/api/v1beta1/util.go @@ -222,11 +222,11 @@ func validateControlPlane(t pkgTest.T, clients *test.Clients, names test.Resourc if ready, err := v1b1test.IsRevisionReady(r); !ready { return false, fmt.Errorf("revision %s did not become ready to serve traffic: %w", names.Revision, err) } - if r.Status.ImageDigest == "" { + if r.Status.DeprecatedImageDigest == "" { return false, fmt.Errorf("imageDigest not present for revision %s", names.Revision) } - if validDigest, err := validateImageDigest(names.Image, r.Status.ImageDigest); !validDigest { - return false, fmt.Errorf("imageDigest %s is not valid for imageName %s: %w", r.Status.ImageDigest, names.Image, err) + if validDigest, err := validateImageDigest(names.Image, r.Status.DeprecatedImageDigest); !validDigest { + return false, fmt.Errorf("imageDigest %s is not valid for imageName %s: %w", r.Status.DeprecatedImageDigest, names.Image, err) } return true, nil })