From 2ca1e2e500c84c30099ed32731f2d5f1e8b39909 Mon Sep 17 00:00:00 2001 From: Dave Protasowski Date: Fri, 3 Aug 2018 08:55:22 -0400 Subject: [PATCH] No need to pass a build name around when reconciling a configuration --- .../v1alpha1/configuration/configuration.go | 4 +-- .../configuration/configuration_test.go | 25 ++++++++----------- .../configuration/resources/revision.go | 4 +-- .../configuration/resources/revision_test.go | 3 +-- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/pkg/reconciler/v1alpha1/configuration/configuration.go b/pkg/reconciler/v1alpha1/configuration/configuration.go index 78f727e69d63..c6360398133c 100644 --- a/pkg/reconciler/v1alpha1/configuration/configuration.go +++ b/pkg/reconciler/v1alpha1/configuration/configuration.go @@ -201,7 +201,6 @@ func (c *Reconciler) reconcile(ctx context.Context, config *v1alpha1.Configurati func (c *Reconciler) createRevision(config *v1alpha1.Configuration, revName string) (*v1alpha1.Revision, error) { logger := loggerWithConfigInfo(c.Logger, config.Namespace, config.Name) - buildName := "" if config.Spec.Build != nil { // TODO(mattmoor): Determine whether we reuse the previous build. build := resources.MakeBuild(config) @@ -211,10 +210,9 @@ func (c *Reconciler) createRevision(config *v1alpha1.Configuration, revName stri } logger.Infof("Created Build:\n%+v", created.Name) c.Recorder.Eventf(config, corev1.EventTypeNormal, "Created", "Created Build %q", created.Name) - buildName = created.Name } - rev := resources.MakeRevision(config, buildName) + rev := resources.MakeRevision(config) 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 7d7f62c3bf5b..a7dc91916190 100644 --- a/pkg/reconciler/v1alpha1/configuration/configuration_test.go +++ b/pkg/reconciler/v1alpha1/configuration/configuration_test.go @@ -48,8 +48,6 @@ var ( } ) -const noBuildName = "" - // This is heavily based on the way the OpenShift Ingress controller tests its reconciliation method. func TestReconcile(t *testing.T) { table := TableTest{{ @@ -64,7 +62,7 @@ func TestReconcile(t *testing.T) { cfg("no-revisions-yet", "foo", 1234), }, WantCreates: []metav1.Object{ - resources.MakeRevision(cfg("no-revisions-yet", "foo", 1234), noBuildName), + resources.MakeRevision(cfg("no-revisions-yet", "foo", 1234)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("no-revisions-yet", "foo", 1234, @@ -86,7 +84,7 @@ func TestReconcile(t *testing.T) { setConcurrencyModel(cfg("validation-failure", "foo", 1234), "Bogus"), }, WantCreates: []metav1.Object{ - setRevConcurrencyModel(resources.MakeRevision(cfg("validation-failure", "foo", 1234), noBuildName), "Bogus"), + setRevConcurrencyModel(resources.MakeRevision(cfg("validation-failure", "foo", 1234)), "Bogus"), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: setConcurrencyModel(cfgWithStatus("validation-failure", "foo", 1234, @@ -107,8 +105,7 @@ 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.MakeBuild(cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec)).Name), + resources.MakeRevision(cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithBuildAndStatus("need-rev-and-build", "foo", 99998, &buildSpec, @@ -127,7 +124,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile revision matching generation (ready: unknown)", Objects: []runtime.Object{ cfg("matching-revision-not-done", "foo", 5432), - resources.MakeRevision(cfg("matching-revision-not-done", "foo", 5432), noBuildName), + resources.MakeRevision(cfg("matching-revision-not-done", "foo", 5432)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("matching-revision-not-done", "foo", 5432, @@ -146,7 +143,7 @@ func TestReconcile(t *testing.T) { Name: "reconcile revision matching generation (ready: true)", Objects: []runtime.Object{ cfg("matching-revision-done", "foo", 5555), - makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done", "foo", 5555), noBuildName)), + makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done", "foo", 5555))), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("matching-revision-done", "foo", 5555, @@ -176,14 +173,14 @@ func TestReconcile(t *testing.T) { }}, }, ), - makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done-idempotent", "foo", 5566), noBuildName)), + makeRevReady(t, resources.MakeRevision(cfg("matching-revision-done-idempotent", "foo", 5566))), }, Key: "foo/matching-revision-done-idempotent", }, { Name: "reconcile revision matching generation (ready: false)", Objects: []runtime.Object{ cfg("matching-revision-failed", "foo", 5555), - makeRevFailed(resources.MakeRevision(cfg("matching-revision-failed", "foo", 5555), noBuildName)), + makeRevFailed(resources.MakeRevision(cfg("matching-revision-failed", "foo", 5555))), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("matching-revision-failed", "foo", 5555, @@ -204,7 +201,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), noBuildName), + makeRevStatus(resources.MakeRevision(cfg("bad-condition", "foo", 5555)), v1alpha1.RevisionStatus{ Conditions: []v1alpha1.RevisionCondition{{ Type: v1alpha1.RevisionConditionReady, @@ -265,7 +262,7 @@ func TestReconcile(t *testing.T) { cfg("create-revision-failure", "foo", 99998), }, WantCreates: []metav1.Object{ - resources.MakeRevision(cfg("create-revision-failure", "foo", 99998), noBuildName), + resources.MakeRevision(cfg("create-revision-failure", "foo", 99998)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("create-revision-failure", "foo", 99998, @@ -291,7 +288,7 @@ func TestReconcile(t *testing.T) { cfg("update-config-failure", "foo", 1234), }, WantCreates: []metav1.Object{ - resources.MakeRevision(cfg("update-config-failure", "foo", 1234), noBuildName), + resources.MakeRevision(cfg("update-config-failure", "foo", 1234)), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("update-config-failure", "foo", 1234, @@ -321,7 +318,7 @@ func TestReconcile(t *testing.T) { }}, }, ), - makeRevReady(t, resources.MakeRevision(cfg("revision-recovers", "foo", 1337), noBuildName)), + makeRevReady(t, resources.MakeRevision(cfg("revision-recovers", "foo", 1337))), }, WantUpdates: []clientgotesting.UpdateActionImpl{{ Object: cfgWithStatus("revision-recovers", "foo", 1337, diff --git a/pkg/reconciler/v1alpha1/configuration/resources/revision.go b/pkg/reconciler/v1alpha1/configuration/resources/revision.go index 659fc37f17bd..ae17ff1cf60d 100644 --- a/pkg/reconciler/v1alpha1/configuration/resources/revision.go +++ b/pkg/reconciler/v1alpha1/configuration/resources/revision.go @@ -25,7 +25,7 @@ import ( "github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources/names" ) -func MakeRevision(config *v1alpha1.Configuration, buildName string) *v1alpha1.Revision { +func MakeRevision(config *v1alpha1.Configuration) *v1alpha1.Revision { // Start from the ObjectMeta/Spec inlined in the Configuration resources. rev := &v1alpha1.Revision{ ObjectMeta: config.Spec.RevisionTemplate.ObjectMeta, @@ -51,7 +51,7 @@ func MakeRevision(config *v1alpha1.Configuration, buildName string) *v1alpha1.Re rev.OwnerReferences = append(rev.OwnerReferences, *reconciler.NewControllerRef(config)) // Fill in the build name, if specified. - rev.Spec.BuildName = buildName + rev.Spec.BuildName = names.Build(config) return rev } diff --git a/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go b/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go index d0894b3b9e8f..c60b251e8820 100644 --- a/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go +++ b/pkg/reconciler/v1alpha1/configuration/resources/revision_test.go @@ -25,7 +25,6 @@ import ( buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1" "github.com/knative/serving/pkg/apis/serving" "github.com/knative/serving/pkg/apis/serving/v1alpha1" - "github.com/knative/serving/pkg/reconciler/v1alpha1/configuration/resources/names" ) func TestRevisions(t *testing.T) { @@ -227,7 +226,7 @@ func TestRevisions(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - got := MakeRevision(test.configuration, names.Build(test.configuration)) + got := MakeRevision(test.configuration) if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("MakeRevision (-want, +got) = %v", diff) }