diff --git a/pkg/reconciler/v1alpha1/configuration/configuration.go b/pkg/reconciler/v1alpha1/configuration/configuration.go index d7ea29da67de..dbd747b7b877 100644 --- a/pkg/reconciler/v1alpha1/configuration/configuration.go +++ b/pkg/reconciler/v1alpha1/configuration/configuration.go @@ -218,19 +218,26 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati func (c *Reconciler) createRevision(ctx context.Context, config *v1alpha1.Configuration, revName string) (*v1alpha1.Revision, error) { logger := logging.FromContext(ctx) + var buildRef *corev1.ObjectReference if config.Spec.Build != nil { // TODO(mattmoor): Determine whether we reuse the previous build. build := resources.MakeBuild(config) gvr, _ := meta.UnsafeGuessKindToResource(build.GroupVersionKind()) + created, err := c.DynamicClientSet.Resource(gvr).Namespace(build.GetNamespace()).Create(build) if err != nil { return nil, err } logger.Infof("Created Build:\n%+v", created.GetName()) c.Recorder.Eventf(config, corev1.EventTypeNormal, "Created", "Created Build %q", created.GetName()) + buildRef = &corev1.ObjectReference{ + APIVersion: created.GetAPIVersion(), + Kind: created.GetKind(), + Name: created.GetName(), + } } - rev := resources.MakeRevision(config) + rev := resources.MakeRevision(config, buildRef) created, err := c.ServingClientSet.ServingV1alpha1().Revisions(config.Namespace).Create(rev) if err != nil { return nil, err diff --git a/pkg/reconciler/v1alpha1/configuration/configuration_test.go b/pkg/reconciler/v1alpha1/configuration/configuration_test.go index 710dbf69757e..06bca15e918d 100644 --- a/pkg/reconciler/v1alpha1/configuration/configuration_test.go +++ b/pkg/reconciler/v1alpha1/configuration/configuration_test.go @@ -71,7 +71,7 @@ func TestReconcile(t *testing.T) { cfg("no-revisions-yet", "foo", 1234), }, WantCreates: []metav1.Object{ - resources.MakeRevision(cfg("no-revisions-yet", "foo", 1234)), + resources.MakeRevision(cfg("no-revisions-yet", "foo", 1234), nil), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("no-revisions-yet", "foo", 1234, @@ -93,7 +93,7 @@ func TestReconcile(t *testing.T) { setConcurrencyModel(cfg("validation-failure", "foo", 1234), "Bogus"), }, WantCreates: []metav1.Object{ - setRevConcurrencyModel(resources.MakeRevision(cfg("validation-failure", "foo", 1234)), "Bogus"), + setRevConcurrencyModel(resources.MakeRevision(cfg("validation-failure", "foo", 1234), nil), "Bogus"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: setConcurrencyModel(cfgWithStatus("validation-failure", "foo", 1234, @@ -114,7 +114,11 @@ func TestReconcile(t *testing.T) { }, WantCreates: []metav1.Object{ resources.MakeBuild(cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec)), - resources.MakeRevision(cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec)), + resources.MakeRevision(cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec), &corev1.ObjectReference{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "Build", + Name: "need-rev-and-build-99998", + }), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithBuildAndStatus("need-rev-and-build", "foo", 99998, &buildSpec, @@ -133,7 +137,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile revision matching generation (ready: unknown)", Objects: []runtime.Object{ cfg("matching-revision-not-done", "foo", 5432), - setRevCreationTimestamp(resources.MakeRevision(cfg("matching-revision-not-done", "foo", 5432)), time.Now()), + setRevCreationTimestamp(resources.MakeRevision(cfg("matching-revision-not-done", "foo", 5432), nil), time.Now()), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("matching-revision-not-done", "foo", 5432, @@ -152,7 +156,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile revision matching generation (ready: true)", Objects: []runtime.Object{ cfg("matching-revision-done", "foo", 5555), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done", "foo", 5555))), time.Now()), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done", "foo", 5555), nil)), time.Now()), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("matching-revision-done", "foo", 5555, @@ -182,14 +186,14 @@ func TestReconcile(t *testing.T) { }}, }, ), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done-idempotent", "foo", 5566))), time.Now()), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done-idempotent", "foo", 5566), nil)), time.Now()), }, Key: "foo/matching-revision-done-idempotent", }, { Name: "reconcile revision matching generation (ready: false)", Objects: []runtime.Object{ cfg("matching-revision-failed", "foo", 5555), - setRevCreationTimestamp(makeRevFailed(resources.MakeRevision(cfg("matching-revision-failed", "foo", 5555))), time.Now()), + setRevCreationTimestamp(makeRevFailed(resources.MakeRevision(cfg("matching-revision-failed", "foo", 5555), nil)), time.Now()), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("matching-revision-failed", "foo", 5555, @@ -210,7 +214,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile revision matching generation (ready: bad)", Objects: []runtime.Object{ cfg("bad-condition", "foo", 5555), - makeRevStatus(resources.MakeRevision(cfg("bad-condition", "foo", 5555)), + makeRevStatus(resources.MakeRevision(cfg("bad-condition", "foo", 5555), nil), v1alpha1.RevisionStatus{ Conditions: duckv1alpha1.Conditions{{ Type: v1alpha1.RevisionConditionReady, @@ -271,7 +275,7 @@ func TestReconcile(t *testing.T) { cfg("create-revision-failure", "foo", 99998), }, WantCreates: []metav1.Object{ - resources.MakeRevision(cfg("create-revision-failure", "foo", 99998)), + resources.MakeRevision(cfg("create-revision-failure", "foo", 99998), nil), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("create-revision-failure", "foo", 99998, @@ -297,7 +301,7 @@ func TestReconcile(t *testing.T) { cfg("update-config-failure", "foo", 1234), }, WantCreates: []metav1.Object{ - resources.MakeRevision(cfg("update-config-failure", "foo", 1234)), + resources.MakeRevision(cfg("update-config-failure", "foo", 1234), nil), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("update-config-failure", "foo", 1234, @@ -327,7 +331,7 @@ func TestReconcile(t *testing.T) { }}, }, ), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("revision-recovers", "foo", 1337))), time.Now()), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("revision-recovers", "foo", 1337), nil)), time.Now()), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("revision-recovers", "foo", 1337, @@ -362,9 +366,9 @@ func TestGCReconcile(t *testing.T) { Name: "dont gc old revision no pin", Objects: []runtime.Object{ cfg("old-revision", "foo", 5556), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5554))), time.Now().Add(-10*time.Minute)), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5555))), time.Now().Add(-10*time.Minute)), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556))), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5554), nil)), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5555), nil)), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556), nil)), time.Now().Add(-10*time.Minute)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("old-revision", "foo", 5556, @@ -384,7 +388,7 @@ func TestGCReconcile(t *testing.T) { Name: "dont gc old latest revision no pin", Objects: []runtime.Object{ cfg("old-revision", "foo", 5556), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556))), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556), nil)), time.Now().Add(-10*time.Minute)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("old-revision", "foo", 5556, @@ -404,8 +408,8 @@ func TestGCReconcile(t *testing.T) { Name: "dont gc revisions minimum generations no pin", Objects: []runtime.Object{ cfg("old-revision", "foo", 5556), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5555))), time.Now().Add(-10*time.Minute)), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556))), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5555), nil)), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556), nil)), time.Now().Add(-10*time.Minute)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("old-revision", "foo", 5556, @@ -426,10 +430,10 @@ func TestGCReconcile(t *testing.T) { Objects: []runtime.Object{ cfg("old-revision", "foo", 5556), setRevLastPinned(setRevCreationTimestamp( - makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5554))), + makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5554), nil)), time.Now().Add(-10*time.Minute)), time.Now().Add(-10*time.Minute)), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5555))), time.Now().Add(-10*time.Minute)), - setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556))), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5555), nil)), time.Now().Add(-10*time.Minute)), + setRevCreationTimestamp(makeRevReady(t, resources.MakeRevision(cfg("old-revision", "foo", 5556), nil)), time.Now().Add(-10*time.Minute)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("old-revision", "foo", 5556, @@ -612,7 +616,7 @@ func TestIsRevisionStale(t *testing.T) { serving.ConfigurationGenerationLabelKey: "1", }, Annotations: map[string]string{ - "serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()), + "serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()), }, }, }, @@ -627,7 +631,7 @@ func TestIsRevisionStale(t *testing.T) { serving.ConfigurationGenerationLabelKey: "1", }, Annotations: map[string]string{ - "serving.knative.dev/lastPinned": fmt.Sprintf("%d", curTime.Unix()), + "serving.knative.dev/lastPinned": fmt.Sprintf("%d", curTime.Unix()), }, }, }, @@ -642,7 +646,7 @@ func TestIsRevisionStale(t *testing.T) { serving.ConfigurationGenerationLabelKey: "2", }, Annotations: map[string]string{ - "serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()), + "serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()), }, }, }, @@ -657,7 +661,7 @@ func TestIsRevisionStale(t *testing.T) { serving.ConfigurationGenerationLabelKey: "1", }, Annotations: map[string]string{ - "serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()), + "serving.knative.dev/lastPinned": fmt.Sprintf("%d", staleTime.Unix()), }, }, }, diff --git a/pkg/reconciler/v1alpha1/configuration/resources/revision.go b/pkg/reconciler/v1alpha1/configuration/resources/revision.go index 5f9727112ba9..8ea9107c3576 100644 --- a/pkg/reconciler/v1alpha1/configuration/resources/revision.go +++ b/pkg/reconciler/v1alpha1/configuration/resources/revision.go @@ -26,7 +26,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -func MakeRevision(config *v1alpha1.Configuration) *v1alpha1.Revision { +func MakeRevision(config *v1alpha1.Configuration, buildRef *corev1.ObjectReference) *v1alpha1.Revision { // Start from the ObjectMeta/Spec inlined in the Configuration resources. rev := &v1alpha1.Revision{ ObjectMeta: config.Spec.RevisionTemplate.ObjectMeta, @@ -54,24 +54,8 @@ func MakeRevision(config *v1alpha1.Configuration) *v1alpha1.Revision { // Populate OwnerReferences so that deletes cascade. rev.OwnerReferences = append(rev.OwnerReferences, *kmeta.NewControllerRef(config)) - // Fill in the build name, if specified. - rev.Spec.BuildName = names.Build(config) - // Fill in buildRef if build is involved - rev.Spec.BuildRef = buildRef(config) + rev.Spec.BuildRef = buildRef return rev } - -func buildRef(config *v1alpha1.Configuration) *corev1.ObjectReference { - if config.Spec.Build == nil { - return nil - } - - b := MakeBuild(config) - return &corev1.ObjectReference{ - APIVersion: b.GetAPIVersion(), - Kind: b.GetKind(), - Name: b.GetName(), - } -} diff --git a/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go b/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go index 9bb5e84fa2f6..a1779e49610c 100644 --- a/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go +++ b/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go @@ -31,6 +31,7 @@ func TestMakeRevisions(t *testing.T) { tests := []struct { name string configuration *v1alpha1.Configuration + buildRef *corev1.ObjectReference want *v1alpha1.Revision }{{ name: "no build", @@ -52,8 +53,8 @@ func TestMakeRevisions(t *testing.T) { }, want: &v1alpha1.Revision{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "no", - Name: "build-00012", + Namespace: "no", + Name: "build-00012", Annotations: map[string]string{}, OwnerReferences: []metav1.OwnerReference{{ APIVersion: v1alpha1.SchemeGroupVersion.String(), @@ -63,9 +64,9 @@ func TestMakeRevisions(t *testing.T) { BlockOwnerDeletion: &boolTrue, }}, Labels: map[string]string{ - serving.ConfigurationLabelKey: "build", + serving.ConfigurationLabelKey: "build", serving.ConfigurationGenerationLabelKey: "12", - serving.ServiceLabelKey: "", + serving.ServiceLabelKey: "", }, }, Spec: v1alpha1.RevisionSpec{ @@ -97,10 +98,15 @@ func TestMakeRevisions(t *testing.T) { }, }, }, + buildRef: &corev1.ObjectReference{ + APIVersion: "build.knative.dev/v1alpha1", + Kind: "Build", + Name: "build-00099", + }, want: &v1alpha1.Revision{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "with", - Name: "build-00099", + Namespace: "with", + Name: "build-00099", Annotations: map[string]string{}, OwnerReferences: []metav1.OwnerReference{{ APIVersion: v1alpha1.SchemeGroupVersion.String(), @@ -110,13 +116,12 @@ func TestMakeRevisions(t *testing.T) { BlockOwnerDeletion: &boolTrue, }}, Labels: map[string]string{ - serving.ConfigurationLabelKey: "build", + serving.ConfigurationLabelKey: "build", serving.ConfigurationGenerationLabelKey: "99", - serving.ServiceLabelKey: "", + serving.ServiceLabelKey: "", }, }, Spec: v1alpha1.RevisionSpec{ - BuildName: "build-00099", BuildRef: &corev1.ObjectReference{ APIVersion: "build.knative.dev/v1alpha1", Kind: "Build", @@ -153,8 +158,8 @@ func TestMakeRevisions(t *testing.T) { }, want: &v1alpha1.Revision{ ObjectMeta: metav1.ObjectMeta{ - Namespace: "with", - Name: "labels-00099", + Namespace: "with", + Name: "labels-00099", Annotations: map[string]string{}, OwnerReferences: []metav1.OwnerReference{{ APIVersion: v1alpha1.SchemeGroupVersion.String(), @@ -164,9 +169,9 @@ func TestMakeRevisions(t *testing.T) { BlockOwnerDeletion: &boolTrue, }}, Labels: map[string]string{ - serving.ConfigurationLabelKey: "labels", + serving.ConfigurationLabelKey: "labels", serving.ConfigurationGenerationLabelKey: "99", - serving.ServiceLabelKey: "", + serving.ServiceLabelKey: "", "foo": "bar", "baz": "blah", @@ -214,9 +219,9 @@ func TestMakeRevisions(t *testing.T) { BlockOwnerDeletion: &boolTrue, }}, Labels: map[string]string{ - serving.ConfigurationLabelKey: "annotations", + serving.ConfigurationLabelKey: "annotations", serving.ConfigurationGenerationLabelKey: "99", - serving.ServiceLabelKey: "", + serving.ServiceLabelKey: "", }, Annotations: map[string]string{ "foo": "bar", @@ -233,7 +238,7 @@ func TestMakeRevisions(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := MakeRevision(test.configuration) + got := MakeRevision(test.configuration, test.buildRef) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("MakeRevision (-want, +got) = %v", diff) }