feat: take a VPC identifier from the network

internal/controller/networkcontext_controller.go

@@ -23,12 +23,15 @@ import (
 	"slices"
 	"time"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/handler"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	cloudv1alpha1 "go.datum.net/cloud/api/v1alpha1"
 	"go.datum.net/cloud/internal/identifier"
@@ -44,6 +47,7 @@ type NetworkContextReconciler struct {
 }
 
 // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkcontexts,verbs=get;list;watch
+// +kubebuilder:rbac:groups=cloud.datumapis.com,resources=networkfabricidentities,verbs=get;list;watch
 // +kubebuilder:rbac:groups=networking.datumapis.com,resources=subnets,verbs=get;list;watch
 // +kubebuilder:rbac:groups=cloud.datumapis.com,resources=vpcs,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=cloud.datumapis.com,resources=vpcs/status,verbs=get;update;patch
@@ -82,12 +86,23 @@ func (r *NetworkContextReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return ctrl.Result{}, fmt.Errorf("reconcile VPC %s: %w", vpc.Name, err)
 	}
 
+	// Everything below is conditioned on the VPC not having an identifier yet. A
+	// VPC that already has one keeps it: the VRF device is named from it and the
+	// Route Target derives from it, so renumbering a live VPC would rename its
+	// interface and change its routes under running traffic.
 	if vpc.Status.VPC == "" {
-		allocated, err := r.allocateVPCIdentifier(ctx)
+		identity, found, err := r.fabricIdentity(ctx, &networkContext)
 		if err != nil {
 			return ctrl.Result{}, err
 		}
-		vpc.Status.VPC = allocated
+		if !found {
+			return ctrl.Result{}, r.markAwaitingFabricIdentity(ctx, vpc)
+		}
+		encoded, err := identifier.VPCBase62(uint64(identity))
+		if err != nil {
+			return ctrl.Result{}, fmt.Errorf("encode fabric identity %d for VPC %s: %w", identity, vpc.Name, err)
+		}
+		vpc.Status.VPC = encoded
 	}
 	vpc.Status.ObservedGeneration = vpc.Generation
 	meta.SetStatusCondition(&vpc.Status.Conditions, metav1.Condition{
@@ -104,6 +119,63 @@ func (r *NetworkContextReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 	return ctrl.Result{}, nil
 }
 
+// fabricIdentity reads the identity carried to this cell for the context's
+// network. It is one object per network, named after the network, in the
+// network's namespace. A missing one is an ordinary answer, not a failure.
+//
+// The identity is allocated centrally, once for the whole network, and carried
+// to each cell the network reaches. Deriving the VPC identifier from it is what
+// makes two locations of one network the same network on the fabric; drawing a
+// random value per location, which is what this used to do, made them two.
+//
+// A VPC whose identity has not arrived waits, and there is deliberately nothing
+// else it can do. The identifier is immutable, so a random value taken while
+// waiting is permanent, and a network whose locations then disagree is worse
+// than one with no VPC at all: a NetworkService spanning them binds no VRF and
+// fails every request, including through healthy members. Waiting ends the
+// moment the identity lands. A random draw never ends.
+func (r *NetworkContextReconciler) fabricIdentity(
+	ctx context.Context, networkContext *networkingv1alpha.NetworkContext,
+) (int64, bool, error) {
+	networkName := networkContext.Spec.Network.Name
+	if networkName == "" {
+		return 0, false, nil
+	}
+
+	var identity cloudv1alpha1.NetworkFabricIdentity
+	key := client.ObjectKey{Namespace: networkContext.Namespace, Name: networkName}
+	if err := r.Get(ctx, key, &identity); err != nil {
+		if apierrors.IsNotFound(err) {
+			return 0, false, nil
+		}
+		return 0, false, fmt.Errorf("read the fabric identity for network %q: %w", networkName, err)
+	}
+	if identity.Spec.Identity == 0 {
+		return 0, false, nil
+	}
+	return identity.Spec.Identity, true, nil
+}
+
+// markAwaitingFabricIdentity says out loud that the VPC has no identifier yet
+// and why, so a network stuck waiting on a central allocation is visible as
+// that rather than as a VPC that silently never became ready.
+func (r *NetworkContextReconciler) markAwaitingFabricIdentity(
+	ctx context.Context, vpc *cloudv1alpha1.VPC,
+) error {
+	vpc.Status.ObservedGeneration = vpc.Generation
+	meta.SetStatusCondition(&vpc.Status.Conditions, metav1.Condition{
+		Type:               cloudv1alpha1.ConditionTypeReady,
+		Status:             metav1.ConditionFalse,
+		Reason:             "AwaitingFabricIdentity",
+		Message:            "Waiting for the identity the fabric knows this network by",
+		ObservedGeneration: vpc.Generation,
+	})
+	if err := r.Status().Update(ctx, vpc); err != nil {
+		return fmt.Errorf("update VPC %s status: %w", vpc.Name, err)
+	}
+	return nil
+}
+
 // networksForContext collects the CIDRs IPAM allocated for this location.
 func (r *NetworkContextReconciler) networksForContext(
 	ctx context.Context, networkContext *networkingv1alpha.NetworkContext,
@@ -142,38 +214,36 @@ func subnetRange(subnet *networkingv1alpha.Subnet) (string, int32, bool) {
 	return "", 0, false
 }
 
-// allocateVPCIdentifier draws a random 48-bit identifier not already in use.
-// A single leader-elected controller is the only writer, so a list plus a
-// collision check serializes correctly.
-func (r *NetworkContextReconciler) allocateVPCIdentifier(ctx context.Context) (string, error) {
-	var vpcs cloudv1alpha1.VPCList
-	if err := r.List(ctx, &vpcs); err != nil {
-		return "", fmt.Errorf("list VPCs: %w", err)
-	}
-	used := make(map[string]struct{}, len(vpcs.Items))
-	for _, vpc := range vpcs.Items {
-		if vpc.Status.VPC != "" {
-			used[vpc.Status.VPC] = struct{}{}
-		}
-	}
-
-	for range maxIdentifierAttempts {
-		candidate, err := identifier.RandomVPCBase62()
-		if err != nil {
-			return "", err
-		}
-		if _, taken := used[candidate]; !taken {
-			return candidate, nil
-		}
-	}
-	return "", fmt.Errorf("no unused VPC identifier found after %d attempts", maxIdentifierAttempts)
-}
-
 // SetupWithManager registers the reconciler with the manager.
 func (r *NetworkContextReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&networkingv1alpha.NetworkContext{}).
 		Owns(&cloudv1alpha1.VPC{}).
+		Watches(&cloudv1alpha1.NetworkFabricIdentity{},
+			handler.EnqueueRequestsFromMapFunc(r.contextsForFabricIdentity)).
 		Named("networkcontext").
 		Complete(r)
 }
+
+// contextsForFabricIdentity wakes a network's presences the moment its identity
+// reaches this cell, so a VPC waiting on one takes it immediately instead of
+// sitting out its poll interval.
+func (r *NetworkContextReconciler) contextsForFabricIdentity(
+	ctx context.Context, object client.Object,
+) []reconcile.Request {
+	var contexts networkingv1alpha.NetworkContextList
+	if err := r.List(ctx, &contexts, client.InNamespace(object.GetNamespace())); err != nil {
+		return nil
+	}
+
+	requests := make([]reconcile.Request, 0, len(contexts.Items))
+	for i := range contexts.Items {
+		if contexts.Items[i].Spec.Network.Name != object.GetName() {
+			continue
+		}
+		requests = append(requests, reconcile.Request{
+			NamespacedName: client.ObjectKeyFromObject(&contexts.Items[i]),
+		})
+	}
+	return requests
+}

internal/controller/networkcontext_controller_test.go

@@ -18,8 +18,18 @@ along with this program.  If not, see <https://www.gnu.org/licenses/>.
 package controller
 
 import (
+	"context"
 	"testing"
 
+	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+
+	cloudv1alpha1 "go.datum.net/cloud/api/v1alpha1"
 	networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha"
 )
 
@@ -50,3 +60,185 @@ func TestSubnetRangePrefersStatusAndFallsBackToSpec(t *testing.T) {
 		t.Fatal("an unallocated subnet should yield nothing")
 	}
 }
+
+const (
+	vpcTestNamespace = "ns-project"
+	vpcTestNetwork   = "prod"
+	vpcTestLocation  = "us-central-1"
+
+	// The identity the fabric knows the test network by, and the identifier a
+	// VPC derives from it.
+	vpcTestIdentity   = 16
+	vpcTestIdentifier = "g"
+)
+
+type vpcFixture struct {
+	t          *testing.T
+	ctx        context.Context
+	client     client.Client
+	reconciler *NetworkContextReconciler
+}
+
+// newVPCFixture stands up one network present at one location, with the subnet
+// that gives the VPC an address space to be created for.
+func newVPCFixture(t *testing.T, objects ...client.Object) *vpcFixture {
+	t.Helper()
+
+	scheme := runtime.NewScheme()
+	if err := networkingv1alpha.AddToScheme(scheme); err != nil {
+		t.Fatalf("build the networking scheme: %v", err)
+	}
+	if err := cloudv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatalf("build the cloud scheme: %v", err)
+	}
+
+	presence := &networkingv1alpha.NetworkContext{}
+	presence.Namespace = vpcTestNamespace
+	presence.Name = vpcTestNetwork + "-" + vpcTestLocation
+	presence.Spec.Network = networkingv1alpha.LocalNetworkRef{Name: vpcTestNetwork}
+	presence.Spec.Location = networkingv1alpha.LocationReference{Name: vpcTestLocation}
+
+	subnet := &networkingv1alpha.Subnet{}
+	subnet.Namespace = vpcTestNamespace
+	subnet.Name = presence.Name
+	subnet.Spec.NetworkContext = networkingv1alpha.LocalNetworkContextRef{Name: presence.Name}
+	subnet.Spec.StartAddress = "fd00::"
+	subnet.Spec.PrefixLength = 48
+
+	all := append([]client.Object{presence, subnet}, objects...)
+	fakeClient := fake.NewClientBuilder().WithScheme(scheme).
+		WithStatusSubresource(&cloudv1alpha1.VPC{}).
+		WithObjects(all...).Build()
+
+	return &vpcFixture{
+		t:          t,
+		ctx:        context.Background(),
+		client:     fakeClient,
+		reconciler: &NetworkContextReconciler{Client: fakeClient, Scheme: scheme},
+	}
+}
+
+func (f *vpcFixture) reconcile() ctrl.Result {
+	f.t.Helper()
+	result, err := f.reconciler.Reconcile(f.ctx, ctrl.Request{NamespacedName: types.NamespacedName{
+		Namespace: vpcTestNamespace,
+		Name:      vpcTestNetwork + "-" + vpcTestLocation,
+	}})
+	if err != nil {
+		f.t.Fatalf("reconcile: %v", err)
+	}
+	return result
+}
+
+func (f *vpcFixture) vpc() *cloudv1alpha1.VPC {
+	f.t.Helper()
+	vpc := &cloudv1alpha1.VPC{}
+	key := types.NamespacedName{Namespace: vpcTestNamespace, Name: vpcTestNetwork + "-" + vpcTestLocation}
+	if err := f.client.Get(f.ctx, key, vpc); err != nil {
+		f.t.Fatalf("read the VPC: %v", err)
+	}
+	return vpc
+}
+
+func fabricIdentityObject() *cloudv1alpha1.NetworkFabricIdentity {
+	object := &cloudv1alpha1.NetworkFabricIdentity{}
+	object.Namespace = vpcTestNamespace
+	object.Name = vpcTestNetwork
+	object.Spec.Identity = vpcTestIdentity
+	object.Spec.NetworkRef = cloudv1alpha1.NetworkFabricIdentityNetworkRef{Name: vpcTestNetwork}
+	return object
+}
+
+// The whole point: every location of one network reaches the same identifier,
+// because every one of them reads the same centrally allocated identity.
+func TestVPCTakesItsIdentifierFromTheNetworksFabricIdentity(t *testing.T) {
+	fixture := newVPCFixture(t, fabricIdentityObject())
+
+	fixture.reconcile()
+
+	vpc := fixture.vpc()
+	if vpc.Status.VPC != vpcTestIdentifier {
+		t.Fatalf("VPC identifier: got %q, want %q", vpc.Status.VPC, vpcTestIdentifier)
+	}
+	if condition := meta.FindStatusCondition(vpc.Status.Conditions, cloudv1alpha1.ConditionTypeReady); condition == nil ||
+		condition.Status != metav1.ConditionTrue {
+		t.Fatalf("a VPC with an identifier should be ready, got %+v", condition)
+	}
+}
+
+// A network whose identity has not reached this cell yet waits, and writes
+// nothing. The identifier is immutable, so a value taken while waiting is
+// permanent, and locations that disagree bind no VRF at all.
+func TestVPCWaitsForAnIdentityThatHasNotArrived(t *testing.T) {
+	fixture := newVPCFixture(t)
+
+	if result := fixture.reconcile(); result.RequeueAfter != 0 {
+		t.Fatalf("the identity's arrival is the trigger, not a poll, got %v", result)
+	}
+
+	vpc := fixture.vpc()
+	if vpc.Status.VPC != "" {
+		t.Fatalf("no identifier should be written while waiting, got %q", vpc.Status.VPC)
+	}
+	condition := meta.FindStatusCondition(vpc.Status.Conditions, cloudv1alpha1.ConditionTypeReady)
+	if condition == nil || condition.Status != metav1.ConditionFalse ||
+		condition.Reason != "AwaitingFabricIdentity" {
+		t.Fatalf("the wait should be visible on the VPC, got %+v", condition)
+	}
+}
+
+// The identity landing is what ends the wait, and the identifier that follows
+// is the derived one rather than a random draw.
+func TestVPCTakesTheIdentityOnceItArrives(t *testing.T) {
+	fixture := newVPCFixture(t)
+
+	fixture.reconcile()
+	if got := fixture.vpc().Status.VPC; got != "" {
+		t.Fatalf("no identifier should be written while waiting, got %q", got)
+	}
+
+	if err := fixture.client.Create(fixture.ctx, fabricIdentityObject()); err != nil {
+		t.Fatalf("publish the identity: %v", err)
+	}
+	fixture.reconcile()
+
+	if got := fixture.vpc().Status.VPC; got != vpcTestIdentifier {
+		t.Fatalf("VPC identifier: got %q, want %q", got, vpcTestIdentifier)
+	}
+}
+
+// Renumbering a live VPC would rename its VRF device and change its Route
+// Target under running traffic, so an identifier already written stays written
+// even when it disagrees with the identity that later arrived.
+func TestVPCKeepsAnIdentifierItAlreadyHas(t *testing.T) {
+	existing := &cloudv1alpha1.VPC{}
+	existing.Namespace = vpcTestNamespace
+	existing.Name = vpcTestNetwork + "-" + vpcTestLocation
+	existing.Status.VPC = "R2POk4jT"
+
+	fixture := newVPCFixture(t, existing, fabricIdentityObject())
+
+	fixture.reconcile()
+
+	if got := fixture.vpc().Status.VPC; got != "R2POk4jT" {
+		t.Fatalf("an allocated VPC must keep its identifier, got %q", got)
+	}
+}
+
+// An identity reaching the cell has to wake the presences waiting on it, or a
+// VPC would sit out its poll interval for a value already there.
+func TestFabricIdentityWakesTheNetworksPresences(t *testing.T) {
+	fixture := newVPCFixture(t)
+
+	requests := fixture.reconciler.contextsForFabricIdentity(fixture.ctx, fabricIdentityObject())
+
+	if len(requests) != 1 || requests[0].Name != vpcTestNetwork+"-"+vpcTestLocation {
+		t.Fatalf("the network's presence should be enqueued, got %v", requests)
+	}
+
+	other := fabricIdentityObject()
+	other.Name = "unrelated"
+	if requests := fixture.reconciler.contextsForFabricIdentity(fixture.ctx, other); len(requests) != 0 {
+		t.Fatalf("another network's identity should enqueue nothing, got %v", requests)
+	}
+}