From e05a830d1685afb2d7d998f59662853c556f6fcf Mon Sep 17 00:00:00 2001 From: Scot Wells Date: Fri, 14 Aug 2026 13:02:05 -0500 Subject: [PATCH] feat: ask for a network's presence where a workload runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workload that lands in a location needs its network to exist there before an instance can get an address. Compute now says so: once a cell reports the location it serves a deployment from, the federator writes a NetworkBinding next to the hub WorkloadDeployment. NSO folds every binding for the same network and location into one shared NetworkContext and propagates it to the cells serving that location, where the interface claim path reads it. The binding is one per WorkloadDeployment and owned by the hub copy, so the hub releases it with its owner and nothing in compute counts the other consumers of a shared presence. Compute stamps only the network and location labels, via a merge patch computed from the live object, leaving the network UID NSO records on the binding untouched. What NSO refuses — an unresolvable project, a missing network, a location the project cannot use — is reported on the deployment's Available condition and acted on nowhere else. Instances stay gated on their own interface claims: a binding waiting on a context must never take a running deployment apart. Co-Authored-By: Claude Opus 5 --- internal/controller/testing_helpers_test.go | 4 +- .../workloaddeployment_federator.go | 39 +- .../workloaddeployment_network_binding.go | 338 +++++++++++ ...workloaddeployment_network_binding_test.go | 568 ++++++++++++++++++ 4 files changed, 943 insertions(+), 6 deletions(-) create mode 100644 internal/controller/workloaddeployment_network_binding.go create mode 100644 internal/controller/workloaddeployment_network_binding_test.go diff --git a/internal/controller/testing_helpers_test.go b/internal/controller/testing_helpers_test.go index 52b3ca36..303161ef 100644 --- a/internal/controller/testing_helpers_test.go +++ b/internal/controller/testing_helpers_test.go @@ -19,6 +19,7 @@ import ( karmadapolicyv1alpha1 "github.com/karmada-io/api/policy/v1alpha1" computev1alpha "go.datum.net/compute/api/v1alpha" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" ) // ─── Scheme helpers ─────────────────────────────────────────────────────────── @@ -34,11 +35,12 @@ func newProjectScheme() *runtime.Scheme { } // newKarmadaScheme builds a runtime.Scheme with the types needed by the Karmada -// API server (corev1 + compute + karmada policy). +// API server (corev1 + compute + networking + karmada policy). func newKarmadaScheme() *runtime.Scheme { s := runtime.NewScheme() _ = corev1.AddToScheme(s) _ = computev1alpha.AddToScheme(s) + _ = networkingv1alpha.AddToScheme(s) _ = karmadapolicyv1alpha1.Install(s) return s } diff --git a/internal/controller/workloaddeployment_federator.go b/internal/controller/workloaddeployment_federator.go index ac384497..95892492 100644 --- a/internal/controller/workloaddeployment_federator.go +++ b/internal/controller/workloaddeployment_federator.go @@ -158,7 +158,8 @@ func (r *WorkloadDeploymentFederator) Reconcile(ctx context.Context, req mcrecon // Upsert the WorkloadDeployment in the downstream control plane via the // strategy client so any future Create calls also go through // ensureDownstreamNamespace automatically. - if err := r.upsertDownstreamDeployment(ctx, strategy.GetClient(), &deployment, downstreamNS); err != nil { + hubDeployment, err := r.upsertDownstreamDeployment(ctx, strategy.GetClient(), &deployment, downstreamNS) + if err != nil { return ctrl.Result{}, err } @@ -166,7 +167,15 @@ func (r *WorkloadDeploymentFederator) Reconcile(ctx context.Context, req mcrecon return ctrl.Result{}, err } - if err := r.syncStatusFromDownstream(ctx, cl.GetClient(), &deployment, downstreamNS); err != nil { + // Ask for the deployment's network to be present where it runs. This follows + // the hub deployment because the location it is placed in is only known from + // the status the cell aggregates back onto it. + binding, err := r.ensureNetworkBinding(ctx, hubDeployment) + if err != nil { + return ctrl.Result{}, err + } + + if err := r.syncStatusFromDownstream(ctx, cl.GetClient(), &deployment, downstreamNS, binding); err != nil { return ctrl.Result{}, err } @@ -276,12 +285,16 @@ func (r *WorkloadDeploymentFederator) ensureDownstreamNamespace(ctx context.Cont // upsertDownstreamDeployment creates or updates the WorkloadDeployment in the // downstream namespace via the provided client (expected to be strategy.GetClient() // so the downstream namespace is created with upstream tracking labels). +// +// It returns the downstream object as it now stands, which is what anything +// hanging off the hub deployment — its UID for an owner reference, its +// aggregated status for the location it landed in — has to be built from. func (r *WorkloadDeploymentFederator) upsertDownstreamDeployment( ctx context.Context, downstreamClient client.Client, deployment *computev1alpha.WorkloadDeployment, downstreamNS string, -) error { +) (*computev1alpha.WorkloadDeployment, error) { kd := &computev1alpha.WorkloadDeployment{ ObjectMeta: metav1.ObjectMeta{ Name: deployment.Name, @@ -328,11 +341,11 @@ func (r *WorkloadDeploymentFederator) upsertDownstreamDeployment( return nil }) if err != nil { - return fmt.Errorf("failed to upsert downstream deployment %s/%s: %w", downstreamNS, deployment.Name, err) + return nil, fmt.Errorf("failed to upsert downstream deployment %s/%s: %w", downstreamNS, deployment.Name, err) } log.FromContext(ctx).Info("upserted downstream deployment", "result", result, "downstreamNamespace", downstreamNS) - return nil + return kd, nil } // ensurePropagationPolicy creates or updates a PropagationPolicy in the downstream @@ -435,6 +448,7 @@ func (r *WorkloadDeploymentFederator) syncStatusFromDownstream( projectClient client.Client, deployment *computev1alpha.WorkloadDeployment, downstreamNS string, + binding *networkingv1alpha.NetworkBinding, ) error { var kd computev1alpha.WorkloadDeployment if err := r.FederationClient.Get(ctx, types.NamespacedName{ @@ -454,6 +468,7 @@ func (r *WorkloadDeploymentFederator) syncStatusFromDownstream( if resolverCond := apimeta.FindStatusCondition(deployment.Status.Conditions, computev1alpha.ReferencedDataReady); resolverCond != nil { apimeta.SetStatusCondition(&merged.Conditions, *resolverCond) } + applyNetworkBindingRefusal(merged, binding, deployment.Generation) if equality.Semantic.DeepEqual(deployment.Status, *merged) { return nil @@ -472,6 +487,7 @@ func (r *WorkloadDeploymentFederator) syncStatusFromDownstream( if resolverCond := apimeta.FindStatusCondition(deployment.Status.Conditions, computev1alpha.ReferencedDataReady); resolverCond != nil { apimeta.SetStatusCondition(&merged.Conditions, *resolverCond) } + applyNetworkBindingRefusal(merged, binding, deployment.Generation) if equality.Semantic.DeepEqual(deployment.Status, *merged) { return nil } @@ -575,6 +591,19 @@ func (r *WorkloadDeploymentFederator) SetupWithManager(mgr mcmanager.Manager) er &computev1alpha.WorkloadDeployment{}, preserveClusterName, )) + + // Watch the NetworkBindings this controller writes, so what NSO says + // about a declared presence reaches the deployment's own status without + // waiting for a resync, and so a recreate after a location change is + // driven by the old binding disappearing. + preserveOnBinding := func(_ multicluster.ClusterName, _ cluster.Cluster) handler.TypedEventHandler[*networkingv1alpha.NetworkBinding, mcreconcile.Request] { + return mchandler.TypedEnqueueRequestsFromMapFuncWithClusterPreservation(r.mapNetworkBindingToRequest) + } + b = b.WatchesRawSource(milosource.MustNewClusterSource( + r.FederationCluster, + &networkingv1alpha.NetworkBinding{}, + preserveOnBinding, + )) } return b.Complete(r) diff --git a/internal/controller/workloaddeployment_network_binding.go b/internal/controller/workloaddeployment_network_binding.go new file mode 100644 index 00000000..ea132afd --- /dev/null +++ b/internal/controller/workloaddeployment_network_binding.go @@ -0,0 +1,338 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "fmt" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/utils/ptr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/multicluster-runtime/pkg/multicluster" + mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" + + computev1alpha "go.datum.net/compute/api/v1alpha" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + "go.miloapis.com/milo/pkg/downstreamclient" +) + +// ensureNetworkBinding declares that the deployment's network is needed in the +// location the deployment is running in, by writing a NetworkBinding next to the +// hub WorkloadDeployment. NSO's presence controller folds every binding for the +// same network and location into one shared NetworkContext and propagates it to +// the cells serving that location, where the interface claim path reads it. +// +// The binding is one per WorkloadDeployment, named after it, rather than one per +// (network, location) pair. A pair-named object would be written by every +// deployment sharing the pair, making each of them a partial owner of something +// shared and leaving nobody able to remove it. Naming it after the deployment +// keeps it wholly compute's: it is created when the deployment lands somewhere, +// and the hub garbage-collects it with its owner. +// +// It returns the binding so the caller can report what NSO says about it. A nil +// binding means there is nothing to declare yet, not that anything failed. +func (r *WorkloadDeploymentFederator) ensureNetworkBinding( + ctx context.Context, + hubDeployment *computev1alpha.WorkloadDeployment, +) (*networkingv1alpha.NetworkBinding, error) { + // The location is written by the cell that serves the deployment and reaches + // the hub through Karmada status aggregation, so it is absent for as long as + // nothing has placed the deployment. There is no presence to ask for until + // then, and a binding without a location cannot be created at all. + if hubDeployment.Status.Location == nil || hubDeployment.Status.Location.Name == "" { + return nil, nil + } + + network, ok := deploymentNetworkRef(hubDeployment) + if !ok { + return nil, nil + } + location := *hubDeployment.Status.Location + + key := client.ObjectKey{Namespace: hubDeployment.Namespace, Name: hubDeployment.Name} + var existing networkingv1alpha.NetworkBinding + err := r.FederationClient.Get(ctx, key, &existing) + switch { + case apierrors.IsNotFound(err): + return r.createNetworkBinding(ctx, hubDeployment, network, location) + case err != nil: + return nil, fmt.Errorf("failed reading network binding %s/%s: %w", key.Namespace, key.Name, err) + } + + // Everything below either rewrites or removes the object under this name. A + // binding this deployment does not own belongs to another consumer of the + // same presence, and taking it over would delete a declaration compute never + // made. + if !metav1.IsControlledBy(&existing, hubDeployment) { + return nil, fmt.Errorf("network binding %s/%s is not controlled by this workload deployment", + key.Namespace, key.Name) + } + + if !existing.DeletionTimestamp.IsZero() { + // A recreate is already in flight. The binding watch reconciles this + // deployment again once the object is gone. + return &existing, nil + } + + // spec.network and spec.location are immutable on a NetworkBinding: a + // deployment whose network was edited, or which a cell now serves from a + // different location, is asking for a different presence. Delete the old + // declaration and let the next pass make the new one, so the crossing is a + // visible replacement rather than a silently rejected update. + if !equality.Semantic.DeepEqual(existing.Spec.Network, network) || + !equality.Semantic.DeepEqual(existing.Spec.Location, location) { + log.FromContext(ctx).Info("network binding declares a different presence, recreating", + "binding", key.String(), + "network", network.Name, "location", location.Name) + if err := r.FederationClient.Delete(ctx, &existing, client.Preconditions{UID: &existing.UID}); err != nil { + return nil, client.IgnoreNotFound(fmt.Errorf("failed deleting diverged network binding %s: %w", key, err)) + } + return nil, nil + } + + if err := r.reconcileNetworkBindingLabels(ctx, &existing, network, location); err != nil { + return nil, err + } + return &existing, nil +} + +// createNetworkBinding writes the binding for a deployment that does not have +// one. The owner reference is a real one to the hub WorkloadDeployment in the +// same namespace, which is what releases the binding when the deployment goes +// away — no finalizer of compute's is involved, and nothing counts the other +// consumers of the presence. +// +// BlockOwnerDeletion is deliberately not set: it would require update access to +// the owner's finalizers subresource wherever OwnerReferencesPermissionEnforcement +// is enabled, and nothing here needs the deployment's own removal held up. +func (r *WorkloadDeploymentFederator) createNetworkBinding( + ctx context.Context, + hubDeployment *computev1alpha.WorkloadDeployment, + network networkingv1alpha.NetworkRef, + location networkingv1alpha.LocationReference, +) (*networkingv1alpha.NetworkBinding, error) { + binding := &networkingv1alpha.NetworkBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: hubDeployment.Name, + Namespace: hubDeployment.Namespace, + Labels: networkBindingLabels(network, location), + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: computev1alpha.GroupVersion.String(), + Kind: kindWorkloadDeployment, + Name: hubDeployment.Name, + UID: hubDeployment.UID, + Controller: ptr.To(true), + }}, + }, + Spec: networkingv1alpha.NetworkBindingSpec{ + Network: network, + Location: location, + Consumer: &networkingv1alpha.NetworkBindingConsumer{ + APIGroup: computev1alpha.GroupVersion.Group, + Kind: kindWorkloadDeployment, + Name: hubDeployment.Name, + }, + }, + } + + if err := r.FederationClient.Create(ctx, binding); err != nil { + if apierrors.IsAlreadyExists(err) { + // The previous declaration is still going away. The next pass, or the + // binding watch, picks it up. + return nil, nil + } + return nil, fmt.Errorf("failed creating network binding %s/%s: %w", + binding.Namespace, binding.Name, err) + } + + log.FromContext(ctx).Info("created network binding", + "binding", client.ObjectKeyFromObject(binding).String(), + "network", network.Name, "location", location.Name) + return binding, nil +} + +// reconcileNetworkBindingLabels brings the labels compute stamps up to date +// without touching anything else on the object. +// +// NSO's presence controller patches the network's UID onto the binding, and that +// label is what its garbage collection keys on. A CreateOrUpdate that assigns +// the whole label map would strip it on every pass, which both breaks NSO's +// bookkeeping and puts the two controllers in a write loop against each other. +// The merge patch below is computed from a copy of the live object and only ever +// adds keys, so a key compute does not set is not in the patch at all. +func (r *WorkloadDeploymentFederator) reconcileNetworkBindingLabels( + ctx context.Context, + binding *networkingv1alpha.NetworkBinding, + network networkingv1alpha.NetworkRef, + location networkingv1alpha.LocationReference, +) error { + desired := networkBindingLabels(network, location) + + stale := false + for k, v := range desired { + if binding.Labels[k] != v { + stale = true + break + } + } + if !stale { + return nil + } + + patch := client.MergeFrom(binding.DeepCopy()) + if binding.Labels == nil { + binding.Labels = map[string]string{} + } + for k, v := range desired { + binding.Labels[k] = v + } + if err := r.FederationClient.Patch(ctx, binding, patch); err != nil { + return fmt.Errorf("failed labelling network binding %s/%s: %w", + binding.Namespace, binding.Name, err) + } + return nil +} + +// networkBindingLabels returns the labels a consumer stamps on its binding. They +// are what makes the consumers of a presence findable as a list, and they are +// the only metadata on the binding compute owns. +func networkBindingLabels( + network networkingv1alpha.NetworkRef, + location networkingv1alpha.LocationReference, +) map[string]string { + return map[string]string{ + networkingv1alpha.NetworkLabel: network.Name, + networkingv1alpha.LocationLabel: location.Name, + } +} + +// deploymentNetworkRef returns the network a deployment's instances attach to. +// The interface list is capped at one entry by the API, and a deployment without +// one asks for no presence at all. +func deploymentNetworkRef(deployment *computev1alpha.WorkloadDeployment) (networkingv1alpha.NetworkRef, bool) { + interfaces := deployment.Spec.Template.Spec.NetworkInterfaces + if len(interfaces) == 0 || interfaces[0].Network.Name == "" { + return networkingv1alpha.NetworkRef{}, false + } + return interfaces[0].Network, true +} + +// mapNetworkBindingToRequest maps an event on a hub NetworkBinding back to the +// project WorkloadDeployment that declared it. +// +// The binding carries no cross-plane identity of its own. Its controller owner +// reference names the hub deployment, whose name is the same on every plane, and +// the hub namespace carries the project namespace and cluster this controller +// stamped on it when it created it. Bindings written by other consumers of the +// same presence have no such owner and are not this controller's to act on. +func (r *WorkloadDeploymentFederator) mapNetworkBindingToRequest( + ctx context.Context, + binding *networkingv1alpha.NetworkBinding, +) []mcreconcile.Request { + logger := log.FromContext(ctx) + + owner := metav1.GetControllerOf(binding) + if owner == nil || owner.Kind != kindWorkloadDeployment || owner.APIVersion != computev1alpha.GroupVersion.String() { + return nil + } + + var ns corev1.Namespace + if err := r.FederationCluster.GetClient().Get(ctx, types.NamespacedName{Name: binding.Namespace}, &ns); err != nil { + logger.V(1).Info("unable to resolve hub namespace for network binding; dropping event", + "hubNamespace", binding.Namespace, "error", err) + return nil + } + + projectNamespace := ns.Labels[downstreamclient.UpstreamOwnerNamespaceLabel] + clusterName := projectClusterNameFromLabel(ns.Labels[downstreamclient.UpstreamOwnerClusterNameLabel]) + if projectNamespace == "" || clusterName == "" { + logger.Error(nil, "hub namespace is missing upstream identity labels; dropping network binding event", + "hubNamespace", binding.Namespace, "name", binding.Name) + return nil + } + + // Same reasoning as the downstream deployment mapping: an unengaged project + // cluster has no WorkloadDeployment to reconcile, and enqueuing one would hot + // loop against a control plane with no compute types. + if _, err := r.mgr.GetCluster(ctx, multicluster.ClusterName(clusterName)); err != nil { + logger.V(1).Info("project cluster not engaged for network binding mapping; dropping event", + "clusterName", clusterName, "hubNamespace", binding.Namespace, "error", err) + return nil + } + + return []mcreconcile.Request{{ + ClusterName: multicluster.ClusterName(clusterName), + Request: ctrl.Request{ + NamespacedName: types.NamespacedName{ + Namespace: projectNamespace, + Name: owner.Name, + }, + }, + }} +} + +// networkBindingRefusal translates what NSO says about a binding into the +// deployment's Available condition, or returns nil when there is nothing worth +// reporting. +// +// Only refusals a person can act on are surfaced. A binding sits at +// NetworkContextNotReady for as long as nothing marks the shared context +// programmed, which is the ordinary steady state and says nothing about the +// deployment. Reporting is all that happens either way: instances are gated on +// their own interface claims, and a binding that stops being ready must not take +// a running deployment apart. +func networkBindingRefusal(binding *networkingv1alpha.NetworkBinding) *metav1.Condition { + if binding == nil { + return nil + } + ready := apimeta.FindStatusCondition(binding.Status.Conditions, networkingv1alpha.NetworkBindingReady) + if ready == nil || ready.Status != metav1.ConditionFalse { + return nil + } + + switch ready.Reason { + case networkingv1alpha.NetworkBindingReasonProjectUnresolved, + networkingv1alpha.NetworkBindingReasonNetworkNotFound, + networkingv1alpha.NetworkBindingReasonLocationNotAvailable: + default: + return nil + } + + return &metav1.Condition{ + Type: computev1alpha.WorkloadDeploymentAvailable, + Status: metav1.ConditionFalse, + Reason: ready.Reason, + Message: ready.Message, + } +} + +// applyNetworkBindingRefusal folds a refused binding into the status the +// federator is about to write, so the reason a network never arrived is readable +// on the deployment the user has rather than only on an object in the hub. +// +// A deployment that is already available keeps its own answer: what its +// instances are observed to be doing is the stronger statement, and a refusal +// that matters shows up there as instances failing to program. +func applyNetworkBindingRefusal( + status *computev1alpha.WorkloadDeploymentStatus, + binding *networkingv1alpha.NetworkBinding, + observedGeneration int64, +) { + refusal := networkBindingRefusal(binding) + if refusal == nil { + return + } + if apimeta.IsStatusConditionTrue(status.Conditions, computev1alpha.WorkloadDeploymentAvailable) { + return + } + refusal.ObservedGeneration = observedGeneration + apimeta.SetStatusCondition(&status.Conditions, *refusal) +} diff --git a/internal/controller/workloaddeployment_network_binding_test.go b/internal/controller/workloaddeployment_network_binding_test.go new file mode 100644 index 00000000..b5ccee56 --- /dev/null +++ b/internal/controller/workloaddeployment_network_binding_test.go @@ -0,0 +1,568 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" + + computev1alpha "go.datum.net/compute/api/v1alpha" + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + "go.miloapis.com/milo/pkg/downstreamclient" +) + +const ( + testNetworkName = "default" + testLocationName = "dfw" + testHubWDUID = types.UID("hub-wd-uid-9999") +) + +// testHubDeployment returns the hub copy of the test WorkloadDeployment, already +// carrying an interface on testNetworkName. Options adjust it further. +func testHubDeployment(opts ...func(*computev1alpha.WorkloadDeployment)) *computev1alpha.WorkloadDeployment { + wd := &computev1alpha.WorkloadDeployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: testWDName, + Namespace: testKarmadaNSStr, + UID: testHubWDUID, + }, + Spec: computev1alpha.WorkloadDeploymentSpec{ + CityCode: testCityCodeLAX, + Template: computev1alpha.InstanceTemplateSpec{ + Spec: computev1alpha.InstanceSpec{ + NetworkInterfaces: []computev1alpha.InstanceNetworkInterface{{ + Name: "eth0", + Network: networkingv1alpha.NetworkRef{Name: testNetworkName}, + }}, + }, + }, + }, + } + for _, opt := range opts { + opt(wd) + } + return wd +} + +// withServingLocation records the location a cell reported for the deployment, +// which on the hub arrives through Karmada status aggregation. +func withServingLocation(name string) func(*computev1alpha.WorkloadDeployment) { + return func(wd *computev1alpha.WorkloadDeployment) { + wd.Status.Location = &networkingv1alpha.LocationReference{Name: name} + } +} + +// withInterfaceNetwork points the deployment's single interface at a network. +func withInterfaceNetwork(name string) func(*computev1alpha.WorkloadDeployment) { + return func(wd *computev1alpha.WorkloadDeployment) { + wd.Spec.Template.Spec.NetworkInterfaces[0].Network = networkingv1alpha.NetworkRef{Name: name} + } +} + +func getBinding(t *testing.T, cl client.Client) (*networkingv1alpha.NetworkBinding, error) { + t.Helper() + var binding networkingv1alpha.NetworkBinding + err := cl.Get(context.Background(), types.NamespacedName{ + Namespace: testKarmadaNSStr, + Name: testWDName, + }, &binding) + return &binding, err +} + +// TestEnsureNetworkBinding_DeclaresPresenceWhereDeploymentRuns verifies the +// binding a placed deployment produces: the network it asks for, the location a +// cell serves it from, a consumer record naming the deployment, and a real owner +// reference so the hub releases it with its owner. +func TestEnsureNetworkBinding_DeclaresPresenceWhereDeploymentRuns(t *testing.T) { + t.Parallel() + + hubWD := testHubDeployment(withServingLocation(testLocationName)) + karmadaClient := newKarmadaFakeClient(hubWD) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + + binding, err := r.ensureNetworkBinding(context.Background(), hubWD) + require.NoError(t, err) + require.NotNil(t, binding) + + stored, err := getBinding(t, karmadaClient) + require.NoError(t, err) + + assert.Equal(t, testNetworkName, stored.Spec.Network.Name) + assert.Equal(t, testLocationName, stored.Spec.Location.Name) + + require.NotNil(t, stored.Spec.Consumer) + assert.Equal(t, computev1alpha.GroupVersion.Group, stored.Spec.Consumer.APIGroup) + assert.Equal(t, kindWorkloadDeployment, stored.Spec.Consumer.Kind) + assert.Equal(t, testWDName, stored.Spec.Consumer.Name) + + require.Len(t, stored.OwnerReferences, 1) + owner := stored.OwnerReferences[0] + assert.Equal(t, kindWorkloadDeployment, owner.Kind) + assert.Equal(t, testWDName, owner.Name) + assert.Equal(t, testHubWDUID, owner.UID) + require.NotNil(t, owner.Controller) + assert.True(t, *owner.Controller) + assert.Nil(t, owner.BlockOwnerDeletion, + "blocking the owner's deletion needs finalizers access compute is not granted on the hub") + + assert.Equal(t, testNetworkName, stored.Labels[networkingv1alpha.NetworkLabel]) + assert.Equal(t, testLocationName, stored.Labels[networkingv1alpha.LocationLabel]) +} + +// TestEnsureNetworkBinding_NothingToDeclareYet verifies that a deployment with no +// location, or with no interface, produces no binding at all: an unplaced +// deployment has no location to name, and a binding cannot be created without one. +func TestEnsureNetworkBinding_NothingToDeclareYet(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + hubWD *computev1alpha.WorkloadDeployment + }{ + { + name: "no serving location", + hubWD: testHubDeployment(), + }, + { + name: "no network interfaces", + hubWD: testHubDeployment(withServingLocation(testLocationName), func(wd *computev1alpha.WorkloadDeployment) { + wd.Spec.Template.Spec.NetworkInterfaces = nil + }), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + karmadaClient := newKarmadaFakeClient(tc.hubWD) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + + binding, err := r.ensureNetworkBinding(context.Background(), tc.hubWD) + require.NoError(t, err) + assert.Nil(t, binding) + + var bindings networkingv1alpha.NetworkBindingList + require.NoError(t, karmadaClient.List(context.Background(), &bindings)) + assert.Empty(t, bindings.Items) + }) + } +} + +// TestEnsureNetworkBinding_PreservesNSOOwnedLabels is the regression guard for +// the write loop this design is built to avoid. NSO's presence controller stamps +// the network's UID on the binding, and its garbage collection keys on that +// label. A reconcile that assigned the label map wholesale would strip it every +// pass and put the two controllers in a fight neither one settles. +func TestEnsureNetworkBinding_PreservesNSOOwnedLabels(t *testing.T) { + t.Parallel() + + ctx := context.Background() + hubWD := testHubDeployment(withServingLocation(testLocationName)) + karmadaClient := newKarmadaFakeClient(hubWD) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + + _, err := r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + + // NSO stamps the network UID, and drops a label compute never wrote so the + // test also catches the case of compute pruning keys it does not own. + stored, err := getBinding(t, karmadaClient) + require.NoError(t, err) + patch := client.MergeFrom(stored.DeepCopy()) + stored.Labels[networkingv1alpha.NetworkUIDLabel] = "network-uid-from-nso" + stored.Labels["networking.datumapis.com/some-other-key"] = "keep-me" + // Also drop one of compute's own labels, so the reconcile below has a reason + // to write and cannot pass by doing nothing at all. + delete(stored.Labels, networkingv1alpha.LocationLabel) + require.NoError(t, karmadaClient.Patch(ctx, stored, patch)) + + _, err = r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + + after, err := getBinding(t, karmadaClient) + require.NoError(t, err) + assert.Equal(t, "network-uid-from-nso", after.Labels[networkingv1alpha.NetworkUIDLabel], + "NSO's network-uid label must survive a compute reconcile") + assert.Equal(t, "keep-me", after.Labels["networking.datumapis.com/some-other-key"]) + assert.Equal(t, testLocationName, after.Labels[networkingv1alpha.LocationLabel], + "compute's own labels are restored") +} + +// TestEnsureNetworkBinding_NoWriteWhenSettled verifies the steady state is quiet: +// a binding that already declares the right presence is not rewritten, so the +// binding watch does not feed itself. +func TestEnsureNetworkBinding_NoWriteWhenSettled(t *testing.T) { + t.Parallel() + + ctx := context.Background() + hubWD := testHubDeployment(withServingLocation(testLocationName)) + karmadaClient := newKarmadaFakeClient(hubWD) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + + _, err := r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + first, err := getBinding(t, karmadaClient) + require.NoError(t, err) + + _, err = r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + second, err := getBinding(t, karmadaClient) + require.NoError(t, err) + + assert.Equal(t, first.ResourceVersion, second.ResourceVersion, + "a settled binding must not be written again") +} + +// TestEnsureNetworkBinding_RecreatesOnDivergence covers both ways the declared +// pair can stop matching reality: a user edits the workload's network, and a cell +// serves the deployment from a different location. Both fields are immutable on a +// NetworkBinding, so the old declaration is removed and the next pass makes the +// new one. +func TestEnsureNetworkBinding_RecreatesOnDivergence(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + changed func(*computev1alpha.WorkloadDeployment) + wantNetwork string + wantLocation string + }{ + { + name: "network changed", + changed: withInterfaceNetwork("other-network"), + wantNetwork: "other-network", + wantLocation: testLocationName, + }, + { + name: "serving location changed", + changed: withServingLocation("ord"), + wantNetwork: testNetworkName, + wantLocation: "ord", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + ctx := context.Background() + hubWD := testHubDeployment(withServingLocation(testLocationName)) + karmadaClient := newKarmadaFakeClient(hubWD) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + + _, err := r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + + tc.changed(hubWD) + + // The diverged declaration goes away first, and nothing is returned to + // report on while it does. + binding, err := r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + assert.Nil(t, binding) + _, err = getBinding(t, karmadaClient) + assert.True(t, apierrors.IsNotFound(err), "diverged binding should be deleted, got %v", err) + + // The next pass declares the new presence. + _, err = r.ensureNetworkBinding(ctx, hubWD) + require.NoError(t, err) + recreated, err := getBinding(t, karmadaClient) + require.NoError(t, err) + + assert.Equal(t, tc.wantNetwork, recreated.Spec.Network.Name) + assert.Equal(t, tc.wantLocation, recreated.Spec.Location.Name) + }) + } +} + +// TestEnsureNetworkBinding_LeavesForeignBindingAlone verifies compute refuses to +// rewrite or delete a binding under the same name that it does not own. Deleting +// another consumer's declaration would take away a presence compute never asked +// for. +func TestEnsureNetworkBinding_LeavesForeignBindingAlone(t *testing.T) { + t.Parallel() + + ctx := context.Background() + hubWD := testHubDeployment(withServingLocation(testLocationName)) + foreign := &networkingv1alpha.NetworkBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: testWDName, + Namespace: testKarmadaNSStr, + }, + Spec: networkingv1alpha.NetworkBindingSpec{ + Network: networkingv1alpha.NetworkRef{Name: "someone-elses-network"}, + Location: networkingv1alpha.LocationReference{Name: testLocationName}, + }, + } + karmadaClient := newKarmadaFakeClient(hubWD, foreign) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + + _, err := r.ensureNetworkBinding(ctx, hubWD) + require.Error(t, err) + + stored, err := getBinding(t, karmadaClient) + require.NoError(t, err) + assert.Equal(t, "someone-elses-network", stored.Spec.Network.Name) +} + +// TestWorkloadDeploymentFederator_CreatesNetworkBindingOnReconcile verifies the +// binding is produced by an ordinary federation pass once the hub deployment +// carries the location a cell reported. +func TestWorkloadDeploymentFederator_CreatesNetworkBindingOnReconcile(t *testing.T) { + t.Parallel() + + ctx := context.Background() + wd := testWorkloadDeployment(withFinalizer, func(wd *computev1alpha.WorkloadDeployment) { + wd.Spec.Template.Spec.NetworkInterfaces = []computev1alpha.InstanceNetworkInterface{{ + Name: "eth0", + Network: networkingv1alpha.NetworkRef{Name: testNetworkName}, + }} + }) + projectClient := newProjectFakeClient(testProjectNamespace(), wd) + // The hub copy already exists carrying the aggregated serving location, which + // is the only place the location is known. + karmadaClient := newKarmadaFakeClient(testHubDeployment(withServingLocation(testLocationName))) + r := newTestFederator(projectClient, karmadaClient) + + _, err := r.Reconcile(ctx, reconcileRequest()) + require.NoError(t, err) + + stored, err := getBinding(t, karmadaClient) + require.NoError(t, err) + assert.Equal(t, testNetworkName, stored.Spec.Network.Name) + assert.Equal(t, testLocationName, stored.Spec.Location.Name) +} + +// TestMapNetworkBindingToRequest verifies the binding-to-deployment mapping: the +// deployment name comes from the binding's controller owner, and the project +// namespace and cluster from the hub namespace this controller stamped. A binding +// another consumer wrote maps to nothing. +func TestMapNetworkBindingToRequest(t *testing.T) { + t.Parallel() + + hubNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: testKarmadaNSStr, + Labels: map[string]string{ + downstreamclient.UpstreamOwnerClusterNameLabel: EncodeClusterName(testCluster), + downstreamclient.UpstreamOwnerNamespaceLabel: testProjNS, + }, + }, + } + unlabelledNS := &corev1.Namespace{ + ObjectMeta: metav1.ObjectMeta{Name: testKarmadaNSStr}, + } + + owned := &networkingv1alpha.NetworkBinding{ + ObjectMeta: metav1.ObjectMeta{ + Name: testWDName, + Namespace: testKarmadaNSStr, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: computev1alpha.GroupVersion.String(), + Kind: kindWorkloadDeployment, + Name: testWDName, + UID: testHubWDUID, + Controller: func() *bool { b := true; return &b }(), + }}, + }, + } + foreign := &networkingv1alpha.NetworkBinding{ + ObjectMeta: metav1.ObjectMeta{Name: "someone-else", Namespace: testKarmadaNSStr}, + } + + tests := []struct { + name string + ns *corev1.Namespace + binding *networkingv1alpha.NetworkBinding + want []mcreconcile.Request + }{ + { + name: "maps to the declaring deployment", + ns: hubNS, + binding: owned, + want: []mcreconcile.Request{{ + ClusterName: testCluster, + Request: ctrl.Request{ + NamespacedName: types.NamespacedName{Namespace: testProjNS, Name: testWDName}, + }, + }}, + }, + { + name: "another consumer's binding is not ours", + ns: hubNS, + binding: foreign, + want: nil, + }, + { + name: "hub namespace without identity labels is dropped", + ns: unlabelledNS, + binding: owned, + want: nil, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + karmadaClient := newKarmadaFakeClient(tc.ns) + r := newTestFederator(newProjectFakeClient(), karmadaClient) + r.FederationCluster = newFakeCluster(karmadaClient) + + got := r.mapNetworkBindingToRequest(context.Background(), tc.binding) + assert.Equal(t, tc.want, got) + }) + } +} + +// TestNetworkBindingRefusalReporting verifies which of NSO's answers reach the +// deployment's Available condition. Only refusals a person can act on are +// reported; the ordinary not-ready state says nothing about the deployment and +// would otherwise mark every deployment unavailable forever, since nothing marks +// a network context programmed today. +func TestNetworkBindingRefusalReporting(t *testing.T) { + t.Parallel() + + binding := func(status metav1.ConditionStatus, reason string) *networkingv1alpha.NetworkBinding { + return &networkingv1alpha.NetworkBinding{ + Status: networkingv1alpha.NetworkBindingStatus{ + Conditions: []metav1.Condition{{ + Type: networkingv1alpha.NetworkBindingReady, + Status: status, + Reason: reason, + Message: "from NSO", + LastTransitionTime: metav1.Now(), + }}, + }, + } + } + + tests := []struct { + name string + binding *networkingv1alpha.NetworkBinding + wantReason string + }{ + {name: "no binding yet", binding: nil}, + { + name: "location not available", + binding: binding(metav1.ConditionFalse, networkingv1alpha.NetworkBindingReasonLocationNotAvailable), + wantReason: networkingv1alpha.NetworkBindingReasonLocationNotAvailable, + }, + { + name: "network not found", + binding: binding(metav1.ConditionFalse, networkingv1alpha.NetworkBindingReasonNetworkNotFound), + wantReason: networkingv1alpha.NetworkBindingReasonNetworkNotFound, + }, + { + name: "project unresolved", + binding: binding(metav1.ConditionFalse, networkingv1alpha.NetworkBindingReasonProjectUnresolved), + wantReason: networkingv1alpha.NetworkBindingReasonProjectUnresolved, + }, + { + name: "context not ready is the ordinary state", + binding: binding(metav1.ConditionFalse, networkingv1alpha.NetworkBindingReasonNetworkContextNotReady), + }, + { + name: "pending is not an answer yet", + binding: binding(metav1.ConditionUnknown, networkingv1alpha.NetworkBindingReasonPending), + }, + { + name: "ready", + binding: binding(metav1.ConditionTrue, networkingv1alpha.NetworkBindingReasonNetworkContextReady), + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := networkBindingRefusal(tc.binding) + if tc.wantReason == "" { + assert.Nil(t, got) + return + } + require.NotNil(t, got) + assert.Equal(t, computev1alpha.WorkloadDeploymentAvailable, got.Type) + assert.Equal(t, metav1.ConditionFalse, got.Status) + assert.Equal(t, tc.wantReason, got.Reason) + assert.Equal(t, "from NSO", got.Message) + }) + } +} + +// TestApplyNetworkBindingRefusal_KeepsAnAvailableDeploymentAvailable verifies a +// refusal never contradicts a deployment whose instances are observed running. +func TestApplyNetworkBindingRefusal_KeepsAnAvailableDeploymentAvailable(t *testing.T) { + t.Parallel() + + status := &computev1alpha.WorkloadDeploymentStatus{ + Conditions: []metav1.Condition{{ + Type: computev1alpha.WorkloadDeploymentAvailable, + Status: metav1.ConditionTrue, + Reason: "InstancesAvailable", + LastTransitionTime: metav1.Now(), + }}, + } + refused := &networkingv1alpha.NetworkBinding{ + Status: networkingv1alpha.NetworkBindingStatus{ + Conditions: []metav1.Condition{{ + Type: networkingv1alpha.NetworkBindingReady, + Status: metav1.ConditionFalse, + Reason: networkingv1alpha.NetworkBindingReasonNetworkNotFound, + LastTransitionTime: metav1.Now(), + }}, + }, + } + + applyNetworkBindingRefusal(status, refused, 1) + + assert.Equal(t, "InstancesAvailable", status.Conditions[0].Reason) + assert.Equal(t, metav1.ConditionTrue, status.Conditions[0].Status) +} + +// TestNetworkBindingStateDoesNotGateInstances pins the decision that a binding is +// reported on and never acted on. Nothing marks a network context programmed +// today, so a binding sits not-ready indefinitely; gating on it would stop every +// instance from ever starting, and tearing down on it would take running +// instances apart and release their addresses. Instances remain gated on their +// own interface claim holding what it asked for. +func TestNetworkBindingStateDoesNotGateInstances(t *testing.T) { + t.Parallel() + + claim := &networkingv1alpha.NetworkInterfaceClaim{ + Status: networkingv1alpha.NetworkInterfaceClaimStatus{ + Conditions: []metav1.Condition{ + claimCondition(networkingv1alpha.NetworkInterfaceClaimBound, metav1.ConditionTrue, "Bound"), + claimCondition(networkingv1alpha.NetworkInterfaceClaimAllocated, metav1.ConditionTrue, "Allocated"), + }, + }, + } + assert.True(t, networkInterfaceClaimSatisfied(claim), + "claim satisfaction is decided by the claim alone") + + notReady := &networkingv1alpha.NetworkBinding{ + Status: networkingv1alpha.NetworkBindingStatus{ + Conditions: []metav1.Condition{{ + Type: networkingv1alpha.NetworkBindingReady, + Status: metav1.ConditionFalse, + Reason: networkingv1alpha.NetworkBindingReasonNetworkContextNotReady, + LastTransitionTime: metav1.Now(), + }}, + }, + } + assert.Nil(t, networkBindingRefusal(notReady), + "a binding waiting on a context must not even be reported, let alone acted on") + assert.True(t, networkInterfaceClaimSatisfied(claim), + "binding state is not an input to instance gating") +}