diff --git a/internal/controller/networkcontext_controller.go b/internal/controller/networkcontext_controller.go index bddb95f..77bed0b 100644 --- a/internal/controller/networkcontext_controller.go +++ b/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 +} diff --git a/internal/controller/networkcontext_controller_test.go b/internal/controller/networkcontext_controller_test.go index 5734222..36e24e2 100644 --- a/internal/controller/networkcontext_controller_test.go +++ b/internal/controller/networkcontext_controller_test.go @@ -18,8 +18,18 @@ along with this program. If not, see . 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) + } +} diff --git a/internal/identifier/identifier.go b/internal/identifier/identifier.go index 5360aa8..657d22d 100644 --- a/internal/identifier/identifier.go +++ b/internal/identifier/identifier.go @@ -60,11 +60,6 @@ func Random(max uint64) (string, error) { return Hex(n.Uint64()+1, max) } -// RandomVPC returns a random 48-bit VPC identifier in hex. -func RandomVPC() (string, error) { - return Random(MaxVPC) -} - // RandomVPCAttachment returns a random 16-bit attachment identifier in hex. func RandomVPCAttachment() (string, error) { return Random(MaxVPCAttachment) @@ -80,9 +75,10 @@ func Base62ToHex(value string) (string, error) { return baseconv.Convert(value, baseconv.Digits62, baseconv.DigitsHex) } -// RandomVPCBase62 returns a random VPC identifier in base62. -func RandomVPCBase62() (string, error) { - hex, err := RandomVPC() +// VPCBase62 renders a VPC identifier in base62, with the reserved-value guards +// and the width that keeps a kernel interface name inside fifteen characters. +func VPCBase62(value uint64) (string, error) { + hex, err := Hex(value, MaxVPC) if err != nil { return "", err } diff --git a/internal/identifier/identifier_test.go b/internal/identifier/identifier_test.go index 938633b..5e7cda3 100644 --- a/internal/identifier/identifier_test.go +++ b/internal/identifier/identifier_test.go @@ -29,14 +29,6 @@ func TestHexRejectsReservedValues(t *testing.T) { func TestRandomIdentifiersFitTheirInterfaceNameSegment(t *testing.T) { for range 200 { - vpc, err := RandomVPCBase62() - if err != nil { - t.Fatalf("RandomVPCBase62: %v", err) - } - if len(vpc) > 9 { - t.Errorf("VPC identifier %q exceeds nine base62 characters", vpc) - } - attachment, err := RandomVPCAttachmentBase62() if err != nil { t.Fatalf("RandomVPCAttachmentBase62: %v", err) @@ -48,9 +40,9 @@ func TestRandomIdentifiersFitTheirInterfaceNameSegment(t *testing.T) { } func TestBase62RoundTrip(t *testing.T) { - hex, err := RandomVPC() + hex, err := Random(MaxVPC) if err != nil { - t.Fatalf("RandomVPC: %v", err) + t.Fatalf("Random(MaxVPC): %v", err) } base62, err := HexToBase62(hex) if err != nil { @@ -73,3 +65,23 @@ func trimLeadingZeros(value string) string { } return "0" } + +func TestVPCBase62RejectsReservedValuesAndFitsItsSegment(t *testing.T) { + for _, value := range []uint64{0, MaxVPC, MaxVPC + 1} { + if _, err := VPCBase62(value); err == nil { + t.Errorf("VPCBase62(%d): got no error, want one", value) + } + } + + // The widest fabric identity is 32 bits, well inside the 48 the VPC + // identifier holds, so nothing an identity carries can overflow the segment. + for _, value := range []uint64{1, 16, 1 << 31, 1<<32 - 1} { + encoded, err := VPCBase62(value) + if err != nil { + t.Fatalf("VPCBase62(%d): %v", value, err) + } + if len(encoded) > 9 { + t.Errorf("VPCBase62(%d) = %q exceeds nine base62 characters", value, encoded) + } + } +}