Merge pull request #550 from rikatz/route-bump-generation-on-updates

OCPBUGS-61228: Bump Route generation when spec is updated

Author: openshift-merge-bot[bot]

pkg/route/apiserver/registry/route/strategy.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 
 	authorizationapi "k8s.io/api/authorization/v1"
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation/field"
@@ -66,6 +67,7 @@ func (s routeStrategy) routeValidationOptions() routecommon.RouteValidationOptio
 func (s routeStrategy) PrepareForCreate(ctx context.Context, obj runtime.Object) {
 	route := obj.(*routeapi.Route)
 	route.Status = routeapi.RouteStatus{}
+	route.Generation = 1
 	stripEmptyDestinationCACertificate(route)
 
 	// In kube APIs, disabled fields are stripped from inbound objects.
@@ -96,6 +98,15 @@ func (s routeStrategy) PrepareForUpdate(ctx context.Context, obj, old runtime.Ob
 			route.Spec.TLS.ExternalCertificate = nil
 		}
 	}
+
+	// Any changes to the spec increment the generation number.
+	// Changes to status, or to any metadata field (eg.: labels and annotations)
+	// should not impact on the generation
+	// Preserve existing generation unless the Spec changes.
+	route.Generation = oldRoute.Generation
+	if !apiequality.Semantic.DeepEqual(oldRoute.Spec, route.Spec) {
+		route.Generation = oldRoute.Generation + 1
+	}
 }
 
 func (s routeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {

pkg/route/apiserver/registry/route/strategy_test.go

@@ -120,6 +120,7 @@ func TestEmptyDefaultCACertificate(t *testing.T) {
 					Name:            "myroute",
 					UID:             types.UID("abc"),
 					ResourceVersion: "1",
+					Generation:      1,
 				},
 				Spec: routeapi.RouteSpec{
 					Host: "myhost.com",
@@ -1112,3 +1113,45 @@ func TestExternalCertRemoval(t *testing.T) {
 		}
 	}
 }
+
+func TestRouteGenerationManagement(t *testing.T) {
+	ctx := apirequest.NewContext()
+	strategy := NewStrategy(testAllocator{}, &testSAR{allow: true}, &testSecretGetter{}, true)
+
+	simpleRoute := &routeapi.Route{}
+	strategy.Validate(ctx, simpleRoute)
+	if simpleRoute.Spec.Host != "mygeneratedhost.com" {
+		t.Fatalf("Expected host to be allocated, got %s", simpleRoute.Spec.Host)
+	}
+
+	if simpleRoute.Generation > 0 {
+		t.Fatalf("Expected generation to not be allocated yet, got %d", simpleRoute.Generation)
+	}
+
+	// PrepareForCreate should set a generation 1
+	strategy.PrepareForCreate(ctx, simpleRoute)
+	if simpleRoute.Generation != 1 {
+		t.Fatalf("Expected generation after create to be 1, got %d", simpleRoute.Generation)
+	}
+
+	newRoute := simpleRoute.DeepCopy()
+	// Changing annotations and labels should not bump the generation
+	newRoute.Annotations = map[string]string{
+		"someannotation": "novalue",
+	}
+	newRoute.Labels = map[string]string{
+		"somelabel": "novalue",
+	}
+
+	strategy.PrepareForUpdate(ctx, newRoute, simpleRoute)
+	if newRoute.Generation != 1 {
+		t.Fatalf("Expected generation after metadata update to still be 1, got %d", newRoute.Generation)
+	}
+
+	// Updating the spec should bump the generation
+	newRoute.Spec.Path = "/xpto"
+	strategy.PrepareForUpdate(ctx, newRoute, simpleRoute)
+	if newRoute.Generation != 2 {
+		t.Fatalf("Expected generation after spec update to be 2, got %d", newRoute.Generation)
+	}
+}