Don't create builds unless the BuildSpec changes.

pkg/reconciler/v1alpha1/configuration/configuration.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/client-go/tools/cache"
 
@@ -224,16 +225,32 @@ func (c *Reconciler) createRevision(ctx context.Context, config *v1alpha1.Config
 		build := resources.MakeBuild(config)
 		gvr, _ := meta.UnsafeGuessKindToResource(build.GroupVersionKind())
 
-		created, err := c.DynamicClientSet.Resource(gvr).Namespace(build.GetNamespace()).Create(build)
+		// First, see if a build with this spec already exists.
+		buildHash := build.GetLabels()[serving.BuildHashLabelKey]
+		ul, err := c.DynamicClientSet.Resource(gvr).Namespace(build.GetNamespace()).List(metav1.ListOptions{
+			LabelSelector: fmt.Sprintf("%s=%s", serving.BuildHashLabelKey, buildHash),
+		})
 		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())
+
+		var result *unstructured.Unstructured
+		if len(ul.Items) != 0 {
+			// If one exists, then have the Revision reference it.
+			result = &ul.Items[0]
+		} else {
+			// Otherwise, create a build and reference that.
+			result, err = c.DynamicClientSet.Resource(gvr).Namespace(build.GetNamespace()).Create(build)
+			if err != nil {
+				return nil, err
+			}
+			logger.Infof("Created Build:\n%+v", result.GetName())
+			c.Recorder.Eventf(config, corev1.EventTypeNormal, "Created", "Created Build %q", result.GetName())
+		}
 		buildRef = &corev1.ObjectReference{
-			APIVersion: created.GetAPIVersion(),
-			Kind:       created.GetKind(),
-			Name:       created.GetName(),
+			APIVersion: result.GetAPIVersion(),
+			Kind:       result.GetKind(),
+			Name:       result.GetName(),
 		}
 	}
 

pkg/reconciler/v1alpha1/configuration/configuration_test.go

@@ -107,6 +107,33 @@ func TestReconcile(t *testing.T) {
 				}), "Bogus"),
 		}},
 		Key: "foo/validation-failure",
