diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/README.md b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/README.md index 8875cc01c..48f7cc555 100644 --- a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/README.md +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/README.md @@ -4,6 +4,11 @@ Provisions networking resources using ClusterUserDefinedNetwork (CUDN) on OpenSh > **Note:** SecurityGroup enforcement (NetworkPolicy) has been extracted to the standalone > `osac.templates.network_policy` role so it can be reused across any K8s-based NetworkClass. +> `cudn_net`'s `create_security_group`/`delete_security_group` entrypoints delegate to that +> role directly (see [Task Files](#task-files)) — the dispatcher resolves `SecurityGroup` to +> this NetworkClass's fabric manager (`cudn_net`) the same way it does for `VirtualNetwork` +> and `Subnet`, so `cudn_net` must provide these entrypoints even though the underlying +> enforcement mechanism lives in `network_policy`. ## Resources @@ -57,6 +62,8 @@ This role implements the `cudn_net` NetworkClass strategy using OpenShift's Clus - `tasks/delete_virtual_network.yaml` - Removes ClusterUserDefinedNetwork CR - `tasks/create_subnet.yaml` - Creates namespace with CUDN labels from Subnet resource - `tasks/delete_subnet.yaml` - Removes namespace +- `tasks/create_security_group.yaml` - Delegates to `osac.templates.network_policy` (`create_security_group`) +- `tasks/delete_security_group.yaml` - Delegates to `osac.templates.network_policy` (`delete_security_group`) ## Usage diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/argument_specs.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/argument_specs.yaml index 2e9863f83..0c6d041f6 100644 --- a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/argument_specs.yaml +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/argument_specs.yaml @@ -53,3 +53,24 @@ argument_specs: type: str required: true description: Name of the VirtualNetwork resource + + # SecurityGroup entrypoints delegate to the network_policy role (see + # tasks/create_security_group.yaml and tasks/delete_security_group.yaml). + create_security_group: + options: + security_group: + type: dict + required: true + description: SecurityGroup CR from fulfillment-api via osac_job_vars + template_parameters: + type: dict + description: Template-specific parameters (reserved for future use) + options: {} + default: {} + + delete_security_group: + options: + security_group: + type: dict + required: true + description: SecurityGroup CR from fulfillment-api via osac_job_vars diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/osac.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/osac.yaml index 5cdaef9ad..9ee6a0538 100644 --- a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/osac.yaml +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/meta/osac.yaml @@ -8,9 +8,13 @@ description: > template_type: network # NetworkClass registration fields +# Note: no k8s_manager is set here. cudn_net's Subnet role already creates a +# self-contained ClusterUserDefinedNetwork (isolated Layer2/Primary) — there is no +# separate physical fabric to bridge into, so this NetworkClass is fabric-only. +# A future k8sManager (e.g. cudn_localnet, bridging OVN to a physical VLAN via +# LocalNet topology) does not exist yet; see OSAC-1511. implementation_strategy: cudn_net fabric_manager: cudn_net -k8s_manager: cudn_localnet is_default: true capabilities: supports_ipv4: true diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/tasks/create_security_group.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/tasks/create_security_group.yaml new file mode 100644 index 000000000..3de0ca1ee --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/tasks/create_security_group.yaml @@ -0,0 +1,9 @@ +--- +# cudn_net delegates SecurityGroup enforcement to the standalone network_policy +# role (NetworkPolicy-based), which is reusable across any K8s-based NetworkClass. +# See README.md for the rationale. + +- name: Delegate SecurityGroup creation to network_policy role + ansible.builtin.include_role: + name: osac.templates.network_policy + tasks_from: create_security_group diff --git a/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/tasks/delete_security_group.yaml b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/tasks/delete_security_group.yaml new file mode 100644 index 000000000..43d74a832 --- /dev/null +++ b/osac-aap/collections/ansible_collections/osac/templates/roles/cudn_net/tasks/delete_security_group.yaml @@ -0,0 +1,9 @@ +--- +# cudn_net delegates SecurityGroup enforcement to the standalone network_policy +# role (NetworkPolicy-based), which is reusable across any K8s-based NetworkClass. +# See README.md for the rationale. + +- name: Delegate SecurityGroup deletion to network_policy role + ansible.builtin.include_role: + name: osac.templates.network_policy + tasks_from: delete_security_group diff --git a/osac-operator/charts/operator/values.yaml b/osac-operator/charts/operator/values.yaml index 7176f76f3..19dcbc68a 100644 --- a/osac-operator/charts/operator/values.yaml +++ b/osac-operator/charts/operator/values.yaml @@ -68,7 +68,7 @@ tenants: [] # The operator discovers managers by selecting ConfigMaps with labels # osac.openshift.io/network-fabric-manager or osac.openshift.io/network-k8s-manager. networkManagers: - enabled: false + enabled: true # capabilitiesSyncInterval controls how often the operator recomputes NetworkClass # capabilities from the fabric/k8s manager ConfigMaps (in addition to reacting # immediately to ConfigMap changes). @@ -80,4 +80,11 @@ networkManagers: Netris SDN controller for physical fabric management. Manages VLAN/VxLAN segments, ACLs, public IP allocation, and NAT gateways. capabilities: "ipv4" + cudn_net: + enabled: true + description: >- + CUDN-based isolated networking (ClusterUserDefinedNetwork). Self-contained + VirtualNetwork/Subnet provisioning with no separate physical fabric to + bridge into — used as the platform default NetworkClass. + capabilities: "ipv4,ipv6,dualStack" k8sManagers: {} diff --git a/osac-operator/cmd/main.go b/osac-operator/cmd/main.go index 2dc0e3777..0a9ad67f8 100644 --- a/osac-operator/cmd/main.go +++ b/osac-operator/cmd/main.go @@ -508,9 +508,22 @@ func setupNetworkingControllers( return fmt.Errorf("externalip attachment provider: %w", err) } + // Build a shared dispatcher Resolver for controllers that support the two-manager + // model (VirtualNetwork, Subnet, SecurityGroup). Only available when a + // fulfillment-service connection and networking namespace are both configured; + // nil otherwise, in which case those controllers always use the legacy + // implementation-strategy path. + var resolver *dispatcher.Resolver if grpcConn != nil && networkingNamespace != "" { + disc, err := networkmanager.NewDiscovery(localMgr.GetClient(), networkingNamespace) + if err != nil { + return fmt.Errorf("network manager discovery: %w", err) + } + networkClassAdapter := dispatcheradapter.NewNetworkClassAdapter(privatev1.NewNetworkClassesClient(grpcConn)) + resolver = dispatcher.NewResolver(networkClassAdapter, disc) + if err := setupNetworkClassCapabilitiesController( - mgr, localMgr, grpcConn, networkingNamespace, + mgr, localMgr, grpcConn, networkingNamespace, resolver, ); err != nil { return err } @@ -518,19 +531,19 @@ func setupNetworkingControllers( if err := setupVirtualNetworkControllers( mgr, localMgr, grpcConn, networkingNamespace, - networkingProvider, statusPollInterval, maxJobHistory, targetCluster, + networkingProvider, statusPollInterval, maxJobHistory, targetCluster, resolver, ); err != nil { return err } if err := setupSubnetControllers( mgr, localMgr, grpcConn, networkingNamespace, - networkingProvider, statusPollInterval, maxJobHistory, targetCluster, + networkingProvider, statusPollInterval, maxJobHistory, targetCluster, resolver, ); err != nil { return err } if err := setupSecurityGroupControllers( mgr, localMgr, grpcConn, networkingNamespace, - networkingProvider, statusPollInterval, maxJobHistory, targetCluster, + networkingProvider, statusPollInterval, maxJobHistory, targetCluster, resolver, ); err != nil { return err } @@ -569,13 +582,9 @@ func setupNetworkingControllers( // is only called when grpcConn is set and a networking namespace is configured. func setupNetworkClassCapabilitiesController( mgr mcmanager.Manager, localMgr ctrl.Manager, grpcConn *grpc.ClientConn, networkingNamespace string, + resolver *dispatcher.Resolver, ) error { - disc, err := networkmanager.NewDiscovery(localMgr.GetClient(), networkingNamespace) - if err != nil { - return fmt.Errorf("network manager discovery: %w", err) - } networkClassesClient := privatev1.NewNetworkClassesClient(grpcConn) - resolver := dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(networkClassesClient), disc) ncReconciler := controller.NewNetworkClassCapabilitiesReconciler( networkClassesClient, resolver, networkingNamespace, @@ -599,6 +608,7 @@ func setupVirtualNetworkControllers( mgr mcmanager.Manager, localMgr ctrl.Manager, grpcConn *grpc.ClientConn, networkingNamespace string, provider provisioning.ProvisioningProvider, statusPollInterval time.Duration, maxJobHistory int, targetCluster multicluster.ClusterName, + resolver *dispatcher.Resolver, ) error { if grpcConn != nil { if err := controller.NewVirtualNetworkFeedbackReconciler( @@ -608,7 +618,7 @@ func setupVirtualNetworkControllers( } } if err := controller.NewVirtualNetworkReconciler( - mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, + mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, resolver, ).SetupWithManager(mgr); err != nil { return fmt.Errorf("virtualnetwork controller: %w", err) } @@ -619,6 +629,7 @@ func setupSubnetControllers( mgr mcmanager.Manager, localMgr ctrl.Manager, grpcConn *grpc.ClientConn, networkingNamespace string, provider provisioning.ProvisioningProvider, statusPollInterval time.Duration, maxJobHistory int, targetCluster multicluster.ClusterName, + resolver *dispatcher.Resolver, ) error { if grpcConn != nil { if err := controller.NewSubnetFeedbackReconciler( @@ -628,7 +639,7 @@ func setupSubnetControllers( } } if err := controller.NewSubnetReconciler( - mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, + mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, resolver, ).SetupWithManager(mgr); err != nil { return fmt.Errorf("subnet controller: %w", err) } @@ -639,6 +650,7 @@ func setupSecurityGroupControllers( mgr mcmanager.Manager, localMgr ctrl.Manager, grpcConn *grpc.ClientConn, networkingNamespace string, provider provisioning.ProvisioningProvider, statusPollInterval time.Duration, maxJobHistory int, targetCluster multicluster.ClusterName, + resolver *dispatcher.Resolver, ) error { if grpcConn != nil { if err := controller.NewSecurityGroupFeedbackReconciler( @@ -648,7 +660,7 @@ func setupSecurityGroupControllers( } } if err := controller.NewSecurityGroupReconciler( - mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, + mgr, networkingNamespace, provider, statusPollInterval, maxJobHistory, targetCluster, resolver, ).SetupWithManager(mgr); err != nil { return fmt.Errorf("securitygroup controller: %w", err) } diff --git a/osac-operator/internal/controller/constants_common.go b/osac-operator/internal/controller/constants_common.go index b0ac637e9..9926ddca4 100644 --- a/osac-operator/internal/controller/constants_common.go +++ b/osac-operator/internal/controller/constants_common.go @@ -42,9 +42,10 @@ const ( // Used by ExternalIPPool (from its own spec) and ExternalIP (inherited from parent pool). defaultExternalIPPoolImplementationStrategy = "metallb-l2" - // defaultSecurityGroupImplementationStrategy is the implementation strategy for SecurityGroup. - // SecurityGroup enforcement uses standard Kubernetes NetworkPolicy, independent of the - // VirtualNetwork's NetworkClass. + // defaultSecurityGroupImplementationStrategy is the fallback implementation strategy + // for SecurityGroup (standard Kubernetes NetworkPolicy) when neither the SecurityGroup + // spec nor the parent VirtualNetwork's NetworkClass (via the dispatcher path) resolve + // one. defaultSecurityGroupImplementationStrategy = "network_policy" conditionReasonConfigurationApplied = "ConfigurationApplied" diff --git a/osac-operator/internal/controller/dispatcher_helpers.go b/osac-operator/internal/controller/dispatcher_helpers.go new file mode 100644 index 000000000..38c6193c5 --- /dev/null +++ b/osac-operator/internal/controller/dispatcher_helpers.go @@ -0,0 +1,70 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "errors" + "fmt" + + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" +) + +// resolveImplementationStrategy determines the value a networking controller should +// write into osacImplementationStrategyAnnotation for AAP playbook selection. +// +// When resolver is configured (non-nil, i.e. the gRPC connection and networking +// namespace needed for manager discovery are set up) and networkClassID is non-empty, +// it resolves the NetworkClass's fabric manager via the dispatcher package (the +// "dispatcher path") and returns the resolved manager's name. If the NetworkClass has +// neither a fabricManager nor a k8sManager set yet (dispatcher.ErrNoManagerConfigured), +// it falls back to legacyStrategy (the "implementation_strategy annotation path"). Any +// other resolution error (e.g. a fabricManager referencing an unregistered manager +// ConfigMap) is returned to the caller as a real reconcile error, since that indicates +// a misconfiguration rather than an expected pre-migration state. +// +// When resolver is nil or networkClassID is empty, dispatch is skipped entirely and +// legacyStrategy is returned unchanged — this is the behavior for deployments without +// the two-manager model configured, or resources using the platform-default +// NetworkClass (which has no ID to resolve against). +func resolveImplementationStrategy( + ctx context.Context, + resolver *dispatcher.Resolver, + kind string, + networkClassID string, + legacyStrategy string, +) (string, error) { + if resolver == nil || networkClassID == "" { + return legacyStrategy, nil + } + + plan, err := dispatcher.NewDispatcher(resolver).Dispatch(ctx, kind, networkClassID) + switch { + case err == nil: + target := plan.FabricTarget() + if target == nil { + // Defensive: every entry in the dispatch table includes the fabric role, + // so this should not happen in practice. + return legacyStrategy, nil + } + return target.Manager.Name, nil + case errors.Is(err, dispatcher.ErrNoManagerConfigured): + return legacyStrategy, nil + default: + return "", fmt.Errorf("resolving dispatch plan for %s (networkClass %q): %w", kind, networkClassID, err) + } +} diff --git a/osac-operator/internal/controller/securitygroup_controller.go b/osac-operator/internal/controller/securitygroup_controller.go index bd56fddf9..d682760dd 100644 --- a/osac-operator/internal/controller/securitygroup_controller.go +++ b/osac-operator/internal/controller/securitygroup_controller.go @@ -34,6 +34,7 @@ import ( mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" "github.com/osac-project/osac/osac-operator/api/v1alpha1" + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" "github.com/osac-project/osac/osac-operator/pkg/provisioning" ) @@ -53,6 +54,10 @@ type SecurityGroupReconciler struct { StatusPollInterval time.Duration MaxJobHistory int targetCluster mc.ClusterName + // Resolver resolves a NetworkClass to its registered managers. Nil when the + // two-manager model isn't configured (no gRPC connection / networking namespace), + // in which case the controller always uses the legacy implementation-strategy path. + Resolver *dispatcher.Resolver } // NewSecurityGroupReconciler creates a new reconciler for SecurityGroup resources. @@ -63,6 +68,7 @@ func NewSecurityGroupReconciler( statusPollInterval time.Duration, maxJobHistory int, targetCluster mc.ClusterName, + resolver *dispatcher.Resolver, ) *SecurityGroupReconciler { if mgr == nil { panic("mgr must not be nil") @@ -83,6 +89,7 @@ func NewSecurityGroupReconciler( StatusPollInterval: statusPollInterval, MaxJobHistory: maxJobHistory, targetCluster: targetCluster, + Resolver: resolver, } } @@ -149,10 +156,36 @@ func (r *SecurityGroupReconciler) handleUpdate(ctx context.Context, sg *v1alpha1 sg.Status.Phase = v1alpha1.SecurityGroupPhaseProgressing } - // Read implementation strategy from spec (set by fulfillment-service), fall back to default - implementationStrategy := sg.Spec.ImplementationStrategy - if implementationStrategy == "" { - implementationStrategy = defaultSecurityGroupImplementationStrategy + // Look up the parent VirtualNetwork's NetworkClass to check whether it has a + // fabricManager registered (dispatcher path). + var networkClassID string + vnetList := &v1alpha1.VirtualNetworkList{} + if err := r.List(ctx, vnetList, + client.InNamespace(sg.Namespace), + client.MatchingLabels{osacVirtualNetworkIDLabel: sg.Spec.VirtualNetwork}, + ); err != nil { + return ctrl.Result{}, err + } else if len(vnetList.Items) > 1 { + return ctrl.Result{}, fmt.Errorf( + "expected exactly one parent VirtualNetwork with uuid %q but found %d", + sg.Spec.VirtualNetwork, len(vnetList.Items)) + } else if len(vnetList.Items) == 1 { + networkClassID = vnetList.Items[0].Spec.NetworkClass + } else { + log.Info("parent VirtualNetwork not found, using legacy implementation strategy", "uuid", sg.Spec.VirtualNetwork) + } + + // Read implementation strategy from spec (set by fulfillment-service), fall back to + // default. This is the legacy value; resolveImplementationStrategy below only uses + // it when the dispatcher path isn't available (see doc comment). + legacyStrategy := sg.Spec.ImplementationStrategy + if legacyStrategy == "" { + legacyStrategy = defaultSecurityGroupImplementationStrategy + } + + implementationStrategy, err := resolveImplementationStrategy(ctx, r.Resolver, "SecurityGroup", networkClassID, legacyStrategy) + if err != nil { + return ctrl.Result{}, err } // Add implementation-strategy annotation if not present or different diff --git a/osac-operator/internal/controller/securitygroup_controller_test.go b/osac-operator/internal/controller/securitygroup_controller_test.go index 6f2d3f3de..396fd2b8a 100644 --- a/osac-operator/internal/controller/securitygroup_controller_test.go +++ b/osac-operator/internal/controller/securitygroup_controller_test.go @@ -35,6 +35,10 @@ import ( mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" osacv1alpha1 "github.com/osac-project/osac/osac-operator/api/v1alpha1" + privatev1 "github.com/osac-project/osac/osac-operator/internal/api/osac/private/v1" + "github.com/osac-project/osac/osac-operator/internal/dispatcheradapter" + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" + "github.com/osac-project/osac/osac-operator/pkg/networkmanager" "github.com/osac-project/osac/osac-operator/pkg/provisioning" ) @@ -688,6 +692,140 @@ var _ = Describe("SecurityGroupReconciler", func() { }) }) + Context("dispatcher path", func() { + It("uses the resolved fabric manager name from the parent VirtualNetwork's NetworkClass", func() { + Expect(fakeClient.Create(ctx, newFabricManagerConfigMap("fm-netris", "test-namespace", "netris"))).To(Succeed()) + disc, err := networkmanager.NewDiscovery(fakeClient, "test-namespace") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-dispatch", FabricManager: "netris"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-dispatch" + Expect(fakeClient.Update(ctx, vnet)).To(Succeed()) + + key := types.NamespacedName{Name: sg.Name, Namespace: sg.Namespace} + mockProvider.triggerProvisionFunc = func(ctx context.Context, resource client.Object) (*provisioning.ProvisionResult, error) { + return &provisioning.ProvisionResult{JobID: "job-dispatch", InitialState: osacv1alpha1.JobStatePending}, nil + } + + // First reconcile adds finalizer, second sets annotation and provisions + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).NotTo(HaveOccurred()) + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.SecurityGroup{} + Expect(fakeClient.Get(ctx, key, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("netris")) + }) + + It("falls back to SecurityGroup's own legacy implementation strategy when fabricManager is not set", func() { + disc, err := networkmanager.NewDiscovery(fakeClient, "test-namespace") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-legacy"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-legacy" + Expect(fakeClient.Update(ctx, vnet)).To(Succeed()) + + // SecurityGroup's own legacy strategy is independent of the parent VNet's + // ImplementationStrategy — set a distinct value here to prove the fallback + // reads from the SecurityGroup spec, not the VirtualNetwork's. + sg.Spec.ImplementationStrategy = "custom-legacy" + Expect(fakeClient.Update(ctx, sg)).To(Succeed()) + + key := types.NamespacedName{Name: sg.Name, Namespace: sg.Namespace} + mockProvider.triggerProvisionFunc = func(ctx context.Context, resource client.Object) (*provisioning.ProvisionResult, error) { + return &provisioning.ProvisionResult{JobID: "job-legacy", InitialState: osacv1alpha1.JobStatePending}, nil + } + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.SecurityGroup{} + Expect(fakeClient.Get(ctx, key, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("custom-legacy")) + }) + + It("returns a reconcile error when the NetworkClass references an unregistered manager", func() { + disc, err := networkmanager.NewDiscovery(fakeClient, "test-namespace") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-broken", FabricManager: "does-not-exist"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-broken" + Expect(fakeClient.Update(ctx, vnet)).To(Succeed()) + + key := types.NamespacedName{Name: sg.Name, Namespace: sg.Namespace} + + // SecurityGroup resolves the dispatch plan on the very first reconcile + // (unlike VirtualNetwork/Subnet, finalizer-add doesn't return early here). + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).To(HaveOccurred()) + }) + + It("falls back to legacy strategy when the parent VirtualNetwork cannot be found", func() { + disc, err := networkmanager.NewDiscovery(fakeClient, "test-namespace") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + nil, &[]*privatev1.NetworkClass{}, + )), disc) + + orphanSG := &osacv1alpha1.SecurityGroup{ + ObjectMeta: metav1.ObjectMeta{Name: "orphan-sg", Namespace: "test-namespace"}, + Spec: osacv1alpha1.SecurityGroupSpec{ + VirtualNetwork: "no-such-vnet-uuid", + ImplementationStrategy: "custom-backend", + }, + } + Expect(fakeClient.Create(ctx, orphanSG)).To(Succeed()) + + key := types.NamespacedName{Name: orphanSG.Name, Namespace: orphanSG.Namespace} + mockProvider.triggerProvisionFunc = func(ctx context.Context, resource client.Object) (*provisioning.ProvisionResult, error) { + return &provisioning.ProvisionResult{JobID: "job-orphan", InitialState: osacv1alpha1.JobStatePending}, nil + } + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).NotTo(HaveOccurred()) + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.SecurityGroup{} + Expect(fakeClient.Get(ctx, key, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("custom-backend")) + }) + + It("returns an error when multiple VirtualNetworks share the parent uuid label", func() { + // Create a second VirtualNetwork with the same osacVirtualNetworkIDLabel as + // the fixture "vnet", simulating an ambiguous parent lookup. + duplicateVnet := &osacv1alpha1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vnet-duplicate", + Namespace: "test-namespace", + Labels: map[string]string{ + osacVirtualNetworkIDLabel: "test-vnet-uuid", + }, + }, + Spec: osacv1alpha1.VirtualNetworkSpec{ + Region: "us-west-1", + NetworkClass: "cudn-net", + ImplementationStrategy: "cudn-net", + }, + } + Expect(fakeClient.Create(ctx, duplicateVnet)).To(Succeed()) + + key := types.NamespacedName{Name: sg.Name, Namespace: sg.Namespace} + + // SecurityGroup resolves the dispatch plan on the very first reconcile + // (unlike VirtualNetwork/Subnet, finalizer-add doesn't return early here). + _, err := reconciler.Reconcile(ctx, mcreconcile.Request{Request: ctrl.Request{NamespacedName: key}}) + Expect(err).To(HaveOccurred()) + }) + }) + Context("provisioning condition updates", func() { It("should set Ready=False condition with error message when job fails", func() { sg.Status.ProvisioningJobs = []osacv1alpha1.JobStatus{ diff --git a/osac-operator/internal/controller/subnet_controller.go b/osac-operator/internal/controller/subnet_controller.go index 94a4b32c3..ef5dd3e3b 100644 --- a/osac-operator/internal/controller/subnet_controller.go +++ b/osac-operator/internal/controller/subnet_controller.go @@ -34,6 +34,7 @@ import ( mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" "github.com/osac-project/osac/osac-operator/api/v1alpha1" + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" "github.com/osac-project/osac/osac-operator/pkg/provisioning" ) @@ -54,6 +55,10 @@ type SubnetReconciler struct { StatusPollInterval time.Duration MaxJobHistory int targetCluster mc.ClusterName + // Resolver resolves a NetworkClass to its registered managers. Nil when the + // two-manager model isn't configured (no gRPC connection / networking namespace), + // in which case the controller always uses the legacy implementation-strategy path. + Resolver *dispatcher.Resolver } // NewSubnetReconciler creates a new reconciler for Subnet resources. @@ -64,6 +69,7 @@ func NewSubnetReconciler( statusPollInterval time.Duration, maxJobHistory int, targetCluster mc.ClusterName, + resolver *dispatcher.Resolver, ) *SubnetReconciler { if mgr == nil { panic("mgr must not be nil") @@ -84,6 +90,7 @@ func NewSubnetReconciler( StatusPollInterval: statusPollInterval, MaxJobHistory: maxJobHistory, targetCluster: targetCluster, + Resolver: resolver, } } @@ -183,10 +190,21 @@ func (r *SubnetReconciler) handleUpdate(ctx context.Context, subnet *v1alpha1.Su log.Info("parent VirtualNetwork not found, requeueing", "uuid", subnet.Spec.VirtualNetwork) return ctrl.Result{RequeueAfter: defaultPreconditionRequeueInterval}, nil } + if len(vnetList.Items) > 1 { + return ctrl.Result{}, fmt.Errorf( + "expected exactly one parent VirtualNetwork with uuid %q but found %d", + subnet.Spec.VirtualNetwork, len(vnetList.Items)) + } vnet := &vnetList.Items[0] - // Read implementation strategy from parent VirtualNetwork spec - implementationStrategy := vnet.Spec.ImplementationStrategy + // Determine implementation strategy: dispatcher path when the parent + // VirtualNetwork's NetworkClass has a fabricManager registered, else the legacy + // implementation_strategy annotation path (from the parent VirtualNetwork spec). + implementationStrategy, err := resolveImplementationStrategy( + ctx, r.Resolver, "Subnet", vnet.Spec.NetworkClass, vnet.Spec.ImplementationStrategy) + if err != nil { + return ctrl.Result{}, err + } if implementationStrategy == "" { log.Info("implementation strategy not set on parent VirtualNetwork, requeueing", "virtualNetwork", vnet.Name) return ctrl.Result{RequeueAfter: defaultPreconditionRequeueInterval}, nil diff --git a/osac-operator/internal/controller/subnet_controller_test.go b/osac-operator/internal/controller/subnet_controller_test.go index 438998a13..28b366969 100644 --- a/osac-operator/internal/controller/subnet_controller_test.go +++ b/osac-operator/internal/controller/subnet_controller_test.go @@ -22,15 +22,22 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" osacv1alpha1 "github.com/osac-project/osac/osac-operator/api/v1alpha1" + privatev1 "github.com/osac-project/osac/osac-operator/internal/api/osac/private/v1" + "github.com/osac-project/osac/osac-operator/internal/dispatcheradapter" + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" + "github.com/osac-project/osac/osac-operator/pkg/networkmanager" "github.com/osac-project/osac/osac-operator/pkg/provisioning" ) @@ -220,6 +227,39 @@ var _ = Describe("SubnetReconciler", func() { _ = k8sClient.Delete(ctx, subnetNoParent) }) + It("should return an error when multiple VirtualNetworks share the parent uuid label", func() { + // Create a second VirtualNetwork with the same osacVirtualNetworkIDLabel as + // the fixture "vnet" created in BeforeEach, simulating an ambiguous parent lookup. + duplicateVnet := &osacv1alpha1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-vnet-duplicate", + Namespace: "default", + Labels: map[string]string{ + osacVirtualNetworkIDLabel: "test-vnet-uuid", + }, + }, + Spec: osacv1alpha1.VirtualNetworkSpec{ + Region: "us-west-1", + IPv4CIDR: "10.9.0.0/16", + NetworkClass: "cudn-net", + ImplementationStrategy: "cudn-net", + }, + } + Expect(k8sClient.Create(ctx, duplicateVnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, duplicateVnet) + + Expect(k8sClient.Create(ctx, subnet)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: subnet.Name, + Namespace: subnet.Namespace, + }, + }}) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("expected exactly one parent VirtualNetwork")) + }) + It("should requeue when parent VirtualNetwork has no ImplementationStrategy", func() { // Create VirtualNetwork without ImplementationStrategy vnetNoStrategy := &osacv1alpha1.VirtualNetwork{ @@ -338,6 +378,133 @@ var _ = Describe("SubnetReconciler", func() { }) }) + Context("dispatcher path", func() { + var fakeDiscoveryClient client.Client + + BeforeEach(func() { + scheme := runtime.NewScheme() + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + fakeDiscoveryClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects( + newFabricManagerConfigMap("fm-netris", "osac", "netris"), + ).Build() + }) + + It("uses the resolved fabric manager name from the parent VirtualNetwork's NetworkClass", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-dispatch", FabricManager: "netris"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + dispatchVnet := &osacv1alpha1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dispatch-vnet", + Namespace: "default", + Labels: map[string]string{osacVirtualNetworkIDLabel: "dispatch-vnet-uuid"}, + }, + Spec: osacv1alpha1.VirtualNetworkSpec{ + Region: "us-west-1", + IPv4CIDR: "10.1.0.0/16", + NetworkClass: "nc-dispatch", + ImplementationStrategy: "legacy-value", + }, + } + Expect(k8sClient.Create(ctx, dispatchVnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, dispatchVnet) + + dispatchSubnet := &osacv1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "dispatch-subnet", Namespace: "default"}, + Spec: osacv1alpha1.SubnetSpec{VirtualNetwork: "dispatch-vnet-uuid", IPv4CIDR: "10.1.1.0/24"}, + } + Expect(k8sClient.Create(ctx, dispatchSubnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, dispatchSubnet) + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: dispatchSubnet.Name, Namespace: dispatchSubnet.Namespace}, + }}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.Subnet{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dispatchSubnet.Name, Namespace: dispatchSubnet.Namespace}, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("netris")) + }) + + It("falls back to the parent VirtualNetwork's legacy implementation strategy when fabricManager is not set", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-legacy"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + dispatchVnet := &osacv1alpha1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "legacy-vnet", + Namespace: "default", + Labels: map[string]string{osacVirtualNetworkIDLabel: "legacy-vnet-uuid"}, + }, + Spec: osacv1alpha1.VirtualNetworkSpec{ + Region: "us-west-1", + IPv4CIDR: "10.2.0.0/16", + NetworkClass: "nc-legacy", + ImplementationStrategy: "cudn-net", + }, + } + Expect(k8sClient.Create(ctx, dispatchVnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, dispatchVnet) + + dispatchSubnet := &osacv1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "legacy-subnet", Namespace: "default"}, + Spec: osacv1alpha1.SubnetSpec{VirtualNetwork: "legacy-vnet-uuid", IPv4CIDR: "10.2.1.0/24"}, + } + Expect(k8sClient.Create(ctx, dispatchSubnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, dispatchSubnet) + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: dispatchSubnet.Name, Namespace: dispatchSubnet.Namespace}, + }}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.Subnet{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: dispatchSubnet.Name, Namespace: dispatchSubnet.Namespace}, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("cudn-net")) + }) + + It("returns a reconcile error when the NetworkClass references an unregistered manager", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-broken", FabricManager: "does-not-exist"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + dispatchVnet := &osacv1alpha1.VirtualNetwork{ + ObjectMeta: metav1.ObjectMeta{ + Name: "broken-vnet", + Namespace: "default", + Labels: map[string]string{osacVirtualNetworkIDLabel: "broken-vnet-uuid"}, + }, + Spec: osacv1alpha1.VirtualNetworkSpec{ + Region: "us-west-1", + IPv4CIDR: "10.3.0.0/16", + NetworkClass: "nc-broken", + }, + } + Expect(k8sClient.Create(ctx, dispatchVnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, dispatchVnet) + + dispatchSubnet := &osacv1alpha1.Subnet{ + ObjectMeta: metav1.ObjectMeta{Name: "broken-subnet", Namespace: "default"}, + Spec: osacv1alpha1.SubnetSpec{VirtualNetwork: "broken-vnet-uuid", IPv4CIDR: "10.3.1.0/24"}, + } + Expect(k8sClient.Create(ctx, dispatchSubnet)).To(Succeed()) + DeferCleanup(deleteObjectWithClearedFinalizers, ctx, dispatchSubnet) + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: dispatchSubnet.Name, Namespace: dispatchSubnet.Namespace}, + }}) + Expect(err).To(HaveOccurred()) + }) + }) + Context("handleProvisioning", func() { BeforeEach(func() { subnet.Status.Phase = osacv1alpha1.SubnetPhaseProgressing @@ -708,6 +875,24 @@ var _ = Describe("SubnetReconciler", func() { }) }) +// deleteObjectWithClearedFinalizers deletes obj, first clearing any finalizers it has +// so it doesn't remain stuck in Terminating state in the shared envtest API server. +// Intended for use with Ginkgo's DeferCleanup. +func deleteObjectWithClearedFinalizers(ctx context.Context, obj client.Object) { + key := client.ObjectKeyFromObject(obj) + if err := k8sClient.Get(ctx, key, obj); err != nil { + if errors.IsNotFound(err) { + return + } + Expect(err).NotTo(HaveOccurred()) + } + if len(obj.GetFinalizers()) > 0 { + obj.SetFinalizers(nil) + Expect(k8sClient.Update(ctx, obj)).To(Succeed()) + } + Expect(k8sClient.Delete(ctx, obj)).To(Succeed()) +} + // mockSubnetProvider implements the ProvisioningProvider interface for Subnet testing type mockSubnetProvider struct { triggerProvisionFunc func(ctx context.Context, resource client.Object) (*provisioning.ProvisionResult, error) diff --git a/osac-operator/internal/controller/virtualnetwork_controller.go b/osac-operator/internal/controller/virtualnetwork_controller.go index 0b1163641..51dd6453c 100644 --- a/osac-operator/internal/controller/virtualnetwork_controller.go +++ b/osac-operator/internal/controller/virtualnetwork_controller.go @@ -35,6 +35,7 @@ import ( mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" "github.com/osac-project/osac/osac-operator/api/v1alpha1" + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" "github.com/osac-project/osac/osac-operator/pkg/provisioning" ) @@ -54,6 +55,10 @@ type VirtualNetworkReconciler struct { StatusPollInterval time.Duration MaxJobHistory int targetCluster mc.ClusterName + // Resolver resolves a NetworkClass to its registered managers. Nil when the + // two-manager model isn't configured (no gRPC connection / networking namespace), + // in which case the controller always uses the legacy implementation-strategy path. + Resolver *dispatcher.Resolver } // NewVirtualNetworkReconciler creates a new reconciler for VirtualNetwork resources. @@ -64,6 +69,7 @@ func NewVirtualNetworkReconciler( statusPollInterval time.Duration, maxJobHistory int, targetCluster mc.ClusterName, + resolver *dispatcher.Resolver, ) *VirtualNetworkReconciler { if mgr == nil { panic("mgr must not be nil") @@ -84,6 +90,7 @@ func NewVirtualNetworkReconciler( StatusPollInterval: statusPollInterval, MaxJobHistory: maxJobHistory, targetCluster: targetCluster, + Resolver: resolver, } } @@ -148,8 +155,14 @@ func (r *VirtualNetworkReconciler) handleUpdate(ctx context.Context, vnet *v1alp vnet.Status.Phase = v1alpha1.VirtualNetworkPhaseProgressing } - // Read implementation strategy from spec (populated by fulfillment-service from NetworkClass) - implementationStrategy := vnet.Spec.ImplementationStrategy + // Determine implementation strategy: dispatcher path when the NetworkClass has a + // fabricManager registered, else the legacy implementation_strategy annotation path + // (populated by fulfillment-service from NetworkClass). + implementationStrategy, err := resolveImplementationStrategy( + ctx, r.Resolver, "VirtualNetwork", vnet.Spec.NetworkClass, vnet.Spec.ImplementationStrategy) + if err != nil { + return ctrl.Result{}, err + } if implementationStrategy == "" { log.Info("implementation strategy not set, requeueing", "virtualNetwork", vnet.Name) return ctrl.Result{RequeueAfter: defaultPreconditionRequeueInterval}, nil @@ -169,8 +182,11 @@ func (r *VirtualNetworkReconciler) handleUpdate(ctx context.Context, vnet *v1alp return ctrl.Result{}, nil } - // Compute desired config version from spec - desiredVersion, err := provisioning.ComputeDesiredConfigVersion(vnet.Spec) + // Compute desired config version from spec and inherited implementation strategy + desiredVersion, err := provisioning.ComputeDesiredConfigVersion(struct { + Spec v1alpha1.VirtualNetworkSpec + ImplementationStrategy string + }{vnet.Spec, implementationStrategy}) if err != nil { return ctrl.Result{}, fmt.Errorf("failed to compute desired config version: %w", err) } diff --git a/osac-operator/internal/controller/virtualnetwork_controller_test.go b/osac-operator/internal/controller/virtualnetwork_controller_test.go index dafff37d2..13c1cc1c4 100644 --- a/osac-operator/internal/controller/virtualnetwork_controller_test.go +++ b/osac-operator/internal/controller/virtualnetwork_controller_test.go @@ -22,15 +22,22 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/reconcile" mcreconcile "sigs.k8s.io/multicluster-runtime/pkg/reconcile" osacv1alpha1 "github.com/osac-project/osac/osac-operator/api/v1alpha1" + privatev1 "github.com/osac-project/osac/osac-operator/internal/api/osac/private/v1" + "github.com/osac-project/osac/osac-operator/internal/dispatcheradapter" + "github.com/osac-project/osac/osac-operator/pkg/dispatcher" + "github.com/osac-project/osac/osac-operator/pkg/networkmanager" "github.com/osac-project/osac/osac-operator/pkg/provisioning" ) @@ -662,6 +669,159 @@ var _ = Describe("VirtualNetworkReconciler", func() { }) }) + Context("dispatcher path", func() { + var fakeDiscoveryClient client.Client + + BeforeEach(func() { + scheme := runtime.NewScheme() + Expect(corev1.AddToScheme(scheme)).To(Succeed()) + fakeDiscoveryClient = fake.NewClientBuilder().WithScheme(scheme).WithObjects( + newFabricManagerConfigMap("fm-netris", "osac", "netris"), + ).Build() + }) + + It("uses the resolved fabric manager name when the NetworkClass has fabricManager set", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-dispatch", FabricManager: "netris"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-dispatch" + vnet.Spec.ImplementationStrategy = "legacy-value" + Expect(k8sClient.Create(ctx, vnet)).To(Succeed()) + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, + }}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.VirtualNetwork{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("netris")) + }) + + It("falls back to the legacy implementation-strategy path when fabricManager is not set", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-legacy"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-legacy" + vnet.Spec.ImplementationStrategy = "cudn-net" + Expect(k8sClient.Create(ctx, vnet)).To(Succeed()) + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, + }}) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.VirtualNetwork{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("cudn-net")) + }) + + It("returns a reconcile error when the NetworkClass references an unregistered manager", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-broken", FabricManager: "does-not-exist"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-broken" + Expect(k8sClient.Create(ctx, vnet)).To(Succeed()) + + _, err = reconciler.Reconcile(ctx, mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, + }}) + Expect(err).To(HaveOccurred()) + }) + + It("triggers a new provisioning job when the resolved strategy changes with the spec unchanged", func() { + disc, err := networkmanager.NewDiscovery(fakeDiscoveryClient, "osac") + Expect(err).NotTo(HaveOccurred()) + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-dispatch"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + vnet.Spec.NetworkClass = "nc-dispatch" + vnet.Spec.ImplementationStrategy = "cudn-net" + Expect(k8sClient.Create(ctx, vnet)).To(Succeed()) + + req := mcreconcile.Request{Request: reconcile.Request{ + NamespacedName: types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, + }} + + // First reconcile: adds finalizer and sets the legacy strategy annotation + // (the NetworkClass has no fabricManager registered yet). + _, err = reconciler.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + + // Second reconcile: triggers the initial provisioning job under the legacy strategy. + mockProvider.triggerProvisionFunc = func(_ context.Context, _ client.Object) (*provisioning.ProvisionResult, error) { + return &provisioning.ProvisionResult{ + JobID: "job-before-strategy-change", + InitialState: osacv1alpha1.JobStatePending, + Message: "Provisioning triggered", + }, nil + } + _, err = reconciler.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + + beforeVnet := &osacv1alpha1.VirtualNetwork{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, beforeVnet)).To(Succeed()) + Expect(beforeVnet.Annotations[osacImplementationStrategyAnnotation]).To(Equal("cudn-net")) + versionBefore := beforeVnet.Status.DesiredConfigVersion + Expect(versionBefore).NotTo(BeEmpty()) + jobBefore := provisioning.FindJobByID(beforeVnet.Status.ProvisioningJobs, "job-before-strategy-change") + Expect(jobBefore).NotTo(BeNil()) + + // Mark the existing job as succeeded at the current desired version, mirroring a + // VirtualNetwork that has already been successfully provisioned under the legacy path. + beforeVnet.Status.Phase = osacv1alpha1.VirtualNetworkPhaseReady + jobBefore.State = osacv1alpha1.JobStateSucceeded + jobBefore.ConfigVersion = versionBefore + Expect(k8sClient.Status().Update(ctx, beforeVnet)).To(Succeed()) + + // Simulate the NetworkClass being updated to register a fabricManager. The + // VirtualNetwork's spec is untouched — only the dynamically-resolved strategy changes. + reconciler.Resolver = dispatcher.NewResolver(dispatcheradapter.NewNetworkClassAdapter(newListingNetworkClassClient( + []*privatev1.NetworkClass{{Id: "nc-dispatch", FabricManager: "netris"}}, &[]*privatev1.NetworkClass{}, + )), disc) + + // Third reconcile: updates the annotation to the newly-resolved manager and requeues. + _, err = reconciler.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + + updated := &osacv1alpha1.VirtualNetwork{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, updated)).To(Succeed()) + Expect(updated.Annotations[osacImplementationStrategyAnnotation]).To(Equal("netris")) + + // Fourth reconcile: the resolved strategy changed with the spec unchanged, so a new + // desired config version — and a new provisioning job — must be produced. + mockProvider.triggerProvisionFunc = func(_ context.Context, _ client.Object) (*provisioning.ProvisionResult, error) { + return &provisioning.ProvisionResult{ + JobID: "job-after-strategy-change", + InitialState: osacv1alpha1.JobStatePending, + Message: "Provisioning triggered", + }, nil + } + _, err = reconciler.Reconcile(ctx, req) + Expect(err).NotTo(HaveOccurred()) + + afterVnet := &osacv1alpha1.VirtualNetwork{} + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: vnet.Name, Namespace: vnet.Namespace}, afterVnet)).To(Succeed()) + Expect(afterVnet.Status.DesiredConfigVersion).NotTo(Equal(versionBefore), + "desired config version must change when the resolved strategy changes, even with an unchanged spec") + // Look up by ID rather than FindLatestJobByType: both jobs may land in the + // same envtest second, and JobStatus.Timestamp only has second resolution. + jobAfter := provisioning.FindJobByID(afterVnet.Status.ProvisioningJobs, "job-after-strategy-change") + Expect(jobAfter).NotTo(BeNil(), + "a new provisioning job must be triggered when the resolved strategy changes") + }) + }) + Context("Phase transitions", func() { It("should transition from Progressing to Ready on success", func() { vnet.Status.Phase = osacv1alpha1.VirtualNetworkPhaseProgressing