diff --git a/cmd/main.go b/cmd/main.go index cb59e59e..7dbf0e6f 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -53,6 +53,7 @@ import ( computewebhook "go.datum.net/compute/internal/webhook" computev1alphawebhooks "go.datum.net/compute/internal/webhook/v1alpha" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" infrastructurev1alpha1 "go.miloapis.com/milo/pkg/apis/infrastructure/v1alpha1" quotav1alpha1 "go.miloapis.com/milo/pkg/apis/quota/v1alpha1" resourcemanagerv1alpha1 "go.miloapis.com/milo/pkg/apis/resourcemanager/v1alpha1" @@ -91,6 +92,7 @@ func init() { utilruntime.Must(config.RegisterDefaults(scheme)) utilruntime.Must(computev1alpha.AddToScheme(scheme)) utilruntime.Must(networkingv1alpha.AddToScheme(scheme)) + utilruntime.Must(locationsv1alpha1.AddToScheme(scheme)) utilruntime.Must(quotav1alpha1.AddToScheme(scheme)) utilruntime.Must(karmadapolicyv1alpha1.Install(scheme)) utilruntime.Must(karmadaclusterv1alpha1.Install(scheme)) @@ -350,6 +352,7 @@ func main() { if enableManagementControllers { if err = (&controller.WorkloadReconciler{ NetworkingEnabled: features.FeatureGate.Enabled(features.NetworkingIntegration), + LocationSource: serverConfig.LocationSource, }).SetupWithManager(mgr); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Workload") os.Exit(1) @@ -374,6 +377,7 @@ func main() { } if err = (&controller.WorkloadDeploymentReconciler{ NetworkingEnabled: features.FeatureGate.Enabled(features.NetworkingIntegration), + LocationSource: serverConfig.LocationSource, }).SetupWithManager(mgr, wdOpts); err != nil { setupLog.Error(err, "unable to create controller", "controller", "WorkloadDeployment") os.Exit(1) @@ -457,7 +461,7 @@ func main() { } if serverConfig.WebhookServer != nil { - if err = computev1alphawebhooks.SetupWorkloadWebhookWithManager(mgr); err != nil { + if err = computev1alphawebhooks.SetupWorkloadWebhookWithManager(mgr, serverConfig.LocationSource); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "Workload") os.Exit(1) } @@ -803,6 +807,9 @@ func loadServerConfig(path string) (config.WorkloadOperator, error) { if err := runtime.DecodeInto(codecs.UniversalDecoder(), configData, &serverConfig); err != nil { return serverConfig, fmt.Errorf("unable to decode server config: %w", err) } + if _, err := serverConfig.LocationSource.Resolve(); err != nil { + return serverConfig, fmt.Errorf("invalid server config: %w", err) + } return serverConfig, nil } diff --git a/cmd/main_test.go b/cmd/main_test.go index c0053469..89fc4f30 100644 --- a/cmd/main_test.go +++ b/cmd/main_test.go @@ -3,10 +3,20 @@ package main import ( + "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/apiutil" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" multiclusterproviders "go.miloapis.com/milo/pkg/multicluster-runtime" + + "go.datum.net/compute/internal/locations" ) // TestComputeWatchProviderClaims is the #171 guard: quota enforcement (and thus @@ -21,3 +31,62 @@ func TestComputeWatchProviderClaims(t *testing.T) { assert.False(t, computeWatchProviderClaims(multiclusterproviders.ProviderKind), "non-milo modes disable quota") } + +// TestLoadServerConfig_LocationSource covers the startup guard: a config that +// names an unknown location source fails to load rather than surfacing later +// on every reconcile. +func TestLoadServerConfig_LocationSource(t *testing.T) { + write := func(t *testing.T, body string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.yaml") + require.NoError(t, os.WriteFile(path, []byte(body), 0o600)) + return path + } + + const header = `apiVersion: apiserver.config.datumapis.com/v1alpha1 +kind: WorkloadOperator +metricsServer: + bindAddress: "0" +` + + cfg, err := loadServerConfig(write(t, header)) + require.NoError(t, err) + assert.Equal(t, locations.SourceNetworkServices, cfg.LocationSource) + + cfg, err = loadServerConfig(write(t, header+"locationSource: Locations\n")) + require.NoError(t, err) + assert.Equal(t, locations.SourceLocations, cfg.LocationSource) + + _, err = loadServerConfig(write(t, header+"locationSource: Nonsense\n")) + require.Error(t, err) +} + +// TestSchemeResolvesLocationKinds is the runtime half of the typed locations +// dependency: compiling against the types says nothing about whether a client +// can resolve them. A kind missing here fails every list at runtime with "no +// kind is registered", not at build time. +func TestSchemeResolvesLocationKinds(t *testing.T) { + for _, object := range []client.Object{ + &locationsv1alpha1.Location{}, + &locationsv1alpha1.ServingLocation{}, + &networkingv1alpha.LocationBinding{}, + &networkingv1alpha.ServingLocation{}, + } { + gvk, err := apiutil.GVKForObject(object, scheme) + require.NoErrorf(t, err, "%T must be registered on the manager scheme", object) + assert.NotEmpty(t, gvk.Kind) + } +} + +// TestServingLocationObjectIsRegistered pins the watch the deployment +// reconciler installs: whichever source is selected, the object it watches has +// to be resolvable on the manager scheme. +func TestServingLocationObjectIsRegistered(t *testing.T) { + for _, source := range []locations.Source{"", locations.SourceNetworkServices, locations.SourceLocations} { + object, err := locations.ServingLocationObject(source) + require.NoError(t, err) + + _, err = apiutil.GVKForObject(object, scheme) + require.NoErrorf(t, err, "the watch object for source %q must be registered", source) + } +} diff --git a/config/components/controller_rbac/role.yaml b/config/components/controller_rbac/role.yaml index acedcc22..0d6d6ed3 100644 --- a/config/components/controller_rbac/role.yaml +++ b/config/components/controller_rbac/role.yaml @@ -76,6 +76,15 @@ rules: verbs: - create - patch +- apiGroups: + - locations.miloapis.com + resources: + - locations + - servinglocations + verbs: + - get + - list + - watch - apiGroups: - networking.datumapis.com resources: diff --git a/go.mod b/go.mod index 9247c2b6..006654a3 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,9 @@ require ( // predates the Prepared condition this gate reads. Re-pin to a tagged // release once one carries it. go.datum.net/network-services-operator v0.26.1-0.20260821014231-aceb24b1b569 + // Pinned by pseudo-version to the commit deployed to staging, which is the + // same one network-services-operator pins. The module publishes no tag yet. + go.miloapis.com/locations v0.0.0-20260825185141-507ac2cbd48c go.miloapis.com/milo v0.32.0 golang.org/x/crypto v0.54.0 golang.org/x/sync v0.22.0 diff --git a/go.sum b/go.sum index 7c7406eb..292b7aa6 100644 --- a/go.sum +++ b/go.sum @@ -338,6 +338,8 @@ go.datum.net/datumctl v0.17.1-0.20260710003126-296c2fcbbd67 h1:Mhgt688CeTh2hX7ql go.datum.net/datumctl v0.17.1-0.20260710003126-296c2fcbbd67/go.mod h1:6skEjcE7aT8VPf/HVamA/BB6Dc9IISA6c/DdYKhqWNc= go.datum.net/network-services-operator v0.26.1-0.20260821014231-aceb24b1b569 h1:14vajo15fGGEAmdystQEE261rFH4AqER9s1Vg7POUKo= go.datum.net/network-services-operator v0.26.1-0.20260821014231-aceb24b1b569/go.mod h1:A7JNOuc+e6j/KkUVCcZ7Z2odvf6JFMQlX0Zo1Awj2TY= +go.miloapis.com/locations v0.0.0-20260825185141-507ac2cbd48c h1:+BQirT3wYCgv7H2lEZtAH+dpMiWu9j/Wa+c8s4jJUIA= +go.miloapis.com/locations v0.0.0-20260825185141-507ac2cbd48c/go.mod h1:gzfAfHhSMwl/N68k/uSNYXOKK3IOBJCdXYaBgCE3gdE= 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/cmd/compute/util/client.go b/internal/cmd/compute/util/client.go index feafc7ab..aa6fa536 100644 --- a/internal/cmd/compute/util/client.go +++ b/internal/cmd/compute/util/client.go @@ -7,6 +7,7 @@ import ( computev1alpha "go.datum.net/compute/api/v1alpha" "go.datum.net/datumctl/plugin" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" quotav1alpha1 "go.miloapis.com/milo/pkg/apis/quota/v1alpha1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/client-go/rest" @@ -52,6 +53,9 @@ func NewClient(project string) (client.Client, error) { if err := networkingv1alpha.AddToScheme(scheme); err != nil { return nil, fmt.Errorf("registering networking scheme: %w", err) } + if err := locationsv1alpha1.AddToScheme(scheme); err != nil { + return nil, fmt.Errorf("registering locations scheme: %w", err) + } if err := quotav1alpha1.AddToScheme(scheme); err != nil { return nil, fmt.Errorf("registering quota scheme: %w", err) } @@ -78,6 +82,9 @@ func NewPlatformClient() (client.Client, error) { } scheme := runtime.NewScheme() + if err := locationsv1alpha1.AddToScheme(scheme); err != nil { + return nil, fmt.Errorf("registering locations scheme: %w", err) + } if err := quotav1alpha1.AddToScheme(scheme); err != nil { return nil, fmt.Errorf("registering quota scheme: %w", err) } diff --git a/internal/config/config.go b/internal/config/config.go index 46bebe59..4b832b50 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -19,6 +19,7 @@ import ( metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "go.datum.net/compute/internal/locations" multiclusterproviders "go.miloapis.com/milo/pkg/multicluster-runtime" ) @@ -42,6 +43,19 @@ type WorkloadOperator struct { // ReferencedData configures the ReferencedDataController. ReferencedData ReferencedDataConfig `json:"referencedData,omitempty"` + + // LocationSource names the API group locations are read from. Use + // "NetworkServices" for networking.datumapis.com LocationBindings and + // ServingLocations, or "Locations" for the dedicated locations.miloapis.com + // service. It governs reads only; nothing about what compute writes changes + // with it. Defaults to "NetworkServices". + LocationSource locations.Source `json:"locationSource,omitempty"` +} + +func SetDefaults_WorkloadOperator(obj *WorkloadOperator) { + if obj.LocationSource == "" { + obj.LocationSource = locations.SourceNetworkServices + } } // +k8s:deepcopy-gen=true diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5a7a3cee..61c4ca33 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -7,6 +7,8 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/serializer" + + "go.datum.net/compute/internal/locations" ) func decode(t *testing.T, data string) *WorkloadOperator { @@ -122,3 +124,30 @@ users: t.Error("QuotaRestConfig() = nil, want non-nil when file exists") } } + +// TestLocationSource_DefaultsToNetworkServices is the safety property behind +// the flag: a config that does not set it reads what it reads today. +func TestLocationSource_DefaultsToNetworkServices(t *testing.T) { + cfg := decode(t, ` +apiVersion: apiserver.config.datumapis.com/v1alpha1 +kind: WorkloadOperator +metricsServer: + bindAddress: "0" +`) + if cfg.LocationSource != locations.SourceNetworkServices { + t.Errorf("LocationSource = %q, want %q", cfg.LocationSource, locations.SourceNetworkServices) + } +} + +func TestLocationSource_Explicit(t *testing.T) { + cfg := decode(t, ` +apiVersion: apiserver.config.datumapis.com/v1alpha1 +kind: WorkloadOperator +metricsServer: + bindAddress: "0" +locationSource: Locations +`) + if cfg.LocationSource != locations.SourceLocations { + t.Errorf("LocationSource = %q, want %q", cfg.LocationSource, locations.SourceLocations) + } +} diff --git a/internal/config/zz_generated.defaults.go b/internal/config/zz_generated.defaults.go index fed95896..0f8a363b 100644 --- a/internal/config/zz_generated.defaults.go +++ b/internal/config/zz_generated.defaults.go @@ -18,6 +18,7 @@ func RegisterDefaults(scheme *runtime.Scheme) error { } func SetObjectDefaults_WorkloadOperator(in *WorkloadOperator) { + SetDefaults_WorkloadOperator(in) SetDefaults_MetricsServerConfig(&in.MetricsServer) SetDefaults_TLSConfig(&in.MetricsServer.TLS) if in.WebhookServer != nil { diff --git a/internal/controller/location_source_test.go b/internal/controller/location_source_test.go new file mode 100644 index 00000000..db168712 --- /dev/null +++ b/internal/controller/location_source_test.go @@ -0,0 +1,160 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + computev1alpha "go.datum.net/compute/api/v1alpha" + "go.datum.net/compute/internal/locations" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" +) + +// newLocationsServiceScheme returns the networking scheme with the locations +// service types added, mirroring what the manager registers. +func newLocationsServiceScheme() *runtime.Scheme { + s := newNetworkingScheme() + _ = locationsv1alpha1.AddToScheme(s) + return s +} + +func newLocationsServiceLocation(name, cityCode string) *locationsv1alpha1.Location { + return &locationsv1alpha1.Location{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: locationsv1alpha1.LocationSpec{ + LocationClassRef: locationsv1alpha1.LocationClassReference{Name: "datum-managed"}, + Topology: map[string]string{locations.TopologyCityCodeKey: cityCode}, + }, + } +} + +func newLocationsServiceServingLocation(name, cityCode string) *locationsv1alpha1.ServingLocation { + return &locationsv1alpha1.ServingLocation{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: locationsv1alpha1.ServingLocationSpec{ + Topology: map[string]string{locations.TopologyCityCodeKey: cityCode}, + }, + } +} + +// TestGetDeploymentsForWorkload_LocationsSource verifies that a workload placed +// in a city is deployed there when the city is only known to the locations +// service, which is what the Locations source reads. +func TestGetDeploymentsForWorkload_LocationsSource(t *testing.T) { + t.Parallel() + + workload := &computev1alpha.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Name: rdTestWorkloadName, + Namespace: testDefaultNamespace, + UID: types.UID("workload-uid"), + }, + Spec: computev1alpha.WorkloadSpec{ + Placements: []computev1alpha.WorkloadPlacement{ + { + Name: testDefaultPlacement, + CityCodes: []string{locTestCityCode}, + ScaleSettings: computev1alpha.HorizontalScaleSettings{ + MinReplicas: 1, + }, + }, + }, + }, + } + + cl := fake.NewClientBuilder(). + WithScheme(newLocationsServiceScheme()). + WithObjects(newLocationsServiceLocation("dfw", locTestCityCode)). + WithIndex(&computev1alpha.WorkloadDeployment{}, deploymentWorkloadUIDIndex, deploymentWorkloadUIDIndexFunc). + Build() + + r := &WorkloadReconciler{LocationSource: locations.SourceLocations} + + desired, orphaned, err := r.getDeploymentsForWorkload(context.Background(), cl, workload) + require.NoError(t, err) + require.Empty(t, orphaned) + require.Len(t, desired, 1) + assert.Equal(t, locTestCityCode, desired[0].Spec.CityCode) +} + +// TestGetDeploymentsForWorkload_LocationsSourceIgnoresBindings verifies the +// sources do not leak into each other: LocationBindings are invisible to a +// deployment reading the locations service. +func TestGetDeploymentsForWorkload_LocationsSourceIgnoresBindings(t *testing.T) { + t.Parallel() + + workload := &computev1alpha.Workload{ + ObjectMeta: metav1.ObjectMeta{ + Name: rdTestWorkloadName, + Namespace: testDefaultNamespace, + UID: types.UID("workload-uid"), + }, + } + + cl := fake.NewClientBuilder(). + WithScheme(newLocationsServiceScheme()). + WithObjects(newTestLocationBinding("dfw", locTestCityCode)). + WithIndex(&computev1alpha.WorkloadDeployment{}, deploymentWorkloadUIDIndex, deploymentWorkloadUIDIndexFunc). + Build() + + r := &WorkloadReconciler{LocationSource: locations.SourceLocations} + + _, _, err := r.getDeploymentsForWorkload(context.Background(), cl, workload) + require.ErrorContains(t, err, "no locations are registered with the system") +} + +// TestResolveLocation_LocationsSource verifies a cell reads its serving +// location from the locations service when that source is selected. +func TestResolveLocation_LocationsSource(t *testing.T) { + t.Parallel() + + const locationName = "loc-dfw-1" + + cl := fake.NewClientBuilder(). + WithScheme(newLocationsServiceScheme()). + WithObjects( + newLocationsServiceServingLocation(locationName, locTestCityCode), + // The network services copy must be ignored by this source. + newTestServingLocation("nso-"+locationName, locTestOtherCityCode), + ). + Build() + + deployment := newLocationTestDeployment("test-wd") + + r := &WorkloadDeploymentReconciler{LocationSource: locations.SourceLocations} + result, err := r.resolveLocation(context.Background(), cl) + require.NoError(t, err) + result.evaluate(deployment) + + require.NotNil(t, result.reference) + assert.Equal(t, locationName, result.reference.Name) + assert.Empty(t, result.reason) + assert.False(t, result.blocked) +} + +// TestResolveLocation_NetworkServicesSourceIgnoresLocationsService is the +// safety property behind the flag: a deployment that does not set it reads +// exactly what it reads today. +func TestResolveLocation_NetworkServicesSourceIgnoresLocationsService(t *testing.T) { + t.Parallel() + + cl := fake.NewClientBuilder(). + WithScheme(newLocationsServiceScheme()). + WithObjects(newLocationsServiceServingLocation("loc-dfw-1", locTestCityCode)). + Build() + + r := &WorkloadDeploymentReconciler{} + result, err := r.resolveLocation(context.Background(), cl) + require.NoError(t, err) + + assert.Nil(t, result.reference) + assert.Equal(t, computev1alpha.WorkloadDeploymentReasonNoMatchingLocation, result.reason) +} diff --git a/internal/controller/workload_controller.go b/internal/controller/workload_controller.go index c9d8ac98..6bba2365 100644 --- a/internal/controller/workload_controller.go +++ b/internal/controller/workload_controller.go @@ -31,6 +31,7 @@ import ( mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" computev1alpha "go.datum.net/compute/api/v1alpha" + "go.datum.net/compute/internal/locations" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" ) @@ -49,12 +50,18 @@ type WorkloadReconciler struct { // on control planes without the integration, and engaging a watch against a // missing kind wedges the manager. NetworkingEnabled bool + + // LocationSource selects the API group placement locations are read from. + // The zero value reads network services, which is what every deployment + // does today. + LocationSource locations.Source } // +kubebuilder:rbac:groups=compute.datumapis.com,resources=workloads,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=compute.datumapis.com,resources=workloads/status,verbs=get;update;patch // +kubebuilder:rbac:groups=compute.datumapis.com,resources=workloads/finalizers,verbs=update // +kubebuilder:rbac:groups=networking.datumapis.com,resources=networks,verbs=get;list;watch +// +kubebuilder:rbac:groups=locations.miloapis.com,resources=locations,verbs=get;list;watch func (r *WorkloadReconciler) Reconcile(ctx context.Context, req mcreconcile.Request) (ctrl.Result, error) { logger := log.FromContext(ctx) @@ -446,28 +453,21 @@ func (r *WorkloadReconciler) getDeploymentsForWorkload( existingDeployments.Insert(deployment.Name) } - var locations networkingv1alpha.LocationBindingList - if err := upstreamClient.List(ctx, &locations); err != nil { - return nil, nil, fmt.Errorf("failed to list location bindings: %w", err) + placementLocations, err := locations.ListPlacementLocations(ctx, upstreamClient, r.LocationSource) + if err != nil { + return nil, nil, err } - if len(locations.Items) == 0 { + if len(placementLocations) == 0 { return nil, nil, fmt.Errorf("no locations are registered with the system") } + cityCodes := locations.CityCodes(placementLocations) + // Remember this: namespace, name, err := cache.SplitMetaNamespaceKey(key) for _, placement := range workload.Spec.Placements { for _, cityCode := range placement.CityCodes { - foundLocation := false - for _, location := range locations.Items { - locationCityCode, ok := location.Spec.Topology[networkingv1alpha.TopologyCityCodeKey] - if ok && cityCode == locationCityCode { - foundLocation = true - break - } - } - - if !foundLocation { + if !cityCodes.Has(cityCode) { // TODO(jreese) update status condition on placement if no locations are // found. continue diff --git a/internal/controller/workload_controller_test.go b/internal/controller/workload_controller_test.go index d8dbef80..c5a8e5dd 100644 --- a/internal/controller/workload_controller_test.go +++ b/internal/controller/workload_controller_test.go @@ -19,6 +19,18 @@ import ( networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" ) +// newTestLocationBinding builds the projection a project control plane holds +// today for a location it may place workloads at. +func newTestLocationBinding(name, cityCode string) *networkingv1alpha.LocationBinding { + return &networkingv1alpha.LocationBinding{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: networkingv1alpha.LocationBindingSpec{ + LocationRef: corev1.LocalObjectReference{Name: name}, + Topology: map[string]string{networkingv1alpha.TopologyCityCodeKey: cityCode}, + }, + } +} + // makeWorkload builds a Workload with the given generation for use in // reconcileWorkloadStatus unit tests. func makeWorkload(generation int64) *computev1alpha.Workload { @@ -87,7 +99,7 @@ func TestGetDeploymentsForWorkload_InitializesReplicas(t *testing.T) { workload := &computev1alpha.Workload{ ObjectMeta: metav1.ObjectMeta{ - Name: "test-workload", + Name: rdTestWorkloadName, Namespace: testDefaultNamespace, UID: types.UID("workload-uid"), }, @@ -103,13 +115,7 @@ func TestGetDeploymentsForWorkload_InitializesReplicas(t *testing.T) { }, }, } - location := &networkingv1alpha.LocationBinding{ - ObjectMeta: metav1.ObjectMeta{Name: "dfw"}, - Spec: networkingv1alpha.LocationBindingSpec{ - LocationRef: corev1.LocalObjectReference{Name: "dfw"}, - Topology: map[string]string{"topology.datum.net/city-code": "DFW"}, - }, - } + location := newTestLocationBinding("dfw", "DFW") s := newNetworkingScheme() cl := fake.NewClientBuilder(). diff --git a/internal/controller/workloaddeployment_controller.go b/internal/controller/workloaddeployment_controller.go index 06bc90b8..c959c27d 100644 --- a/internal/controller/workloaddeployment_controller.go +++ b/internal/controller/workloaddeployment_controller.go @@ -31,6 +31,7 @@ import ( mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" computev1alpha "go.datum.net/compute/api/v1alpha" + "go.datum.net/compute/internal/locations" networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" "go.datum.net/compute/internal/controller/instancecontrol" @@ -60,6 +61,11 @@ type WorkloadDeploymentReconciler struct { // When true, new Instances whose template references ConfigMaps or Secrets // receive the ReferencedData scheduling gate at creation time. enableReferencedDataGate bool + + // LocationSource selects the API group the cell's serving location is read + // from. The zero value reads network services, which is what every + // deployment does today. + LocationSource locations.Source } func effectiveDesiredReplicas(deployment *computev1alpha.WorkloadDeployment) int32 { @@ -81,6 +87,7 @@ func workloadDeploymentPodSelector(deployment *computev1alpha.WorkloadDeployment // +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=servinglocations,verbs=get;list;watch +// +kubebuilder:rbac:groups=locations.miloapis.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 @@ -639,7 +646,7 @@ type servingLocationResult struct { // servingLocation is the single ServingLocation the cell was delivered, or // nil when it was delivered none or more than one. - servingLocation *networkingv1alpha.ServingLocation + servingLocation *locations.ServingLocation // reason and message are the Available condition the deployment should // report while the location is unusable. Both are empty once it is usable. @@ -662,14 +669,14 @@ func (r *WorkloadDeploymentReconciler) resolveLocation( ctx context.Context, c client.Client, ) (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) + servingLocations, err := locations.ListServingLocations(ctx, c, r.LocationSource) + if err != nil { + return servingLocationResult{}, err } - if len(servingLocations.Items) > 1 { - names := make([]string, 0, len(servingLocations.Items)) - for _, servingLocation := range servingLocations.Items { + if len(servingLocations) > 1 { + names := make([]string, 0, len(servingLocations)) + for _, servingLocation := range servingLocations { names = append(names, servingLocation.Name) } slices.Sort(names) @@ -682,7 +689,7 @@ func (r *WorkloadDeploymentReconciler) resolveLocation( }, nil } - if len(servingLocations.Items) == 0 { + if len(servingLocations) == 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") @@ -690,11 +697,11 @@ func (r *WorkloadDeploymentReconciler) resolveLocation( 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), + locations.ServingLocationTopologyLabel), }, nil } - servingLocation := &servingLocations.Items[0] + servingLocation := &servingLocations[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 @@ -919,6 +926,15 @@ func (r *WorkloadDeploymentReconciler) SetupWithManager(mgr mcmanager.Manager, o // On cells without network-services-operator these watches would log spurious // errors for missing CRDs. if r.NetworkingEnabled { + if err := locations.EnsureServingLocationKind(mgr.GetLocalManager().GetRESTMapper(), r.LocationSource); err != nil { + return err + } + + servingLocationObject, err := locations.ServingLocationObject(r.LocationSource) + if err != nil { + return err + } + b = b. // A claim becoming bound and allocated is what releases an instance's // Network gate, and nothing else wakes this reconciler for it. The claim @@ -933,7 +949,7 @@ func (r *WorkloadDeploymentReconciler) SetupWithManager(mgr mcmanager.Manager, o // 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] { + Watches(servingLocationObject, 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) }) diff --git a/internal/controller/workloaddeployment_servinglocation_guard_test.go b/internal/controller/workloaddeployment_servinglocation_guard_test.go new file mode 100644 index 00000000..6255a2f1 --- /dev/null +++ b/internal/controller/workloaddeployment_servinglocation_guard_test.go @@ -0,0 +1,141 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package controller + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/cluster" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/log/zap" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" + "sigs.k8s.io/multicluster-runtime/pkg/multicluster" + mcsingle "sigs.k8s.io/multicluster-runtime/providers/single" + + "go.datum.net/compute/internal/locations" +) + +// moduleCRD resolves a single CRD manifest shipped by a dependency, so the +// test installs the schema that module actually serves. +func moduleCRD(t *testing.T, module, dir, file string) string { + t.Helper() + + out, err := exec.Command("go", "list", "-m", "-f", "{{.Dir}}", module).Output() + require.NoError(t, err) + + path := filepath.Join(strings.TrimSpace(string(out)), filepath.FromSlash(dir), file) + _, err = os.Stat(path) + require.NoError(t, err) + return path +} + +// TestSetupWithManager_ServingLocationKindGuard covers the watch guard against +// real discovery. +// +// Registering a watch on a kind the cell does not serve wedges the manager +// during cache sync, so setup refuses instead. The guard has to fire for the +// selected source and stay silent about the other one: a deployment reading +// network services must not acquire a startup dependency on the locations +// service it never opted into. +// +// The refusals run through SetupWithManager, which also settles that discovery +// answers at registration time, on a manager that has not been started. The +// permitting direction is asserted on the manager's own REST mapper, because +// controller-runtime registers controller names in a process-global set and +// TestWorkloadDeploymentSetupWithManager_CellModeNoNetworkingCRD already claims +// this reconciler's name for the package. +func TestSetupWithManager_ServingLocationKindGuard(t *testing.T) { + ctrl.SetLogger(zap.New(zap.UseDevMode(true), zap.WriteTo(os.Stderr))) + + cfg := startComputeEnvtest(t) + scheme := newLocationsServiceScheme() + + // newManager builds a manager the way cmd/main.go does for a cell. + newManager := func(t *testing.T) mcmanager.Manager { + t.Helper() + + deploymentCluster, err := cluster.New(rest.CopyConfig(cfg), func(o *cluster.Options) { o.Scheme = scheme }) + require.NoError(t, err) + + mgr, err := mcmanager.New(rest.CopyConfig(cfg), + mcsingle.New(multicluster.ClusterName("single"), deploymentCluster), + ctrl.Options{ + Scheme: scheme, + Metrics: metricsserver.Options{BindAddress: "0"}, + HealthProbeBindAddress: "0", + }) + require.NoError(t, err) + return mgr + } + + setupWithSource := func(t *testing.T, source locations.Source) error { + t.Helper() + r := &WorkloadDeploymentReconciler{NetworkingEnabled: true, LocationSource: source} + return r.SetupWithManager(newManager(t)) + } + + // ensureWithSource runs the guard the way SetupWithManager runs it, against + // the same live REST mapper. + ensureWithSource := func(t *testing.T, source locations.Source) error { + t.Helper() + return locations.EnsureServingLocationKind(newManager(t).GetLocalManager().GetRESTMapper(), source) + } + + installCRD := func(t *testing.T, path string) { + t.Helper() + _, err := envtest.InstallCRDs(cfg, envtest.CRDInstallOptions{Paths: []string{path}}) + require.NoError(t, err) + } + + networkServicesCRD := moduleCRD(t, "go.datum.net/network-services-operator", + "config/crd/bases", "networking.datumapis.com_servinglocations.yaml") + locationsCRD := moduleCRD(t, "go.miloapis.com/locations", + "config/base/crd/bases", "locations.miloapis.com_servinglocations.yaml") + + t.Run("neither kind served", func(t *testing.T) { + err := setupWithSource(t, "") + require.Error(t, err, "the default source must refuse to watch a kind the cell does not serve") + assert.Contains(t, err.Error(), "servinglocations.networking.datumapis.com", + "the error must name the CRD to install") + assert.Contains(t, err.Error(), "NetworkServices", + "the error must name the locationSource that requires it") + assert.Contains(t, err.Error(), "ServingLocation") + + err = setupWithSource(t, locations.SourceLocations) + require.Error(t, err) + assert.Contains(t, err.Error(), "servinglocations.locations.miloapis.com") + assert.Contains(t, err.Error(), "Locations") + }) + + // Install only the locations service kind. The Locations source must now be + // permitted and the default must still refuse: each source gates on the one + // kind it watches. + installCRD(t, locationsCRD) + + t.Run("only the locations service kind served", func(t *testing.T) { + require.NoError(t, ensureWithSource(t, locations.SourceLocations), + "the selected source's kind is served, so the guard must permit it") + + err := setupWithSource(t, "") + require.Error(t, err, "installing the locations service must not satisfy the default source") + assert.Contains(t, err.Error(), "servinglocations.networking.datumapis.com") + }) + + installCRD(t, networkServicesCRD) + + t.Run("both kinds served", func(t *testing.T) { + for _, source := range []locations.Source{"", locations.SourceNetworkServices, locations.SourceLocations} { + require.NoErrorf(t, ensureWithSource(t, source), + "the guard must permit source %q once its kind is served", source) + } + }) +} diff --git a/internal/locations/locations.go b/internal/locations/locations.go new file mode 100644 index 00000000..09c9026a --- /dev/null +++ b/internal/locations/locations.go @@ -0,0 +1,260 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +// Package locations reads the two location facts compute depends on: which +// cities a project may place workloads in, and which location a cell serves. +// +// Both are served today by network-services-operator and are moving to the +// locations service. Which one is read is selected per deployment by Source, +// so a control plane that has not been migrated keeps reading the types it +// already has. +package locations + +import ( + "context" + "fmt" + "strings" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + apimeta "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/util/sets" + "sigs.k8s.io/controller-runtime/pkg/client" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" +) + +const ( + // TopologyCityCodeKey is the topology key holding a location's city. + TopologyCityCodeKey = locationsv1alpha1.TopologyCityCodeKey + + // ServingLocationTopologyLabel is the cluster label a cell carries to claim + // the location it serves. + ServingLocationTopologyLabel = locationsv1alpha1.ServingLocationTopologyLabel +) + +// Source names the API group locations are read from. +type Source string + +const ( + // SourceNetworkServices reads networking.datumapis.com LocationBindings and + // ServingLocations. This is what every deployment reads today. + SourceNetworkServices Source = "NetworkServices" + + // SourceLocations reads locations.miloapis.com Locations and + // ServingLocations, served by the locations service. + SourceLocations Source = "Locations" +) + +// Resolve reports which source to read. An unset source reads network +// services, matching the config default. +func (s Source) Resolve() (Source, error) { + switch s { + case "", SourceNetworkServices: + return SourceNetworkServices, nil + case SourceLocations: + return SourceLocations, nil + default: + return "", fmt.Errorf("unknown location source %q, want %q or %q", s, SourceNetworkServices, SourceLocations) + } +} + +// PlacementLocation is a location a project may place workloads at. +type PlacementLocation struct { + Name string + Topology map[string]string +} + +// CityCode returns the city the location serves, and whether it declares one. +func (l PlacementLocation) CityCode() (string, bool) { + code, ok := l.Topology[TopologyCityCodeKey] + return code, ok +} + +// ServingLocation is the location a cell has been told it serves. +type ServingLocation struct { + Name string + Topology map[string]string +} + +// CityCode returns the city the cell sits in. +func (l ServingLocation) CityCode() string { + return l.Topology[TopologyCityCodeKey] +} + +// ListPlacementLocations returns the locations a project may place workloads +// at, read from the project's control plane. +func ListPlacementLocations(ctx context.Context, c client.Client, source Source) ([]PlacementLocation, error) { + resolved, err := source.Resolve() + if err != nil { + return nil, err + } + + if resolved == SourceNetworkServices { + var bindings networkingv1alpha.LocationBindingList + if err := c.List(ctx, &bindings); err != nil { + return nil, fmt.Errorf("failed to list location bindings: %w", err) + } + + found := make([]PlacementLocation, 0, len(bindings.Items)) + for _, binding := range bindings.Items { + found = append(found, PlacementLocation{ + Name: binding.Name, + Topology: binding.Spec.Topology, + }) + } + return found, nil + } + + var list locationsv1alpha1.LocationList + if err := c.List(ctx, &list); err != nil { + if kindNotInstalled(err) { + return nil, nil + } + return nil, fmt.Errorf("failed to list locations: %w", err) + } + + found := make([]PlacementLocation, 0, len(list.Items)) + for _, location := range list.Items { + found = append(found, PlacementLocation{ + Name: location.Name, + Topology: location.Spec.Topology, + }) + } + return found, nil +} + +// ListServingLocations returns the locations delivered to a cell. +func ListServingLocations(ctx context.Context, c client.Client, source Source) ([]ServingLocation, error) { + resolved, err := source.Resolve() + if err != nil { + return nil, err + } + + if resolved == SourceNetworkServices { + var list networkingv1alpha.ServingLocationList + if err := c.List(ctx, &list); err != nil { + return nil, fmt.Errorf("failed to list serving locations: %w", err) + } + + found := make([]ServingLocation, 0, len(list.Items)) + for _, servingLocation := range list.Items { + found = append(found, ServingLocation{ + Name: servingLocation.Name, + Topology: servingLocation.Spec.Topology, + }) + } + return found, nil + } + + var list locationsv1alpha1.ServingLocationList + if err := c.List(ctx, &list); err != nil { + if kindNotInstalled(err) { + return nil, nil + } + return nil, fmt.Errorf("failed to list serving locations: %w", err) + } + + found := make([]ServingLocation, 0, len(list.Items)) + for _, servingLocation := range list.Items { + found = append(found, ServingLocation{ + Name: servingLocation.Name, + Topology: servingLocation.Spec.Topology, + }) + } + return found, nil +} + +// ServingLocationObject returns the object a controller watches to learn that +// a cell has been told where it sits. +func ServingLocationObject(source Source) (client.Object, error) { + resolved, err := source.Resolve() + if err != nil { + return nil, err + } + + if resolved == SourceNetworkServices { + return &networkingv1alpha.ServingLocation{}, nil + } + return &locationsv1alpha1.ServingLocation{}, nil +} + +// ServingLocationGVK returns the kind a controller watches for the source. +func ServingLocationGVK(source Source) (schema.GroupVersionKind, error) { + resolved, err := source.Resolve() + if err != nil { + return schema.GroupVersionKind{}, err + } + + if resolved == SourceNetworkServices { + return networkingv1alpha.GroupVersion.WithKind("ServingLocation"), nil + } + return locationsv1alpha1.GroupVersion.WithKind("ServingLocation"), nil +} + +// EnsureServingLocationKind fails unless the control plane serves the kind the +// source watches. +// +// A watch is not a read: a list against a kind the control plane does not serve +// degrades to no locations, but a watch cannot, and registering one wedges the +// manager during cache sync. Refusing to start says which CRD is missing, where +// a wedged manager says nothing. +// +// This gates only the selected source. A deployment reading network services +// must not be made to depend on the locations service being installed. +func EnsureServingLocationKind(mapper apimeta.RESTMapper, source Source) error { + gvk, err := ServingLocationGVK(source) + if err != nil { + return err + } + + if _, err := mapper.RESTMapping(gvk.GroupKind(), gvk.Version); err != nil { + if kindNotInstalled(err) { + resolved, _ := source.Resolve() + return fmt.Errorf( + "locationSource %q watches %s, which this control plane does not serve: "+ + "install the %s CustomResourceDefinition, or set locationSource to %q", + resolved, gvk, crdName(gvk), otherSource(resolved)) + } + return fmt.Errorf("failed to determine whether %s is served: %w", gvk, err) + } + + return nil +} + +func crdName(gvk schema.GroupVersionKind) string { + return fmt.Sprintf("%ss.%s", strings.ToLower(gvk.Kind), gvk.Group) +} + +func otherSource(source Source) Source { + if source == SourceLocations { + return SourceNetworkServices + } + return SourceLocations +} + +// CityCodes returns the cities the given locations serve. +func CityCodes(found []PlacementLocation) sets.Set[string] { + codes := sets.Set[string]{} + for _, location := range found { + if code, ok := location.CityCode(); ok { + codes.Insert(code) + } + } + return codes +} + +// kindNotInstalled reports whether a list failed because the control plane +// does not serve the kind. A control plane is only expected to serve the kinds +// its consumers read, so a kind that is not there reads as empty rather than +// failing the caller. +// +// The REST mapper answers first. A typed client reaches it through discovery, +// so the no-match arrives wrapped in an ErrResourceDiscoveryFailed rather than +// bare, and only errors.Is unwrapping finds it. A mapper still holding the kind +// from before the CRD went away leaves the API server to answer, with a 404. A +// type missing from the scheme is neither of these: it is a wiring mistake, and +// must keep surfacing as one. +func kindNotInstalled(err error) bool { + return apimeta.IsNoMatchError(err) || apierrors.IsNotFound(err) +} diff --git a/internal/locations/locations_envtest_test.go b/internal/locations/locations_envtest_test.go new file mode 100644 index 00000000..02b7a4a2 --- /dev/null +++ b/internal/locations/locations_envtest_test.go @@ -0,0 +1,123 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package locations + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" +) + +// locationsCRDDir resolves the CRDs shipped by the locations module, so the +// test installs the same schemas the service serves rather than a local copy +// that can drift from the pinned commit. +func locationsCRDDir(t *testing.T) string { + t.Helper() + + out, err := exec.Command("go", "list", "-m", "-f", "{{.Dir}}", "go.miloapis.com/locations").Output() + require.NoError(t, err, "the locations module must be resolvable") + + dir := filepath.Join(strings.TrimSpace(string(out)), "config", "base", "crd", "bases") + _, err = os.Stat(dir) + require.NoError(t, err) + return dir +} + +// TestLocationsSource_AgainstAPIServer is the runtime half of the typed switch. +// Compiling against the locations types proves nothing about whether a client +// can resolve the kinds, so this runs both states against a real API server: +// the CRDs absent, which must read as no locations, and the CRDs installed, +// which must read the objects back. +func TestLocationsSource_AgainstAPIServer(t *testing.T) { + if os.Getenv("KUBEBUILDER_ASSETS") == "" { + assets := filepath.Join("..", "..", "bin", "k8s", + fmt.Sprintf("1.31.0-%s-%s", runtime.GOOS, runtime.GOARCH)) + if _, err := os.Stat(assets); err != nil { + t.Skip("no envtest assets; run via make test") + } + t.Setenv("KUBEBUILDER_ASSETS", assets) + } + + testEnv := &envtest.Environment{} + cfg, err := testEnv.Start() + require.NoError(t, err) + t.Cleanup(func() { _ = testEnv.Stop() }) + + ctx := context.Background() + scheme := testScheme(t) + + newClient := func() client.Client { + c, err := client.New(rest.CopyConfig(cfg), client.Options{Scheme: scheme}) + require.NoError(t, err) + return c + } + + t.Run("CRDs absent reads as no locations", func(t *testing.T) { + c := newClient() + + // A typed client reaches the REST mapper through discovery, which wraps + // the no-match. Assert on the raw error so a client-go or + // controller-runtime bump that changes the wrapping fails here, rather + // than quietly turning the degrade into a reconcile failure. + var absent locationsv1alpha1.LocationList + rawErr := c.List(ctx, &absent) + require.Error(t, rawErr, "the CRD really is absent, so the degrade is under test") + assert.True(t, kindNotInstalled(rawErr), + "an absent CRD must stay recognisable as such: %T / %v", rawErr, rawErr) + + found, err := ListPlacementLocations(ctx, c, SourceLocations) + require.NoError(t, err, "a control plane without the CRDs must not fail the reconcile") + assert.Empty(t, found) + + serving, err := ListServingLocations(ctx, c, SourceLocations) + require.NoError(t, err) + assert.Empty(t, serving) + + // The default source has no such degrade: its kinds are expected + // everywhere it runs, so their absence stays an error. + _, err = ListPlacementLocations(ctx, c, SourceNetworkServices) + require.Error(t, err) + }) + + _, err = envtest.InstallCRDs(cfg, envtest.CRDInstallOptions{ + Paths: []string{locationsCRDDir(t)}, + }) + require.NoError(t, err) + + t.Run("CRDs installed resolve and read back", func(t *testing.T) { + c := newClient() + + require.NoError(t, c.Create(ctx, newLocation("dfw", testCityCode))) + require.NoError(t, c.Create(ctx, &locationsv1alpha1.ServingLocation{ + ObjectMeta: metav1.ObjectMeta{Name: "dfw"}, + Spec: locationsv1alpha1.ServingLocationSpec{ + Topology: map[string]string{TopologyCityCodeKey: testCityCode}, + }, + })) + + found, err := ListPlacementLocations(ctx, c, SourceLocations) + require.NoError(t, err) + require.Len(t, found, 1) + assert.Equal(t, []string{testCityCode}, CityCodes(found).UnsortedList()) + + serving, err := ListServingLocations(ctx, c, SourceLocations) + require.NoError(t, err) + require.Len(t, serving, 1) + assert.Equal(t, "dfw", serving[0].Name) + assert.Equal(t, testCityCode, serving[0].CityCode()) + }) +} diff --git a/internal/locations/locations_test.go b/internal/locations/locations_test.go new file mode 100644 index 00000000..e32a732b --- /dev/null +++ b/internal/locations/locations_test.go @@ -0,0 +1,274 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package locations + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + apimeta "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" + locationsv1alpha1 "go.miloapis.com/locations/api/v1alpha1" +) + +const ( + testCityCode = "DFW" + testOtherCityCode = "ORD" +) + +func testScheme(t *testing.T) *runtime.Scheme { + t.Helper() + + s := runtime.NewScheme() + require.NoError(t, networkingv1alpha.AddToScheme(s)) + require.NoError(t, locationsv1alpha1.AddToScheme(s)) + return s +} + +func newBinding(name, cityCode string) *networkingv1alpha.LocationBinding { + return &networkingv1alpha.LocationBinding{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: networkingv1alpha.LocationBindingSpec{ + LocationRef: corev1.LocalObjectReference{Name: name}, + Topology: map[string]string{TopologyCityCodeKey: cityCode}, + }, + } +} + +func newLocation(name, cityCode string) *locationsv1alpha1.Location { + return &locationsv1alpha1.Location{ + ObjectMeta: metav1.ObjectMeta{Name: name}, + Spec: locationsv1alpha1.LocationSpec{ + LocationClassRef: locationsv1alpha1.LocationClassReference{Name: "datum-managed"}, + Topology: map[string]string{TopologyCityCodeKey: cityCode}, + }, + } +} + +// TestTopologyKeysAgreeAcrossSources guards the migration's central assumption: +// a city code means the same thing whichever source served it. If the two +// groups ever disagree, switching sources would silently repoint every +// placement. +func TestTopologyKeysAgreeAcrossSources(t *testing.T) { + t.Parallel() + + assert.Equal(t, networkingv1alpha.TopologyCityCodeKey, TopologyCityCodeKey) + assert.Equal(t, networkingv1alpha.ServingLocationTopologyLabel, ServingLocationTopologyLabel) +} + +func TestSourceResolve(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + source Source + want Source + wantOK bool + }{ + {source: "", want: SourceNetworkServices, wantOK: true}, + {source: SourceNetworkServices, want: SourceNetworkServices, wantOK: true}, + {source: SourceLocations, want: SourceLocations, wantOK: true}, + {source: "Nonsense"}, + } { + resolved, err := tc.source.Resolve() + if !tc.wantOK { + require.Error(t, err) + continue + } + require.NoError(t, err) + assert.Equal(t, tc.want, resolved) + } +} + +func TestListPlacementLocations_NetworkServices(t *testing.T) { + t.Parallel() + + cl := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects( + newBinding("dfw", testCityCode), + newBinding("ord", testOtherCityCode), + // A binding with no city code contributes no placement city. + &networkingv1alpha.LocationBinding{ObjectMeta: metav1.ObjectMeta{Name: "nowhere"}}, + // The locations service must not be read when network services is + // selected. + newLocation("lhr", "LHR"), + ). + Build() + + found, err := ListPlacementLocations(context.Background(), cl, SourceNetworkServices) + require.NoError(t, err) + require.Len(t, found, 3) + assert.ElementsMatch(t, []string{testCityCode, testOtherCityCode}, CityCodes(found).UnsortedList()) +} + +func TestListPlacementLocations_Locations(t *testing.T) { + t.Parallel() + + cl := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects( + newLocation("dfw", testCityCode), + newLocation("ord", testOtherCityCode), + // The network services source must not be read when the locations + // service is selected. + newBinding("lhr", "LHR"), + ). + Build() + + found, err := ListPlacementLocations(context.Background(), cl, SourceLocations) + require.NoError(t, err) + require.Len(t, found, 2) + assert.ElementsMatch(t, []string{testCityCode, testOtherCityCode}, CityCodes(found).UnsortedList()) +} + +func TestListPlacementLocations_UnknownSource(t *testing.T) { + t.Parallel() + + cl := fake.NewClientBuilder().WithScheme(testScheme(t)).Build() + + _, err := ListPlacementLocations(context.Background(), cl, "Nonsense") + require.Error(t, err) +} + +// TestKindNotInstalled_ScopedToLocationsSource pins the degrade to the source +// that needs it. A no-match reads as no locations for the locations service, +// which may not be installed yet, and still fails for network services, whose +// kinds every control plane already serves. +func TestKindNotInstalled_ScopedToLocationsSource(t *testing.T) { + t.Parallel() + + noMatch := func(_ context.Context, _ client.WithWatch, list client.ObjectList, _ ...client.ListOption) error { + gvk := list.GetObjectKind().GroupVersionKind() + return &apimeta.NoKindMatchError{GroupKind: gvk.GroupKind().WithVersion("").GroupKind()} + } + + cl := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithInterceptorFuncs(interceptor.Funcs{List: noMatch}). + Build() + + ctx := context.Background() + + found, err := ListPlacementLocations(ctx, cl, SourceLocations) + require.NoError(t, err) + assert.Empty(t, found) + + serving, err := ListServingLocations(ctx, cl, SourceLocations) + require.NoError(t, err) + assert.Empty(t, serving) + + _, err = ListPlacementLocations(ctx, cl, SourceNetworkServices) + require.Error(t, err) + + _, err = ListServingLocations(ctx, cl, SourceNetworkServices) + require.Error(t, err) +} + +// TestListLocations_SchemeMissingStillFails separates a control plane that does +// not serve the kind from a binary that forgot to register it. The first reads +// as empty; the second is a wiring mistake and must keep surfacing. +func TestListLocations_SchemeMissingStillFails(t *testing.T) { + t.Parallel() + + bare := runtime.NewScheme() + require.NoError(t, networkingv1alpha.AddToScheme(bare)) + + cl := fake.NewClientBuilder().WithScheme(bare).Build() + + _, err := ListPlacementLocations(context.Background(), cl, SourceLocations) + require.Error(t, err, "an unregistered type is a wiring mistake, not an empty control plane") +} + +func TestListServingLocations(t *testing.T) { + t.Parallel() + + cl := fake.NewClientBuilder(). + WithScheme(testScheme(t)). + WithObjects( + &networkingv1alpha.ServingLocation{ + ObjectMeta: metav1.ObjectMeta{Name: "nso-dfw"}, + Spec: networkingv1alpha.ServingLocationSpec{ + Topology: map[string]string{TopologyCityCodeKey: testCityCode}, + }, + }, + &locationsv1alpha1.ServingLocation{ + ObjectMeta: metav1.ObjectMeta{Name: "locations-ord"}, + Spec: locationsv1alpha1.ServingLocationSpec{ + Topology: map[string]string{TopologyCityCodeKey: testOtherCityCode}, + }, + }, + ). + Build() + + ctx := context.Background() + + fromNetworkServices, err := ListServingLocations(ctx, cl, SourceNetworkServices) + require.NoError(t, err) + require.Len(t, fromNetworkServices, 1) + assert.Equal(t, "nso-dfw", fromNetworkServices[0].Name) + assert.Equal(t, testCityCode, fromNetworkServices[0].CityCode()) + + fromLocations, err := ListServingLocations(ctx, cl, SourceLocations) + require.NoError(t, err) + require.Len(t, fromLocations, 1) + assert.Equal(t, "locations-ord", fromLocations[0].Name) + assert.Equal(t, testOtherCityCode, fromLocations[0].CityCode()) + + // An unset source reads what every deployment reads today. + fromDefault, err := ListServingLocations(ctx, cl, "") + require.NoError(t, err) + assert.Equal(t, fromNetworkServices, fromDefault) +} + +func TestServingLocationObject(t *testing.T) { + t.Parallel() + + for _, source := range []Source{"", SourceNetworkServices} { + object, err := ServingLocationObject(source) + require.NoError(t, err) + assert.IsType(t, &networkingv1alpha.ServingLocation{}, object) + } + + object, err := ServingLocationObject(SourceLocations) + require.NoError(t, err) + assert.IsType(t, &locationsv1alpha1.ServingLocation{}, object) + + _, err = ServingLocationObject("Nonsense") + require.Error(t, err) +} + +// TestEnsureServingLocationKind_UnknownSource keeps an unreadable config from +// reaching the REST mapper at all. +func TestEnsureServingLocationKind_UnknownSource(t *testing.T) { + t.Parallel() + + require.Error(t, EnsureServingLocationKind(apimeta.NewDefaultRESTMapper(nil), "Nonsense")) +} + +func TestServingLocationGVK(t *testing.T) { + t.Parallel() + + for _, source := range []Source{"", SourceNetworkServices} { + gvk, err := ServingLocationGVK(source) + require.NoError(t, err) + assert.Equal(t, networkingv1alpha.GroupVersion.WithKind("ServingLocation"), gvk) + assert.Equal(t, "servinglocations.networking.datumapis.com", crdName(gvk)) + } + + gvk, err := ServingLocationGVK(SourceLocations) + require.NoError(t, err) + assert.Equal(t, locationsv1alpha1.GroupVersion.WithKind("ServingLocation"), gvk) + assert.Equal(t, "servinglocations.locations.miloapis.com", crdName(gvk)) + + _, err = ServingLocationGVK("Nonsense") + require.Error(t, err) +} diff --git a/internal/webhook/v1alpha/workload_webhook.go b/internal/webhook/v1alpha/workload_webhook.go index e8926b60..7a07680b 100644 --- a/internal/webhook/v1alpha/workload_webhook.go +++ b/internal/webhook/v1alpha/workload_webhook.go @@ -3,28 +3,28 @@ package webhook import ( "context" - "fmt" - "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/util/sets" ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" mcmanager "sigs.k8s.io/multicluster-runtime/pkg/manager" "sigs.k8s.io/multicluster-runtime/pkg/multicluster" computev1alpha "go.datum.net/compute/api/v1alpha" + "go.datum.net/compute/internal/locations" "go.datum.net/compute/internal/validation" computewebhook "go.datum.net/compute/internal/webhook" - networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" ) // SetupWorkloadWebhookWithManager will setup the manager to manage workload // webhooks -func SetupWorkloadWebhookWithManager(mgr mcmanager.Manager) error { +func SetupWorkloadWebhookWithManager(mgr mcmanager.Manager, locationSource locations.Source) error { webhook := &workloadWebhook{ - mgr: mgr, + mgr: mgr, + locationSource: locationSource, } return ctrl.NewWebhookManagedBy(mgr.GetLocalManager(), &computev1alpha.Workload{}). @@ -36,7 +36,16 @@ func SetupWorkloadWebhookWithManager(mgr mcmanager.Manager) error { // +kubebuilder:webhook:path=/mutate-compute-datumapis-com-v1alpha-workload,mutating=true,failurePolicy=fail,sideEffects=None,groups=compute.datumapis.com,resources=workloads,verbs=create;update,versions=v1alpha,name=mworkload.kb.io,admissionReviewVersions=v1 type workloadWebhook struct { - mgr mcmanager.Manager + mgr mcmanager.Manager + locationSource locations.Source +} + +func (r *workloadWebhook) validCityCodes(ctx context.Context, c client.Client) ([]string, error) { + placementLocations, err := locations.ListPlacementLocations(ctx, c, r.locationSource) + if err != nil { + return nil, err + } + return sets.List(locations.CityCodes(placementLocations)), nil } var _ admission.Defaulter[*computev1alpha.Workload] = &workloadWebhook{} @@ -89,17 +98,9 @@ func (r *workloadWebhook) ValidateCreate(ctx context.Context, workload *computev // that means for the scheduling phase, since there would not currently be // sufficient context to know who created the workload and what locations // are valid candidates based on that. Maybe an annotation, or spec field? - var locations networkingv1alpha.LocationBindingList - if err := clusterClient.List(ctx, &locations); err != nil { - return nil, fmt.Errorf("failed to list location bindings: %w", err) - } - - validCityCodes := sets.Set[string]{} - for _, location := range locations.Items { - cityCode, ok := location.Spec.Topology[networkingv1alpha.TopologyCityCodeKey] - if ok { - validCityCodes.Insert(cityCode) - } + validCityCodes, err := r.validCityCodes(ctx, clusterClient) + if err != nil { + return nil, err } opts := validation.WorkloadValidationOptions{ @@ -107,7 +108,7 @@ func (r *workloadWebhook) ValidateCreate(ctx context.Context, workload *computev Client: clusterClient, AdmissionRequest: req, Workload: workload, - ValidCityCodes: sets.List(validCityCodes), + ValidCityCodes: validCityCodes, } if errs := validation.ValidateWorkloadCreate(workload, opts); len(errs) > 0 { @@ -134,17 +135,9 @@ func (r *workloadWebhook) ValidateUpdate(ctx context.Context, _ *computev1alpha. return nil, err } - var locations networkingv1alpha.LocationBindingList - if err := clusterClient.List(ctx, &locations); err != nil { - return nil, fmt.Errorf("failed to list location bindings: %w", err) - } - - validCityCodes := sets.Set[string]{} - for _, location := range locations.Items { - cityCode, ok := location.Spec.Topology[networkingv1alpha.TopologyCityCodeKey] - if ok { - validCityCodes.Insert(cityCode) - } + validCityCodes, err := r.validCityCodes(ctx, clusterClient) + if err != nil { + return nil, err } opts := validation.WorkloadValidationOptions{ @@ -152,7 +145,7 @@ func (r *workloadWebhook) ValidateUpdate(ctx context.Context, _ *computev1alpha. Client: clusterClient, AdmissionRequest: req, Workload: newWorkload, - ValidCityCodes: sets.List(validCityCodes), + ValidCityCodes: validCityCodes, } if errs := validation.ValidateWorkloadCreate(newWorkload, opts); len(errs) > 0 {