diff --git a/api/v1alpha/instance_types.go b/api/v1alpha/instance_types.go index ffcf582f..b8e14a7a 100644 --- a/api/v1alpha/instance_types.go +++ b/api/v1alpha/instance_types.go @@ -838,10 +838,23 @@ const ( WorkloadReasonNetworkNotFound = "NetworkNotFound" // WorkloadDeploymentReasonNoMatchingLocation is set on WorkloadDeployment.Available - // while no Location matches the deployment's city code. The message names the - // unresolved city; network provisioning cannot start until that Location exists. + // while the cell has not been told which location it serves, so the deployment + // cannot be given one. The value is kept for compatibility with clients that + // already match on it. WorkloadDeploymentReasonNoMatchingLocation = "NoMatchingLocation" + // WorkloadDeploymentReasonAmbiguousServingLocation is set on + // WorkloadDeployment.Available when more than one location has been delivered + // to the cell. The cell will not guess which one it serves, so the deployment + // waits until the platform resolves the conflict. + WorkloadDeploymentReasonAmbiguousServingLocation = "AmbiguousServingLocation" + + // WorkloadDeploymentReasonCityCodeMismatch is set on + // WorkloadDeployment.Available when the deployment asks for one city and the + // cell serves another. It means the deployment was placed on the wrong cell, + // which is a platform fault rather than anything the user can correct. + WorkloadDeploymentReasonCityCodeMismatch = "CityCodeMismatch" + // WorkloadDeploymentReasonNetworkProvisioning is set on WorkloadDeployment.Available // while the network binding or subnet is still being provisioned. // Replaces the previously-emitted inline literal "ProvisioningNetwork". diff --git a/config/base/crd/bases/compute.datumapis.com_instances.yaml b/config/base/crd/bases/compute.datumapis.com_instances.yaml index 5d3a8daa..5e050e61 100644 --- a/config/base/crd/bases/compute.datumapis.com_instances.yaml +++ b/config/base/crd/bases/compute.datumapis.com_instances.yaml @@ -97,12 +97,8 @@ spec: name: description: Name of a datum location type: string - namespace: - description: Namespace for the datum location - type: string required: - name - - namespace type: object networkInterfaces: description: |- diff --git a/config/base/crd/bases/compute.datumapis.com_workloaddeployments.yaml b/config/base/crd/bases/compute.datumapis.com_workloaddeployments.yaml index 391987d1..48fa5d2e 100644 --- a/config/base/crd/bases/compute.datumapis.com_workloaddeployments.yaml +++ b/config/base/crd/bases/compute.datumapis.com_workloaddeployments.yaml @@ -210,12 +210,8 @@ spec: name: description: Name of a datum location type: string - namespace: - description: Namespace for the datum location - type: string required: - name - - namespace type: object networkInterfaces: description: |- @@ -1281,12 +1277,8 @@ spec: name: description: Name of a datum location type: string - namespace: - description: Namespace for the datum location - type: string required: - name - - namespace type: object observedGeneration: description: |- diff --git a/config/base/crd/bases/compute.datumapis.com_workloads.yaml b/config/base/crd/bases/compute.datumapis.com_workloads.yaml index 2b8e3dfd..41dfd418 100644 --- a/config/base/crd/bases/compute.datumapis.com_workloads.yaml +++ b/config/base/crd/bases/compute.datumapis.com_workloads.yaml @@ -214,12 +214,8 @@ spec: name: description: Name of a datum location type: string - namespace: - description: Namespace for the datum location - type: string required: - name - - namespace type: object networkInterfaces: description: |- diff --git a/config/components/controller_rbac/role.yaml b/config/components/controller_rbac/role.yaml index 236bff0b..935b4b8a 100644 --- a/config/components/controller_rbac/role.yaml +++ b/config/components/controller_rbac/role.yaml @@ -79,24 +79,24 @@ rules: - apiGroups: - networking.datumapis.com resources: - - locations - - networkinterfaces - - networks + - networkinterfaceclaims verbs: + - create + - delete - get - list + - patch + - update - watch - apiGroups: - networking.datumapis.com resources: - - networkinterfaceclaims + - networkinterfaces + - networks + - servinglocations verbs: - - create - - delete - get - list - - patch - - update - watch - apiGroups: - quota.miloapis.com diff --git a/go.mod b/go.mod index b9e749fd..146ed055 100644 --- a/go.mod +++ b/go.mod @@ -10,9 +10,11 @@ require ( github.com/onsi/gomega v1.42.1 github.com/prometheus/client_golang v1.23.2 github.com/stretchr/testify v1.11.1 - // TODO: temporary pin to the NetworkInterfaceClaim branch (datum-cloud/network-services-operator#360). - // Re-pin to a tagged release before merging. - go.datum.net/network-services-operator v0.25.6-0.20260813185515-6a019e3fd9b8 + // TODO: temporary pin to network-services-operator main. The latest tag + // (v0.25.5) predates NetworkInterfaceClaim (#360), ServingLocation (#372), + // and per-location networks (#373); re-pin to a tagged release before + // compute cuts a release. + go.datum.net/network-services-operator v0.25.6-0.20260814125050-f3a972d9da33 go.miloapis.com/milo v0.32.0 golang.org/x/crypto v0.54.0 golang.org/x/sync v0.22.0 @@ -112,7 +114,7 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect - k8s.io/apiextensions-apiserver v0.36.1 + k8s.io/apiextensions-apiserver v0.36.1 // indirect k8s.io/apiserver v0.36.1 // indirect k8s.io/klog/v2 v2.140.0 // indirect k8s.io/kube-openapi v0.0.0-20260330154417-16be699c7b31 // indirect @@ -120,5 +122,5 @@ require ( sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.3.2 // indirect - sigs.k8s.io/yaml v1.6.0 + sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/go.sum b/go.sum index 380e5aaf..30417465 100644 --- a/go.sum +++ b/go.sum @@ -175,8 +175,8 @@ github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -go.datum.net/network-services-operator v0.25.6-0.20260813185515-6a019e3fd9b8 h1:R82aXI8iC+6JMm5WjLxpBJRgRTPUACLlvxOW4svZ03Y= -go.datum.net/network-services-operator v0.25.6-0.20260813185515-6a019e3fd9b8/go.mod h1:oz57/uTB5HAjLGrqs1pn1HTP/jW9vkGgMODPvCiUlvc= +go.datum.net/network-services-operator v0.25.6-0.20260814125050-f3a972d9da33 h1:oAbsJRV6zNrwO7XymY2WJ7+uAa+PBx9mNeXbo48CQgk= +go.datum.net/network-services-operator v0.25.6-0.20260814125050-f3a972d9da33/go.mod h1:oz57/uTB5HAjLGrqs1pn1HTP/jW9vkGgMODPvCiUlvc= go.miloapis.com/milo v0.32.0 h1:TkNIQu/37d+SEquLJ5+GmdisSl+K2RT7eEC4idg6RIs= go.miloapis.com/milo v0.32.0/go.mod h1:GKK3afjCwshfZfvhjNe1wp/H45z4m7x5oG/8xbSgU1M= go.miloapis.com/service-catalog v0.4.0 h1:LvO1WCHMCoFokpS5igWMP8kyqly9gUFQmQj5IGhwuKs= diff --git a/internal/controller/indexers.go b/internal/controller/indexers.go index 65ea258c..9816273b 100644 --- a/internal/controller/indexers.go +++ b/internal/controller/indexers.go @@ -16,10 +16,6 @@ import ( const ( deploymentWorkloadUIDIndex = "deploymentWorkloadUIDIndex" workloadNetworksIndex = "workloadNetworksIndex" - // deploymentCityCodeIndex indexes WorkloadDeployments by their Spec.CityCode - // so that the Location watch can efficiently find the deployments targeting - // the city a changed Location serves. - deploymentCityCodeIndex = "deploymentCityCodeIndex" deploymentLocationIndex = "deploymentLocationIndex" @@ -55,10 +51,6 @@ func addWorkloadDeploymentIndexers(ctx context.Context, mgr mcmanager.Manager) e return fmt.Errorf("failed to add workload deployment indexer %q: %w", deploymentWorkloadUIDIndex, err) } - if err := mgr.GetFieldIndexer().IndexField(ctx, &computev1alpha.WorkloadDeployment{}, deploymentCityCodeIndex, deploymentCityCodeIndexFunc); err != nil { - return fmt.Errorf("failed to add workload deployment indexer %q: %w", deploymentCityCodeIndex, err) - } - // Index workload deployments by location if err := mgr.GetFieldIndexer().IndexField(ctx, &computev1alpha.WorkloadDeployment{}, deploymentLocationIndex, deploymentLocationIndexFunc); err != nil { return fmt.Errorf("failed to add workload deployment indexer %q: %w", deploymentLocationIndex, err) @@ -79,7 +71,7 @@ func addWorkloadDeploymentIndexers(ctx context.Context, mgr mcmanager.Manager) e func deploymentWorkloadUIDIndexFunc(o client.Object) []string { // Skip deployments without a workload UID: indexing them under the empty // key would make them matchable by a GC query built from a corrupt (empty) - // UID, mirroring deploymentCityCodeIndexFunc. + // UID. uid := string(o.(*computev1alpha.WorkloadDeployment).Spec.WorkloadRef.UID) if uid == "" { return nil @@ -87,26 +79,14 @@ func deploymentWorkloadUIDIndexFunc(o client.Object) []string { return []string{uid} } -func deploymentCityCodeIndexFunc(o client.Object) []string { - deployment := o.(*computev1alpha.WorkloadDeployment) - if deployment.Spec.CityCode == "" { - return nil - } - return []string{deployment.Spec.CityCode} -} - func deploymentLocationIndexFunc(o client.Object) []string { deployment := o.(*computev1alpha.WorkloadDeployment) if deployment.Status.Location == nil { return nil } - return []string{ - types.NamespacedName{ - Namespace: deployment.Status.Location.Namespace, - Name: deployment.Status.Location.Name, - }.String(), - } + // Locations are cluster-scoped, so the name alone identifies one. + return []string{deployment.Status.Location.Name} } func addWorkloadIndexers(ctx context.Context, mgr mcmanager.Manager) error { diff --git a/internal/controller/instancecontrol/stateful/stateful_control_test.go b/internal/controller/instancecontrol/stateful/stateful_control_test.go index dc0557f2..393ab437 100644 --- a/internal/controller/instancecontrol/stateful/stateful_control_test.go +++ b/internal/controller/instancecontrol/stateful/stateful_control_test.go @@ -338,8 +338,7 @@ func TestInstanceLocation_SetWhenDeploymentStatusLocationPresent(t *testing.T) { deployment := getWorkloadDeployment("test-location-set", 1) deployment.Status.Location = &networkingv1alpha.LocationReference{ - Name: "loc-dfw-1", - Namespace: "networking-system", + Name: "loc-dfw-1", } var currentInstances []v1alpha.Instance @@ -353,7 +352,6 @@ func TestInstanceLocation_SetWhenDeploymentStatusLocationPresent(t *testing.T) { assert.NotNil(t, instance.Spec.Location, "Spec.Location must be set when deployment.Status.Location is non-nil") assert.Equal(t, "loc-dfw-1", instance.Spec.Location.Name) - assert.Equal(t, "networking-system", instance.Spec.Location.Namespace) } // TestInstanceLocation_NilWhenDeploymentStatusLocationAbsent verifies that when diff --git a/internal/controller/workload_controller.go b/internal/controller/workload_controller.go index 562df4a8..c9d8ac98 100644 --- a/internal/controller/workload_controller.go +++ b/internal/controller/workload_controller.go @@ -460,7 +460,7 @@ func (r *WorkloadReconciler) getDeploymentsForWorkload( for _, cityCode := range placement.CityCodes { foundLocation := false for _, location := range locations.Items { - locationCityCode, ok := location.Spec.Topology["topology.datum.net/city-code"] + locationCityCode, ok := location.Spec.Topology[networkingv1alpha.TopologyCityCodeKey] if ok && cityCode == locationCityCode { foundLocation = true break diff --git a/internal/controller/workloaddeployment_controller.go b/internal/controller/workloaddeployment_controller.go index b435c758..308b3700 100644 --- a/internal/controller/workloaddeployment_controller.go +++ b/internal/controller/workloaddeployment_controller.go @@ -6,6 +6,7 @@ import ( "context" "fmt" "slices" + "strings" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -79,7 +80,7 @@ func workloadDeploymentPodSelector(deployment *computev1alpha.WorkloadDeployment // +kubebuilder:rbac:groups=compute.datumapis.com,resources=workloaddeployments,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=compute.datumapis.com,resources=workloaddeployments/status,verbs=get;update;patch // +kubebuilder:rbac:groups=compute.datumapis.com,resources=workloaddeployments/finalizers,verbs=update -// +kubebuilder:rbac:groups=networking.datumapis.com,resources=locations,verbs=get;list;watch +// +kubebuilder:rbac:groups=networking.datumapis.com,resources=servinglocations,verbs=get;list;watch // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaceclaims,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networkinterfaces,verbs=get;list;watch // The management-mode WorkloadReconciler watches Networks. Declare the grant as @@ -137,6 +138,25 @@ func (r *WorkloadDeploymentReconciler) Reconcile(ctx context.Context, req mcreco // Status().Update call when nothing changed (see loop-prevention comment below). existingStatus := *deployment.Status.DeepCopy() + // Resolve the cell's location before instances are built: the instance + // control strategy stamps Instance.Spec.Location from Status.Location as it + // creates them, so resolving afterwards left the first generation of + // instances permanently without one. + var location servingLocationResult + if r.NetworkingEnabled { + location, err = r.resolveLocation(ctx, cl.GetClient()) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed resolving location: %w", err) + } + location.evaluate(&deployment) + + // A location the cell contradicts is never written to status: an Instance + // carrying the wrong location is worse than one carrying none. + if location.reference != nil { + deployment.Status.Location = location.reference + } + } + // Collect all instances for this deployment listOpts := client.MatchingLabels{ computev1alpha.WorkloadDeploymentUIDLabel: string(deployment.GetUID()), @@ -178,27 +198,24 @@ func (r *WorkloadDeploymentReconciler) Reconcile(ctx context.Context, req mcreco // interface claims are created. This lets Instances reach the runtime on cells // where network-services-operator (VPC) is not yet available. networkReadyByInstance := make(map[string]bool, len(instances.Items)) - locationResolved := true - if !r.NetworkingEnabled { + switch { + case !r.NetworkingEnabled: for _, instance := range instances.Items { networkReadyByInstance[instance.Name] = true } - } else { - resolvedLocation, err := r.resolveLocation(ctx, cl.GetClient(), &deployment) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed resolving location: %w", err) - } - // Persist the resolved Location to status so downstream components (e.g. - // the stateful instance control strategy) can propagate it to Instances. - // When no matching Location exists, resolvedLocation is nil and - // Status.Location remains nil — instance creation is not blocked, and - // interface claims do not depend on it: a claim is served by the control - // plane it is created in, which is already location scoped. - locationResolved = resolvedLocation != nil - if resolvedLocation != nil { - deployment.Status.Location = resolvedLocation - } + case location.blocked: + // The cell has stated an identity that contradicts where this deployment + // was asked to run. Addresses are allocated out of the cell's own + // location, so claiming them here would place the workload in a city the + // user did not ask for. Leaving every instance's Network gate held keeps + // the deployment out of service until the placement fault is corrected, + // and releases it again with no further action once it is. The Available + // condition names the fault. + logger.Info("holding instances: cell location contradicts deployment placement", + "reason", location.reason, "message", location.message) + + default: networkReadyByInstance, err = r.reconcileNetworkInterfaceClaims(ctx, cl.GetClient(), &deployment, instances.Items) if err != nil { return ctrl.Result{}, fmt.Errorf("failed reconciling network interface claims: %w", err) @@ -279,7 +296,7 @@ func (r *WorkloadDeploymentReconciler) Reconcile(ctx context.Context, req mcreco ObservedGeneration: deployment.Generation, }) } else { - availCond := selectWDBlockingCondition(&deployment, networkReady, locationResolved, quotaBlockedReplicas, referencedDataBlockedReplicas, replicas, desiredReplicas) + availCond := selectWDBlockingCondition(&deployment, networkReady, location, quotaBlockedReplicas, referencedDataBlockedReplicas, replicas, desiredReplicas) apimeta.SetStatusCondition(&deployment.Status.Conditions, availCond) } @@ -308,6 +325,19 @@ func (r *WorkloadDeploymentReconciler) reconcileInstanceGates( ) (currentReplicas, updatedReplicas, readyReplicas, quotaBlockedReplicas, referencedDataBlockedReplicas int, err error) { templateHash := instancecontrol.ComputeHash(deployment.Spec.Template) for _, instance := range instances { + // Instances are stamped with the deployment's location as they are + // created, which leaves any instance that predates the cell learning its + // own location without one, and nothing else ever revisits it. Backfill + // it here. Best-effort by design: a failure is logged and the instance + // keeps running, because location has never gated scheduling. + if deployment.Status.Location != nil && instance.Spec.Location == nil { + base := instance.DeepCopy() + instance.Spec.Location = deployment.Status.Location + if patchErr := c.Patch(ctx, &instance, client.MergeFrom(base)); patchErr != nil { + log.FromContext(ctx).Error(patchErr, "failed backfilling instance location", "instance", instance.Name) + } + } + // Propagate suspension state from deployment to instance. if instance.Status.Suspended != deployment.Status.Suspended { base := instance.DeepCopy() @@ -460,7 +490,8 @@ func wdRefDataCondChanged(old, new *metav1.Condition) bool { // actionable one (e.g. missing referenced data). func selectWDBlockingCondition( deployment *computev1alpha.WorkloadDeployment, - networkReady, locationResolved bool, + networkReady bool, + location servingLocationResult, quotaBlockedReplicas, referencedDataBlockedReplicas, replicas int, desiredReplicas int32, ) metav1.Condition { @@ -480,13 +511,12 @@ func selectWDBlockingCondition( } } - // An unresolved city is the only user-visible signal that instances are - // running without the Location their placement was asked for, and it is - // considered first so it wins over the generic provisioning reason they - // share a priority with. - if !locationResolved { - consider(computev1alpha.WorkloadDeploymentReasonNoMatchingLocation, - fmt.Sprintf("No Location matches city code %q", deployment.Spec.CityCode)) + // An unusable cell location is the only user-visible signal that instances + // are running without the location their placement asked for, and it is + // considered first so it wins over the generic provisioning reason it shares + // a priority with. + if location.reason != "" { + consider(location.reason, location.message) } if !networkReady { @@ -567,6 +597,9 @@ func selectWDBlockingCondition( // 5 - SourceNotFound / SourceTooLarge / SourceUnauthorized (hard spec error) // 6 - NetworkNotFound (hard error; user action required) // 7 - NetworkFailedToCreate (hard infra error) +// 8 - CityCodeMismatch / AmbiguousServingLocation (the deployment is on a cell +// that cannot serve it; nothing the user does clears it, and no other +// blocker is worth reporting until it is fixed) func wdBlockingReasonPriority(reason string) int { switch reason { case computev1alpha.WorkloadDeploymentReasonInstancesProvisioning: @@ -589,42 +622,108 @@ func wdBlockingReasonPriority(reason string) int { return 6 case reasonNetworkFailedToCreate: return 7 + case computev1alpha.WorkloadDeploymentReasonCityCodeMismatch, + computev1alpha.WorkloadDeploymentReasonAmbiguousServingLocation: + return 8 default: return 0 } } -// resolveLocation returns the Location matching the deployment's city code, or -// nil when the city has no Location yet. It is reported to users on the -// Available condition and persisted to status so instances carry it, but -// nothing is gated on it: interface claims are served by the control plane they -// are created in, which is already location scoped. +// servingLocationResult is what the cell was able to say about the location it +// serves, expressed the way the deployment needs it. +type servingLocationResult struct { + // reference is the location to stamp on the deployment and its instances. It + // is nil whenever the cell's answer is missing or unusable. + reference *networkingv1alpha.LocationReference + + // servingLocation is the single ServingLocation the cell was delivered, or + // nil when it was delivered none or more than one. + servingLocation *networkingv1alpha.ServingLocation + + // reason and message are the Available condition the deployment should + // report while the location is unusable. Both are empty once it is usable. + reason string + message string + + // blocked distinguishes a cell that contradicts the deployment's placement + // from one that simply has not been identified yet. Only the former holds + // instances back; an unidentified cell must never stop a workload from + // running. + blocked bool +} + +// resolveLocation reads the location the cell serves. +// +// A cell cannot tell where it is on its own; the platform delivers it exactly +// one ServingLocation naming the place it sits in. Anything other than exactly +// one is reported rather than guessed at, per the ServingLocation contract. func (r *WorkloadDeploymentReconciler) resolveLocation( ctx context.Context, c client.Client, - deployment *computev1alpha.WorkloadDeployment, -) (*networkingv1alpha.LocationReference, error) { - // With Karmada propagation the WorkloadDeployment lands in the cluster that - // serves the requested city, so the Location object for that city must exist - // locally. - var locationList networkingv1alpha.LocationList - if err := c.List(ctx, &locationList); err != nil { - return nil, fmt.Errorf("failed to list locations: %w", err) - } - - for _, location := range locationList.Items { - if cityCode, ok := location.Spec.Topology["topology.datum.net/city-code"]; ok && cityCode == deployment.Spec.CityCode { - return &networkingv1alpha.LocationReference{ - Name: location.Name, - Namespace: location.Namespace, - }, nil +) (servingLocationResult, error) { + var servingLocations networkingv1alpha.ServingLocationList + if err := c.List(ctx, &servingLocations); err != nil { + return servingLocationResult{}, fmt.Errorf("failed to list serving locations: %w", err) + } + + if len(servingLocations.Items) > 1 { + names := make([]string, 0, len(servingLocations.Items)) + for _, servingLocation := range servingLocations.Items { + names = append(names, servingLocation.Name) } + slices.Sort(names) + + return servingLocationResult{ + reason: computev1alpha.WorkloadDeploymentReasonAmbiguousServingLocation, + message: fmt.Sprintf("This cell has been given %d locations to serve (%s) and will not guess between them", + len(names), strings.Join(names, ", ")), + blocked: true, + }, nil + } + + if len(servingLocations.Items) == 0 { + // Not an error: a cell that has not been identified yet still runs + // workloads, it just cannot tell them where they are. + log.FromContext(ctx).V(1).Info("cell has no serving location, waiting") + + return servingLocationResult{ + reason: computev1alpha.WorkloadDeploymentReasonNoMatchingLocation, + message: fmt.Sprintf("This cell has not been told which location it serves; it needs the %s cluster label, or its location has not reached it yet", + networkingv1alpha.ServingLocationTopologyLabel), + }, nil + } + + servingLocation := &servingLocations.Items[0] + + // A ServingLocation takes the name of the Location it was copied from, and + // Location is cluster scoped, so the reference carries a name and no + // namespace. + return servingLocationResult{ + reference: &networkingv1alpha.LocationReference{Name: servingLocation.Name}, + servingLocation: servingLocation, + }, nil +} + +// evaluate checks the resolved location against where the deployment asked to +// run. A deployment that reaches a cell serving another city was misplaced by +// the propagation layer, so it is reported as a fault rather than silently run +// in the wrong city. +func (s *servingLocationResult) evaluate(deployment *computev1alpha.WorkloadDeployment) { + if s.servingLocation == nil { + return + } + + cityCode := s.servingLocation.CityCode() + if cityCode == deployment.Spec.CityCode { + return } - // Surfaced to users via the Available condition (NoMatchingLocation); the log - // is debug-level detail only. - log.FromContext(ctx).V(1).Info("no location found for city code, waiting", "cityCode", deployment.Spec.CityCode) - return nil, nil + s.reference = nil + s.reason = computev1alpha.WorkloadDeploymentReasonCityCodeMismatch + s.message = fmt.Sprintf("Deployment asked for city %q but this cell serves %q; it was delivered to the wrong cell", + deployment.Spec.CityCode, cityCode) + s.blocked = true } // reconcileNetworkInterfaceClaims ensures one NetworkInterfaceClaim exists per @@ -784,14 +883,13 @@ func (r *WorkloadDeploymentReconciler) SetupWithManager(mgr mcmanager.Manager, o return enqueueWorkloadDeploymentForClaim(ctx, cl.GetClient(), clusterName, o) }) }). - // A deployment whose city has no Location yet waits without any other - // wake-up event, and the reconciler does not poll. Watching Locations - // re-reconciles the waiting deployments when their city's Location - // appears (or its topology changes) so Status.Location is filled in. - Watches(&networkingv1alpha.Location{}, func(clusterName multicluster.ClusterName, cl cluster.Cluster) handler.TypedEventHandler[client.Object, mcreconcile.Request] { - return handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []mcreconcile.Request { - location := o.(*networkingv1alpha.Location) - return enqueueWorkloadDeploymentsForLocation(ctx, cl.GetClient(), clusterName, location) + // A deployment on a cell that does not yet know its own location waits + // without any other wake-up event, and the reconciler does not poll. + // Watching ServingLocations re-reconciles those deployments as soon as + // the cell learns where it is, so Status.Location is filled in. + Watches(&networkingv1alpha.ServingLocation{}, func(clusterName multicluster.ClusterName, cl cluster.Cluster) handler.TypedEventHandler[client.Object, mcreconcile.Request] { + return handler.TypedEnqueueRequestsFromMapFunc(func(ctx context.Context, _ client.Object) []mcreconcile.Request { + return enqueueWorkloadDeploymentsForServingLocation(ctx, cl.GetClient(), clusterName) }) }) } @@ -836,20 +934,16 @@ func enqueueWorkloadDeploymentForClaim(ctx context.Context, c client.Client, clu } } -// enqueueWorkloadDeploymentsForLocation maps a Location to the -// WorkloadDeployments that target its city, via the deploymentCityCodeIndex. -func enqueueWorkloadDeploymentsForLocation(ctx context.Context, c client.Client, clusterName multicluster.ClusterName, location *networkingv1alpha.Location) []mcreconcile.Request { +// enqueueWorkloadDeploymentsForServingLocation maps a ServingLocation change to +// every WorkloadDeployment on the cell. A cell's identity applies to all of +// them: it decides the location they are stamped with, and whether they are on +// the right cell at all. There is at most one ServingLocation per cell, so this +// fans out once per delivery, not per object. +func enqueueWorkloadDeploymentsForServingLocation(ctx context.Context, c client.Client, clusterName multicluster.ClusterName) []mcreconcile.Request { logger := log.FromContext(ctx) - cityCode, ok := location.Spec.Topology["topology.datum.net/city-code"] - if !ok { - return nil - } - var workloadDeployments computev1alpha.WorkloadDeploymentList - if err := c.List(ctx, &workloadDeployments, client.MatchingFields{ - deploymentCityCodeIndex: cityCode, - }); err != nil { + if err := c.List(ctx, &workloadDeployments); err != nil { logger.Error(err, "failed to list workload deployments") return nil } diff --git a/internal/controller/workloaddeployment_controller_test.go b/internal/controller/workloaddeployment_controller_test.go index d8944a3e..c19dee2b 100644 --- a/internal/controller/workloaddeployment_controller_test.go +++ b/internal/controller/workloaddeployment_controller_test.go @@ -900,7 +900,7 @@ func TestWDAvailableCondition_ReferencedDataSourceNotFound(t *testing.T) { deployment := makeWDForAvailTest(gen, metav1.ConditionFalse, computev1alpha.ReferencedDataReasonSourceNotFound, msg) - cond := selectWDBlockingCondition(deployment, true, true, 0, 1, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 1, replicas, desiredReplicas) assert.Equal(t, computev1alpha.WorkloadDeploymentAvailable, cond.Type) assert.Equal(t, metav1.ConditionFalse, cond.Status) @@ -919,7 +919,7 @@ func TestWDAvailableCondition_QuotaNotGranted(t *testing.T) { ) deployment := makeWDForAvailTest(gen, metav1.ConditionTrue, computev1alpha.ReferencedDataReasonReady, "all present") - cond := selectWDBlockingCondition(deployment, true, true, 2, 0, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 2, 0, replicas, desiredReplicas) assert.Equal(t, metav1.ConditionFalse, cond.Status) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonQuotaNotGranted, cond.Reason) @@ -940,7 +940,7 @@ func TestWDAvailableCondition_ReferencedDataWinsOverQuota(t *testing.T) { computev1alpha.ReferencedDataReasonSourceNotFound, `ConfigMap "X" not found in namespace "default"`) - cond := selectWDBlockingCondition(deployment, true, true, 1, 1, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 1, 1, replicas, desiredReplicas) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, cond.Reason, "ReferencedDataNotReady (priority 4) must beat QuotaNotGranted (priority 3)") @@ -961,7 +961,7 @@ func TestWDAvailableCondition_NetworkProvisioningVsReferencedData(t *testing.T) computev1alpha.ReferencedDataReasonSourceNotFound, `ConfigMap "X" not found`) - cond := selectWDBlockingCondition(deployment, false /* !networkReady */, true, 0, 1, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, false /* !networkReady */, resolvedTestLocation(), 0, 1, replicas, desiredReplicas) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, cond.Reason, "ReferencedDataNotReady (priority 4) must beat NetworkProvisioning (priority 2)") @@ -1004,7 +1004,7 @@ func TestWDAvailableCondition_ObservedGeneration(t *testing.T) { const gen = int64(42) deployment := makeWDForAvailTest(gen, "", "", "") - cond := selectWDBlockingCondition(deployment, true, true, 0, 0, 0, 1) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 0, 0, 1) assert.Equal(t, gen, cond.ObservedGeneration, "ObservedGeneration must match deployment generation") // Verify the condition is also reachable via apimeta.FindStatusCondition (field @@ -1062,7 +1062,7 @@ func TestWDAvailableCondition_AnnotationSourceNotFound(t *testing.T) { ) deployment := makeWDWithAnnotation(gen, annot) - cond := selectWDBlockingCondition(deployment, true, true, 0, 1, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 1, replicas, desiredReplicas) assert.Equal(t, computev1alpha.WorkloadDeploymentAvailable, cond.Type) assert.Equal(t, metav1.ConditionFalse, cond.Status) @@ -1100,7 +1100,7 @@ func TestWDAvailableCondition_AnnotationAndConditionBothPresent(t *testing.T) { }, } - cond := selectWDBlockingCondition(deployment, true, true, 0, 1, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 1, replicas, desiredReplicas) assert.Equal(t, metav1.ConditionFalse, cond.Status) // Both paths arrive at the same terminal reason; the winner is stable regardless @@ -1125,7 +1125,7 @@ func TestWDAvailableCondition_AnnotationWinsOverQuota(t *testing.T) { deployment := makeWDWithAnnotation(gen, annot) // quotaBlockedReplicas=1 would normally surface QuotaNotGranted (priority 3). - cond := selectWDBlockingCondition(deployment, true, true, 1, 0, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 1, 0, replicas, desiredReplicas) assert.Equal(t, computev1alpha.ReferencedDataReasonSourceNotFound, cond.Reason, "SourceNotFound (priority 5) must beat QuotaNotGranted (priority 3)") @@ -1143,7 +1143,7 @@ func TestWDAvailableCondition_NoAnnotationPropagationLag(t *testing.T) { // No annotation, no ReferencedDataReady condition: companions still propagating. deployment := makeWDForAvailTest(gen, "", "", "") - cond := selectWDBlockingCondition(deployment, true, true, 0, 1, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 1, replicas, desiredReplicas) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonReferencedDataNotReady, cond.Reason, "propagation-lag path must still fire when annotation is absent") @@ -1160,7 +1160,7 @@ func TestWDAvailableCondition_AnnotationEmptyString(t *testing.T) { ) deployment := makeWDWithAnnotation(gen, "") - cond := selectWDBlockingCondition(deployment, true, true, 0, 0, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 0, replicas, desiredReplicas) // No real blockers; falls through to InstancesProvisioning. assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason) @@ -1178,7 +1178,7 @@ func TestWDAvailableCondition_AnnotationMalformedJSON(t *testing.T) { deployment := makeWDWithAnnotation(gen, "not-valid-json{{") // Should not panic; malformed annotation is skipped. - cond := selectWDBlockingCondition(deployment, true, true, 0, 0, replicas, desiredReplicas) + cond := selectWDBlockingCondition(deployment, true, resolvedTestLocation(), 0, 0, replicas, desiredReplicas) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason, "malformed annotation must be silently ignored; fallback to InstancesProvisioning") diff --git a/internal/controller/workloaddeployment_federator.go b/internal/controller/workloaddeployment_federator.go index 7e8b05d9..ac384497 100644 --- a/internal/controller/workloaddeployment_federator.go +++ b/internal/controller/workloaddeployment_federator.go @@ -30,6 +30,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" "go.miloapis.com/milo/pkg/downstreamclient" milosource "go.miloapis.com/milo/pkg/multicluster-runtime/source" ) @@ -45,7 +46,7 @@ const ( // and is used by PropagationPolicy selectors to route them to the correct // POP-cell clusters. Downstream Cluster objects are expected to carry this // label with their city-code value. - cityCodeLabel = "topology.datum.net/city-code" + cityCodeLabel = networkingv1alpha.TopologyCityCodeKey kindWorkloadDeployment = "WorkloadDeployment" ) diff --git a/internal/controller/workloaddeployment_location_test.go b/internal/controller/workloaddeployment_location_test.go index 5337824c..20e5739f 100644 --- a/internal/controller/workloaddeployment_location_test.go +++ b/internal/controller/workloaddeployment_location_test.go @@ -27,20 +27,13 @@ import ( const ( // locTestCityCode / locTestOtherCityCode: deployments under test target - // locTestCityCode; locTestOtherCityCode identifies a decoy Location that - // must never match. + // locTestCityCode; locTestOtherCityCode identifies the city a mis-delivered + // cell serves. locTestCityCode = "DFW" locTestOtherCityCode = "ORD" - // locTestNamespace mirrors where Location objects live in real clusters. - locTestNamespace = "networking-system" - // locTestWDNamespace is the namespace of the deployments under test. locTestWDNamespace = "default" - - // locTestTopologyKey is the production topology key that carries a - // Location's city code. - locTestTopologyKey = "topology.datum.net/city-code" ) // newNetworkingScheme returns a scheme with compute + networkingv1alpha types. @@ -51,89 +44,146 @@ func newNetworkingScheme() *runtime.Scheme { return s } -// newTestLocation builds a Location fixture shaped like production: the city -// code is carried in Spec.Topology under the topology.datum.net/city-code key. -func newTestLocation(name, cityCode string) *networkingv1alpha.Location { - return &networkingv1alpha.Location{ - ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: locTestNamespace}, - Spec: networkingv1alpha.LocationSpec{ - Topology: map[string]string{locTestTopologyKey: cityCode}, +// newTestServingLocation builds a ServingLocation fixture shaped like the one a +// cell is delivered: cluster scoped, and carrying its city under the +// topology.datum.net/city-code key. +func newTestServingLocation(name, cityCode string) *networkingv1alpha.ServingLocation { + return &networkingv1alpha.ServingLocation{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: networkingv1alpha.ServingLocationSpec{ + Topology: map[string]string{networkingv1alpha.TopologyCityCodeKey: cityCode}, }, } } -// TestResolveLocation_PersistsLocation_WhenLocationFound verifies that when a -// Location object matching the deployment's city code exists in the cluster, the -// resolved LocationReference is returned and can be persisted to -// deployment.Status.Location. -func TestResolveLocation_PersistsLocation_WhenLocationFound(t *testing.T) { - t.Parallel() - - const locationName = "loc-dfw-1" - - location := newTestLocation(locationName, locTestCityCode) - - s := newNetworkingScheme() - cl := fake.NewClientBuilder().WithScheme(s).WithObjects(location).Build() +// resolvedTestLocation is the result of a cell that knows where it is and +// serves the city the deployment asked for. +func resolvedTestLocation() servingLocationResult { + return servingLocationResult{ + reference: &networkingv1alpha.LocationReference{Name: "loc-dfw-1"}, + } +} - deployment := &computev1alpha.WorkloadDeployment{ - ObjectMeta: metav1.ObjectMeta{Name: "test-wd", Namespace: locTestWDNamespace}, +func newLocationTestDeployment(name string) *computev1alpha.WorkloadDeployment { + return &computev1alpha.WorkloadDeployment{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: locTestWDNamespace}, Spec: computev1alpha.WorkloadDeploymentSpec{ CityCode: locTestCityCode, }, } +} - r := &WorkloadDeploymentReconciler{} - resolvedLocation, err := r.resolveLocation(context.Background(), cl, deployment) +// TestResolveLocation_ExactlyOneServingLocation verifies the ordinary case: the +// single ServingLocation delivered to the cell resolves to a LocationReference +// naming it, with no namespace (Location is cluster scoped). +func TestResolveLocation_ExactlyOneServingLocation(t *testing.T) { + t.Parallel() + + const locationName = "loc-dfw-1" + cl := fake.NewClientBuilder(). + WithScheme(newNetworkingScheme()). + WithObjects(newTestServingLocation(locationName, locTestCityCode)). + Build() + + deployment := newLocationTestDeployment("test-wd") + + r := &WorkloadDeploymentReconciler{} + result, err := r.resolveLocation(context.Background(), cl) require.NoError(t, err) - require.NotNil(t, resolvedLocation, - "resolved location must be non-nil when a matching Location object exists") - assert.Equal(t, locationName, resolvedLocation.Name) - assert.Equal(t, locTestNamespace, resolvedLocation.Namespace) - - // Simulate what the Reconcile loop does: persist resolvedLocation to Status. - deployment.Status.Location = resolvedLocation - assert.Equal(t, locationName, deployment.Status.Location.Name, - "Status.Location.Name must match the resolved Location object name") + result.evaluate(deployment) + + require.NotNil(t, result.reference, + "the cell's single serving location must resolve") + assert.Equal(t, locationName, result.reference.Name) + assert.Empty(t, result.reason, "a resolved location reports no blocking reason") + assert.False(t, result.blocked) } -// TestResolveLocation_ReturnsNilLocation_WhenNoLocationFound verifies that when -// no Location object in the cluster matches the deployment's city code, the -// resolver returns (nil, nil) — no error and no resolved location. The caller -// must treat a nil location as best-effort and must NOT block instance creation. -func TestResolveLocation_ReturnsNilLocation_WhenNoLocationFound(t *testing.T) { +// TestResolveLocation_NoServingLocation_IsNonGating verifies that a cell which +// has not been told where it is does not block the deployment: no location is +// resolved, nothing is marked blocked, and the reason names what is missing. +func TestResolveLocation_NoServingLocation_IsNonGating(t *testing.T) { t.Parallel() - s := newNetworkingScheme() - // Cluster has a Location for a DIFFERENT city code. - otherLocation := newTestLocation("loc-ord-1", locTestOtherCityCode) - cl := fake.NewClientBuilder().WithScheme(s).WithObjects(otherLocation).Build() + cl := fake.NewClientBuilder().WithScheme(newNetworkingScheme()).Build() - deployment := &computev1alpha.WorkloadDeployment{ - ObjectMeta: metav1.ObjectMeta{Name: "test-wd", Namespace: locTestWDNamespace}, - Spec: computev1alpha.WorkloadDeploymentSpec{ - CityCode: locTestCityCode, // no matching Location - }, - } + deployment := newLocationTestDeployment("test-wd") + + r := &WorkloadDeploymentReconciler{} + result, err := r.resolveLocation(context.Background(), cl) + require.NoError(t, err, "an unidentified cell must not surface as an error") + result.evaluate(deployment) + + assert.Nil(t, result.reference) + assert.False(t, result.blocked, + "a cell that has not been identified yet must never hold instances back") + assert.Equal(t, computev1alpha.WorkloadDeploymentReasonNoMatchingLocation, result.reason) + assert.Contains(t, result.message, networkingv1alpha.ServingLocationTopologyLabel, + "the message must name the cluster label that fixes it") + assert.Nil(t, deployment.Status.Location, + "Status.Location must be left alone when nothing resolved") +} + +// TestResolveLocation_MultipleServingLocations_RefusesToGuess verifies the +// ServingLocation contract: two or more delivered locations means the cell +// cannot tell which one it serves, so it picks neither and blocks. +func TestResolveLocation_MultipleServingLocations_RefusesToGuess(t *testing.T) { + t.Parallel() + + cl := fake.NewClientBuilder(). + WithScheme(newNetworkingScheme()). + WithObjects( + newTestServingLocation("loc-dfw-1", locTestCityCode), + newTestServingLocation("loc-ord-1", locTestOtherCityCode), + ). + Build() + + deployment := newLocationTestDeployment("test-wd") r := &WorkloadDeploymentReconciler{} - resolvedLocation, err := r.resolveLocation(context.Background(), cl, deployment) + result, err := r.resolveLocation(context.Background(), cl) + require.NoError(t, err) + result.evaluate(deployment) + + assert.Nil(t, result.reference, + "an ambiguous cell must not resolve to either candidate") + assert.True(t, result.blocked) + assert.Equal(t, computev1alpha.WorkloadDeploymentReasonAmbiguousServingLocation, result.reason) + assert.Contains(t, result.message, "loc-dfw-1") + assert.Contains(t, result.message, "loc-ord-1") +} - require.NoError(t, err, "missing location must not cause an error") - assert.Nil(t, resolvedLocation, - "resolved location must be nil when no matching Location object exists") +// TestResolveLocation_CityCodeMismatch verifies that a deployment which reaches +// a cell serving another city is reported as a placement fault rather than +// quietly stamped with the wrong location. +func TestResolveLocation_CityCodeMismatch(t *testing.T) { + t.Parallel() - // Status.Location remains nil — callers must not update it in this case. - // Confirm the deployment's Status.Location is unaffected (nil → nil). - assert.Nil(t, deployment.Status.Location, - "Status.Location must remain nil when no Location matches the city code") + cl := fake.NewClientBuilder(). + WithScheme(newNetworkingScheme()). + WithObjects(newTestServingLocation("loc-ord-1", locTestOtherCityCode)). + Build() + + deployment := newLocationTestDeployment("test-wd") + + r := &WorkloadDeploymentReconciler{} + result, err := r.resolveLocation(context.Background(), cl) + require.NoError(t, err) + result.evaluate(deployment) + + assert.Nil(t, result.reference, + "the wrong cell's location must never be stamped on the deployment") + assert.True(t, result.blocked, "a misplaced deployment must not proceed silently") + assert.Equal(t, computev1alpha.WorkloadDeploymentReasonCityCodeMismatch, result.reason) + assert.Contains(t, result.message, locTestCityCode) + assert.Contains(t, result.message, locTestOtherCityCode) } // newLocationTestWDReconciler builds a WorkloadDeploymentReconciler with // networking enabled, wired to a fake cluster, with the controller finalizer // pre-registered the same way SetupWithManager does. Networking must be enabled -// so Reconcile exercises Location resolution. +// so Reconcile exercises location resolution. func newLocationTestWDReconciler(cl client.Client) *WorkloadDeploymentReconciler { r := &WorkloadDeploymentReconciler{ mgr: newFakeMCManager(testCluster, newFakeCluster(cl)), @@ -147,19 +197,15 @@ func newLocationTestWDReconciler(cl client.Client) *WorkloadDeploymentReconciler return r } -// TestWorkloadDeploymentReconcile_NoMatchingLocation_SetsCondition verifies the -// user-visible surface while a deployment waits for its city's Location: the -// Available condition must name the unresolved city (reason NoMatchingLocation), -// and once a matching Location appears the next reconcile must replace that -// reason — the unresolved-city signal must not outlive its cause. -func TestWorkloadDeploymentReconcile_NoMatchingLocation_SetsCondition(t *testing.T) { - t.Parallel() - - deployment := &computev1alpha.WorkloadDeployment{ +// newLocationTestReconcilableWD builds a deployment shaped the way Reconcile +// expects to find one already running: finalized, with replicas and the +// defaulted management policy. +func newLocationTestReconcilableWD(name string) *computev1alpha.WorkloadDeployment { + return &computev1alpha.WorkloadDeployment{ ObjectMeta: metav1.ObjectMeta{ - Name: "location-test-wd", + Name: name, Namespace: locTestWDNamespace, - UID: "location-test-wd-uid", + UID: types.UID(name + "-uid"), // Pre-set the finalizer so Reconcile proceeds past the finalizer-add // branch. Finalizers: []string{workloadControllerFinalizer}, @@ -176,13 +222,14 @@ func TestWorkloadDeploymentReconcile_NoMatchingLocation_SetsCondition(t *testing }, }, } +} - // An instance shaped the way the instance-control strategy creates it: - // ordinal name, controller labels, and the scheduling gates stamped at - // creation. Pre-seeding it (with a CreationTimestamp, which the fake client - // does not stamp on Create) keeps the strategy in its wait path so the test - // exercises only the condition transitions. - instance := &computev1alpha.Instance{ +// newLocationTestInstance builds an instance shaped the way the instance-control +// strategy creates it: ordinal name, deployment UID label, and the scheduling +// gates stamped at creation. The CreationTimestamp (which the fake client does +// not stamp on Create) keeps the strategy in its wait path. +func newLocationTestInstance(deployment *computev1alpha.WorkloadDeployment) *computev1alpha.Instance { + return &computev1alpha.Instance{ ObjectMeta: metav1.ObjectMeta{ Name: deployment.Name + "-0", Namespace: deployment.Namespace, @@ -200,23 +247,35 @@ func TestWorkloadDeploymentReconcile_NoMatchingLocation_SetsCondition(t *testing }, }, } +} - // The only Location in the cluster serves a different city. - otherLocation := newTestLocation("loc-ord-1", locTestOtherCityCode) +func locationTestRequest(deployment *computev1alpha.WorkloadDeployment) mcreconcile.Request { + return mcreconcile.Request{ + ClusterName: testCluster, + Request: ctrl.Request{ + NamespacedName: types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, + }, + } +} + +// TestWorkloadDeploymentReconcile_UnidentifiedCell_SetsCondition verifies the +// user-visible surface while a cell has not learned where it is: the Available +// condition explains what is missing, and once the cell is told, the next +// reconcile resolves the location and replaces the reason — the waiting signal +// must not outlive its cause. +func TestWorkloadDeploymentReconcile_UnidentifiedCell_SetsCondition(t *testing.T) { + t.Parallel() + + deployment := newLocationTestReconcilableWD("location-test-wd") + instance := newLocationTestInstance(deployment) cl := fake.NewClientBuilder(). WithScheme(newNetworkingScheme()). - WithObjects(deployment, instance, otherLocation). + WithObjects(deployment, instance). WithStatusSubresource(deployment). Build() r := newLocationTestWDReconciler(cl) - - req := mcreconcile.Request{ - ClusterName: testCluster, - Request: ctrl.Request{ - NamespacedName: types.NamespacedName{Name: deployment.Name, Namespace: deployment.Namespace}, - }, - } + req := locationTestRequest(deployment) _, err := r.Reconcile(context.Background(), req) require.NoError(t, err) @@ -225,17 +284,17 @@ func TestWorkloadDeploymentReconcile_NoMatchingLocation_SetsCondition(t *testing require.NoError(t, cl.Get(context.Background(), req.NamespacedName, &updated)) cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.WorkloadDeploymentAvailable) - require.NotNil(t, cond, "Available must be set while the city has no Location") + require.NotNil(t, cond, "Available must be set while the cell has no location") assert.Equal(t, metav1.ConditionFalse, cond.Status) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonNoMatchingLocation, cond.Reason) - assert.Contains(t, cond.Message, locTestCityCode, - "the condition message must name the unresolved city code") + assert.Contains(t, cond.Message, networkingv1alpha.ServingLocationTopologyLabel, + "the condition message must name the label that identifies the cell") assert.Nil(t, updated.Status.Location) - // Provision the city's Location; the next reconcile resolves it and must - // replace the NoMatchingLocation reason. - matchingLocation := newTestLocation("loc-dfw-2", locTestCityCode) - require.NoError(t, cl.Create(context.Background(), matchingLocation)) + // Deliver the cell's location; the next reconcile resolves it and must + // replace the waiting reason. + servingLocation := newTestServingLocation("loc-dfw-2", locTestCityCode) + require.NoError(t, cl.Create(context.Background(), servingLocation)) _, err = r.Reconcile(context.Background(), req) require.NoError(t, err) @@ -244,46 +303,105 @@ func TestWorkloadDeploymentReconcile_NoMatchingLocation_SetsCondition(t *testing cond = apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.WorkloadDeploymentAvailable) require.NotNil(t, cond) assert.Equal(t, computev1alpha.WorkloadDeploymentReasonInstancesProvisioning, cond.Reason, - "the unresolved-city reason must give way once the Location resolves") + "the waiting reason must give way once the cell knows where it is") require.NotNil(t, updated.Status.Location) - assert.Equal(t, matchingLocation.Name, updated.Status.Location.Name) + assert.Equal(t, servingLocation.Name, updated.Status.Location.Name) } -// TestEnqueueWorkloadDeploymentsForLocation verifies the Location watch mapping: -// a Location event must enqueue exactly the WorkloadDeployments whose CityCode -// matches the Location's topology (via deploymentCityCodeIndex), and a Location -// without a city code in its topology must map to nothing. -func TestEnqueueWorkloadDeploymentsForLocation(t *testing.T) { +// TestWorkloadDeploymentReconcile_CityCodeMismatch_HoldsInstances verifies that a +// deployment delivered to the wrong cell reports the fault and is kept out of +// service: its instances keep the Network scheduling gate rather than booting in +// a city the user did not ask for. +func TestWorkloadDeploymentReconcile_CityCodeMismatch_HoldsInstances(t *testing.T) { t.Parallel() - wdDFW := &computev1alpha.WorkloadDeployment{ - ObjectMeta: metav1.ObjectMeta{Name: "wd-dfw", Namespace: locTestWDNamespace}, - Spec: computev1alpha.WorkloadDeploymentSpec{CityCode: locTestCityCode}, - } - wdORD := &computev1alpha.WorkloadDeployment{ - ObjectMeta: metav1.ObjectMeta{Name: "wd-ord", Namespace: locTestWDNamespace}, - Spec: computev1alpha.WorkloadDeploymentSpec{CityCode: locTestOtherCityCode}, - } + deployment := newLocationTestReconcilableWD("mismatch-test-wd") + instance := newLocationTestInstance(deployment) + + cl := fake.NewClientBuilder(). + WithScheme(newNetworkingScheme()). + WithObjects(deployment, instance, newTestServingLocation("loc-ord-1", locTestOtherCityCode)). + WithStatusSubresource(deployment). + Build() + r := newLocationTestWDReconciler(cl) + req := locationTestRequest(deployment) + + _, err := r.Reconcile(context.Background(), req) + require.NoError(t, err) + + var updated computev1alpha.WorkloadDeployment + require.NoError(t, cl.Get(context.Background(), req.NamespacedName, &updated)) + + cond := apimeta.FindStatusCondition(updated.Status.Conditions, computev1alpha.WorkloadDeploymentAvailable) + require.NotNil(t, cond) + assert.Equal(t, computev1alpha.WorkloadDeploymentReasonCityCodeMismatch, cond.Reason, + "a misplaced deployment must report the placement fault over any other blocker") + assert.Nil(t, updated.Status.Location, + "the wrong cell's location must never be written to status") + + var updatedInstance computev1alpha.Instance + require.NoError(t, cl.Get(context.Background(), types.NamespacedName{ + Name: instance.Name, Namespace: instance.Namespace, + }, &updatedInstance)) + require.NotNil(t, updatedInstance.Spec.Controller) + assert.Contains(t, updatedInstance.Spec.Controller.SchedulingGates, + computev1alpha.SchedulingGate{Name: instancecontrol.NetworkSchedulingGate.String()}, + "the Network gate must stay held while the deployment is on the wrong cell") +} + +// TestWorkloadDeploymentReconcile_BackfillsInstanceLocation verifies that an +// instance created before the cell knew its location does not stay without one: +// the reconcile that resolves the location also stamps it on the existing +// instance. +func TestWorkloadDeploymentReconcile_BackfillsInstanceLocation(t *testing.T) { + t.Parallel() + + deployment := newLocationTestReconcilableWD("backfill-test-wd") + instance := newLocationTestInstance(deployment) + require.Nil(t, instance.Spec.Location, "the fixture must start without a location") + + servingLocation := newTestServingLocation("loc-dfw-1", locTestCityCode) + + cl := fake.NewClientBuilder(). + WithScheme(newNetworkingScheme()). + WithObjects(deployment, instance, servingLocation). + WithStatusSubresource(deployment). + Build() + r := newLocationTestWDReconciler(cl) + + _, err := r.Reconcile(context.Background(), locationTestRequest(deployment)) + require.NoError(t, err) + + var updatedInstance computev1alpha.Instance + require.NoError(t, cl.Get(context.Background(), types.NamespacedName{ + Name: instance.Name, Namespace: instance.Namespace, + }, &updatedInstance)) + + require.NotNil(t, updatedInstance.Spec.Location, + "an instance predating the cell's location must be backfilled") + assert.Equal(t, servingLocation.Name, updatedInstance.Spec.Location.Name) +} + +// TestEnqueueWorkloadDeploymentsForServingLocation verifies the ServingLocation +// watch mapping: the cell's identity applies to every deployment on it, so all +// of them are enqueued regardless of the city they target. +func TestEnqueueWorkloadDeploymentsForServingLocation(t *testing.T) { + t.Parallel() + + wdDFW := newLocationTestDeployment("wd-dfw") + wdORD := newLocationTestDeployment("wd-ord") + wdORD.Spec.CityCode = locTestOtherCityCode cl := fake.NewClientBuilder(). WithScheme(newNetworkingScheme()). - WithIndex(&computev1alpha.WorkloadDeployment{}, deploymentCityCodeIndex, deploymentCityCodeIndexFunc). WithObjects(wdDFW, wdORD). Build() - location := newTestLocation("loc-dfw-1", locTestCityCode) + requests := enqueueWorkloadDeploymentsForServingLocation(context.Background(), cl, testCluster) + require.Len(t, requests, 2) - requests := enqueueWorkloadDeploymentsForLocation(context.Background(), cl, testCluster, location) - require.Len(t, requests, 1, "only deployments whose CityCode matches the Location must be enqueued") - assert.Equal(t, wdDFW.Name, requests[0].Name) + names := []string{requests[0].Name, requests[1].Name} + assert.ElementsMatch(t, []string{wdDFW.Name, wdORD.Name}, names) assert.Equal(t, locTestWDNamespace, requests[0].Namespace) assert.Equal(t, multicluster.ClusterName(testCluster), requests[0].ClusterName) - - // A Location without a city code in its topology identifies no city, so no - // deployment can match it. - noCityLocation := &networkingv1alpha.Location{ - ObjectMeta: metav1.ObjectMeta{Name: "loc-no-city", Namespace: locTestNamespace}, - Spec: networkingv1alpha.LocationSpec{Topology: map[string]string{}}, - } - assert.Empty(t, enqueueWorkloadDeploymentsForLocation(context.Background(), cl, testCluster, noCityLocation)) } diff --git a/internal/webhook/v1alpha/workload_webhook.go b/internal/webhook/v1alpha/workload_webhook.go index b8cffecb..e8926b60 100644 --- a/internal/webhook/v1alpha/workload_webhook.go +++ b/internal/webhook/v1alpha/workload_webhook.go @@ -96,7 +96,7 @@ func (r *workloadWebhook) ValidateCreate(ctx context.Context, workload *computev validCityCodes := sets.Set[string]{} for _, location := range locations.Items { - cityCode, ok := location.Spec.Topology["topology.datum.net/city-code"] + cityCode, ok := location.Spec.Topology[networkingv1alpha.TopologyCityCodeKey] if ok { validCityCodes.Insert(cityCode) } @@ -141,7 +141,7 @@ func (r *workloadWebhook) ValidateUpdate(ctx context.Context, _ *computev1alpha. validCityCodes := sets.Set[string]{} for _, location := range locations.Items { - cityCode, ok := location.Spec.Topology["topology.datum.net/city-code"] + cityCode, ok := location.Spec.Topology[networkingv1alpha.TopologyCityCodeKey] if ok { validCityCodes.Insert(cityCode) }