+	}, {
+		Name: "elide build when a matching one already exists",
+		Objects: []runtime.Object{
+			cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec),
+			// An existing build is reused!
+			resources.MakeBuild(cfgWithBuild("something-else", "foo", 12345, &buildSpec)),
+		},
+		WantCreates: []metav1.Object{
+			resources.MakeRevision(cfgWithBuild("need-rev-and-build", "foo", 99998, &buildSpec), &corev1.ObjectReference{
+				APIVersion: "build.knative.dev/v1alpha1",
+				Kind:       "Build",
+				Name:       "something-else-12345",
+			}),
+		},
+		WantUpdates: []clientgotesting.UpdateActionImpl{{
+			Object: cfgWithBuildAndStatus("need-rev-and-build", "foo", 99998, &buildSpec,
+				v1alpha1.ConfigurationStatus{
+					LatestCreatedRevisionName: "need-rev-and-build-99998",
+					ObservedGeneration:        99998,
+					Conditions: duckv1alpha1.Conditions{{
+						Type:   v1alpha1.ConfigurationConditionReady,
+						Status: corev1.ConditionUnknown,
+					}},
+				},
+			),
+		}},
+		Key: "foo/need-rev-and-build",
 	}, {
 		Name: "create revision matching generation with build",
 		Objects: []runtime.Object{

test/conformance/route_test.go

@@ -42,13 +42,11 @@ func createRouteAndConfig(logger *logging.BaseLogger, clients *test.Clients, nam
 }
 
 func updateConfigWithImage(clients *test.Clients, names test.ResourceNames, imagePaths []string) error {
-	patches := []jsonpatch.JsonPatchOperation{
-		{
-			Operation: "replace",
-			Path:      "/spec/revisionTemplate/spec/container/image",
-			Value:     imagePaths[1],
-		},
-	}
+	patches := []jsonpatch.JsonPatchOperation{{
+		Operation: "replace",
+		Path:      "/spec/revisionTemplate/spec/container/image",
+		Value:     imagePaths[1],
+	}}
 	patchBytes, err := json.Marshal(patches)
 	if err != nil {
 		return err

test/e2e/build_test.go

@@ -18,15 +18,19 @@ limitations under the License.
 package e2e
 
 import (
+	"encoding/json"
 	"net/http"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1"
 	"github.com/knative/pkg/apis/duck"
 	testbuildv1alpha1 "github.com/knative/serving/test/apis/testing/v1alpha1"
+	"github.com/mattbaird/jsonpatch"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
 
 	duckv1alpha1 "github.com/knative/pkg/apis/duck/v1alpha1"
 	pkgTest "github.com/knative/pkg/test"
@@ -41,7 +45,7 @@ func TestBuildSpecAndServe(t *testing.T) {
 	clients := Setup(t)
 
 	// Add test case specific name to its own logger.
-	logger := logging.GetContextLogger("TestBuildAndServe")
+	logger := logging.GetContextLogger("TestBuildSpecAndServe")
 
 	imagePath := test.ImagePath("helloworld")
 
@@ -116,6 +120,30 @@ func TestBuildSpecAndServe(t *testing.T) {
 	} else if cond.Status != corev1.ConditionTrue {
 		t.Fatalf("Build %q was not successful", buildName)
 	}
+
+	// Update the Configuration with an environment variable, which should trigger a new revision
+	// to be created, but without creating a new build.
+	if err := updateConfigWithEnvVars(clients, names, []corev1.EnvVar{{
+		Name:  "FOO",
+		Value: "bar",
+	}}); err != nil {
+		t.Fatalf("Failed to update config with environment variables: %v", err)
+	}
+
+	nextRevName, err := getNextRevisionName(clients, names)
+	if err != nil {
+		t.Fatalf("Error waiting for next revision to be created: %v", err)
+	}
+	names.Revision = nextRevName
+
+	nextRev, err := clients.ServingClient.Revisions.Get(nextRevName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Failed to get latest Revision: %v", err)
+	}
+
+	if diff := cmp.Diff(rev.Spec.BuildRef, nextRev.Spec.BuildRef); diff != "" {
+		t.Fatalf("Unexpected differences in BuildRef: %v", diff)
+	}
 }
 
 func TestBuildAndServe(t *testing.T) {
@@ -189,6 +217,53 @@ func TestBuildAndServe(t *testing.T) {
 	} else if cond.Status != corev1.ConditionTrue {
 		t.Fatalf("Build %q was not successful", buildName)
 	}
+
+	// Update the Configuration with an environment variable, which should trigger a new revision
+	// to be created, but without creating a new build.
+	if err := updateConfigWithEnvVars(clients, names, []corev1.EnvVar{{
+		Name:  "FOO",
+		Value: "bar",
+	}}); err != nil {
+		t.Fatalf("Failed to update config with environment variables: %v", err)
+	}
+
+	nextRevName, err := getNextRevisionName(clients, names)
+	if err != nil {
+		t.Fatalf("Error waiting for next revision to be created: %v", err)
+	}
+	names.Revision = nextRevName
+
+	nextRev, err := clients.ServingClient.Revisions.Get(nextRevName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Failed to get latest Revision: %v", err)
+	}
+
+	if diff := cmp.Diff(rev.Spec.BuildRef, nextRev.Spec.BuildRef); diff != "" {
+		t.Fatalf("Unexpected differences in BuildRef: %v", diff)
+	}
+
+	// Update the Configuration's Build with an annotation, which should trigger the creation
+	// of BOTH a Build and a Revision.
+	if err := updateConfigWithBuildAnnotation(clients, names, map[string]string{
+		"testing.knative.dev/foo": "bar",
+	}); err != nil {
+		t.Fatalf("Failed to update config with environment variables: %v", err)
+	}
+
+	nextRevName, err = getNextRevisionName(clients, names)
+	if err != nil {
+		t.Fatalf("Error waiting for next revision to be created: %v", err)
+	}
+	names.Revision = nextRevName
+
+	nextRev, err = clients.ServingClient.Revisions.Get(nextRevName, metav1.GetOptions{})
+	if err != nil {
+		t.Fatalf("Failed to get latest Revision: %v", err)
+	}
+
+	if diff := cmp.Diff(rev.Spec.BuildRef, nextRev.Spec.BuildRef); diff == "" {
+		t.Fatalf("Got matching BuildRef, wanted different: %#v", rev.Spec.BuildRef)
+	}
 }
 
 func TestBuildFailure(t *testing.T) {
@@ -258,3 +333,49 @@ func TestBuildFailure(t *testing.T) {
 		t.Fatalf("Build %q was not unsuccessful", buildName)
 	}
 }
+
+func getNextRevisionName(clients *test.Clients, names test.ResourceNames) (string, error) {
+	var newRevisionName string
+	err := test.WaitForConfigurationState(clients.ServingClient, names.Config, func(c *v1alpha1.Configuration) (bool, error) {
+		if c.Status.LatestCreatedRevisionName != names.Revision {
+			newRevisionName = c.Status.LatestCreatedRevisionName
+			return true, nil
+		}
+		return false, nil
+	}, "ConfigurationUpdatedWithRevision")
+	return newRevisionName, err
+}
+
+func updateConfigWithEnvVars(clients *test.Clients, names test.ResourceNames, ev []corev1.EnvVar) error {
+	patches := []jsonpatch.JsonPatchOperation{{
+		Operation: "add",
+		Path:      "/spec/revisionTemplate/spec/container/env",
+		Value:     ev,
+	}}
+	patchBytes, err := json.Marshal(patches)
+	if err != nil {
+		return err
+	}
+	_, err = clients.ServingClient.Configs.Patch(names.Config, types.JSONPatchType, patchBytes, "")
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
+func updateConfigWithBuildAnnotation(clients *test.Clients, names test.ResourceNames, ann map[string]string) error {
+	patches := []jsonpatch.JsonPatchOperation{{
+		Operation: "add",
+		Path:      "/spec/build/metadata/annotations",
+		Value:     ann,
+	}}
+	patchBytes, err := json.Marshal(patches)
+	if err != nil {
+		return err
+	}
+	_, err = clients.ServingClient.Configs.Patch(names.Config, types.JSONPatchType, patchBytes, "")
+	if err != nil {
+		return err
+	}
+	return nil
+}