From 5c3ad4b716287120add9e333d6359f7e62b03a4b Mon Sep 17 00:00:00 2001 From: ushabelgur Date: Tue, 18 Feb 2025 15:00:14 +0530 Subject: [PATCH 1/4] introduce flag to controll NetworkPeeringControllingType --- cmd/metalnetlet/main.go | 14 +- .../controllers/controllers_suite_test.go | 49 +++++++ metalnetlet/controllers/network_controller.go | 61 +++++---- .../controllers/network_controller_test.go | 129 ++++++++++++++++++ 4 files changed, 225 insertions(+), 28 deletions(-) diff --git a/cmd/metalnetlet/main.go b/cmd/metalnetlet/main.go index c3c78aec..13aad8bd 100644 --- a/cmd/metalnetlet/main.go +++ b/cmd/metalnetlet/main.go @@ -57,6 +57,7 @@ func main() { var configOptions config.GetConfigOptions var metalnetKubeconfig string var metalnetNamespace string + var networkPeeringControllingBehavior string flag.StringVar(&name, "name", "", "The name of the partition the metalnetlet represents (required).") flag.StringToStringVar(&nodeLabels, "node-label", nodeLabels, "Additional labels to add to the nodes.") @@ -69,6 +70,10 @@ func main() { configOptions.BindFlags(flag.CommandLine) flag.StringVar(&metalnetKubeconfig, "metalnet-kubeconfig", "", "Metalnet kubeconfig to use.") flag.StringVar(&metalnetNamespace, "metalnet-namespace", corev1.NamespaceDefault, "Metalnet namespace to use.") + flag.StringVar(&networkPeeringControllingBehavior, "network-peering-controlling-behavior", "Native", + "Whether to use metalnet for populating the peered prefixes or not. "+ + "If unset or 'Native' is passed metalnetlet will populate the peered prefixes for the lowlevel Network resources."+ + "If 'None' is passed, metalnetlet will not populate any peered prefixes for the metalnet-related Network resources.") opts := zap.Options{ Development: true, @@ -153,10 +158,11 @@ func main() { } if err := (&controllers.NetworkReconciler{ - Client: mgr.GetClient(), - MetalnetClient: metalnetCluster.GetClient(), - PartitionName: name, - MetalnetNamespace: metalnetNamespace, + Client: mgr.GetClient(), + MetalnetClient: metalnetCluster.GetClient(), + PartitionName: name, + MetalnetNamespace: metalnetNamespace, + NetworkPeeringControllingType: controllers.NetworkPeeringControllingType(networkPeeringControllingBehavior), }).SetupWithManager(mgr, metalnetCluster.GetCache()); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Network") os.Exit(1) diff --git a/metalnetlet/controllers/controllers_suite_test.go b/metalnetlet/controllers/controllers_suite_test.go index 7e6dd866..42d36fcb 100644 --- a/metalnetlet/controllers/controllers_suite_test.go +++ b/metalnetlet/controllers/controllers_suite_test.go @@ -186,6 +186,55 @@ func SetupTest(metalnetNs *corev1.Namespace) { }) } +func SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs *corev1.Namespace) { + BeforeEach(func(ctx SpecContext) { + k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + Metrics: metricsserver.Options{ + BindAddress: "0", + }, + }) + Expect(err).ToNot(HaveOccurred()) + + // register reconciler here + Expect((&NetworkReconciler{ + Client: k8sManager.GetClient(), + MetalnetClient: k8sManager.GetClient(), + PartitionName: partitionName, + MetalnetNamespace: metalnetNs.Name, + NetworkPeeringControllingType: NetworkPeeringControllingTypeNone, + }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) + + Expect((&MetalnetNodeReconciler{ + Client: k8sManager.GetClient(), + MetalnetClient: k8sManager.GetClient(), + PartitionName: partitionName, + NodeLabels: nodeLabels, + }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) + + Expect((&NetworkInterfaceReconciler{ + Client: k8sManager.GetClient(), + MetalnetClient: k8sManager.GetClient(), + PartitionName: partitionName, + MetalnetNamespace: metalnetNs.Name, + }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) + + Expect((&InstanceReconciler{ + Client: k8sManager.GetClient(), + MetalnetClient: k8sManager.GetClient(), + PartitionName: partitionName, + MetalnetNamespace: metalnetNs.Name, + }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) + + mgrCtx, cancel := context.WithCancel(context.Background()) + DeferCleanup(cancel) + go func() { + defer GinkgoRecover() + Expect(k8sManager.Start(mgrCtx)).To(Succeed(), "failed to start manager") + }() + }) +} + func SetupMetalnetNode() *corev1.Node { return SetupObjectStruct[*corev1.Node](&k8sClient, func(node *corev1.Node) { *node = corev1.Node{ diff --git a/metalnetlet/controllers/network_controller.go b/metalnetlet/controllers/network_controller.go index 43a4abba..289f40f4 100644 --- a/metalnetlet/controllers/network_controller.go +++ b/metalnetlet/controllers/network_controller.go @@ -26,6 +26,13 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) +type NetworkPeeringControllingType string + +const ( + NetworkPeeringControllingTypeNative = "Native" + NetworkPeeringControllingTypeNone = "None" +) + type NetworkReconciler struct { client.Client MetalnetClient client.Client @@ -33,6 +40,8 @@ type NetworkReconciler struct { PartitionName string MetalnetNamespace string + + NetworkPeeringControllingType NetworkPeeringControllingType } //+kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -114,18 +123,20 @@ func (r *NetworkReconciler) delete(ctx context.Context, log logr.Logger, network } func (r *NetworkReconciler) updateApinetNetworkStatus(ctx context.Context, log logr.Logger, network *apinetv1alpha1.Network, metalnetNetwork *metalnetv1alpha1.Network) error { - apinetStatusPeerings := metalnetNetworkPeeringsStatusToNetworkPeeringsStatus(metalnetNetwork.Status.Peerings) - if !equality.Semantic.DeepEqual(network.Status.Peerings[r.PartitionName], apinetStatusPeerings) { - log.V(1).Info("Patching apinet network status", "status", apinetStatusPeerings) - networkBase := network.DeepCopy() - if network.Status.Peerings == nil { - network.Status.Peerings = make(map[string][]apinetv1alpha1.NetworkPeeringStatus) - } - network.Status.Peerings[r.PartitionName] = apinetStatusPeerings - if err := r.Status().Patch(ctx, network, client.MergeFrom(networkBase)); err != nil { - return fmt.Errorf("unable to patch network: %w", err) + if r.NetworkPeeringControllingType != NetworkPeeringControllingTypeNone { + apinetStatusPeerings := metalnetNetworkPeeringsStatusToNetworkPeeringsStatus(metalnetNetwork.Status.Peerings) + if !equality.Semantic.DeepEqual(network.Status.Peerings[r.PartitionName], apinetStatusPeerings) { + log.V(1).Info("Patching apinet network status", "status", apinetStatusPeerings) + networkBase := network.DeepCopy() + if network.Status.Peerings == nil { + network.Status.Peerings = make(map[string][]apinetv1alpha1.NetworkPeeringStatus) + } + network.Status.Peerings[r.PartitionName] = apinetStatusPeerings + if err := r.Status().Patch(ctx, network, client.MergeFrom(networkBase)); err != nil { + return fmt.Errorf("unable to patch network: %w", err) + } + log.V(1).Info("Patched apinet network status") } - log.V(1).Info("Patched apinet network status") } return nil } @@ -178,22 +189,24 @@ func (r *NetworkReconciler) reconcile(ctx context.Context, log logr.Logger, netw } var peeredIDs []int32 var peeredPrefixes []metalnetv1alpha1.PeeredPrefix - for _, peering := range network.Spec.Peerings { - id, err := networkid.ParseVNI(peering.ID) - if err != nil { - return ctrl.Result{}, fmt.Errorf("failed to parse peered network ID: %w", err) - } + if r.NetworkPeeringControllingType != NetworkPeeringControllingTypeNone { + for _, peering := range network.Spec.Peerings { + id, err := networkid.ParseVNI(peering.ID) + if err != nil { + return ctrl.Result{}, fmt.Errorf("failed to parse peered network ID: %w", err) + } - // metalnetNetwork.Spec.PeeredIDs = append(metalnetNetwork.Spec.PeeredIDs, id) - peeredIDs = append(peeredIDs, id) + // metalnetNetwork.Spec.PeeredIDs = append(metalnetNetwork.Spec.PeeredIDs, id) + peeredIDs = append(peeredIDs, id) - if len(peering.Prefixes) > 0 { - ipPrefixes := getIPPrefixes(peering.Prefixes) - peeredPrefix := metalnetv1alpha1.PeeredPrefix{ - ID: id, - Prefixes: ipPrefixesToMetalnetPrefixes(ipPrefixes), + if len(peering.Prefixes) > 0 { + ipPrefixes := getIPPrefixes(peering.Prefixes) + peeredPrefix := metalnetv1alpha1.PeeredPrefix{ + ID: id, + Prefixes: ipPrefixesToMetalnetPrefixes(ipPrefixes), + } + peeredPrefixes = append(peeredPrefixes, peeredPrefix) } - peeredPrefixes = append(peeredPrefixes, peeredPrefix) } } // metalnetNetwork.Spec.PeeredPrefixes = peeredPrefixes diff --git a/metalnetlet/controllers/network_controller_test.go b/metalnetlet/controllers/network_controller_test.go index bbac3c9b..db61ebf7 100644 --- a/metalnetlet/controllers/network_controller_test.go +++ b/metalnetlet/controllers/network_controller_test.go @@ -140,4 +140,133 @@ var _ = Describe("NetworkController", func() { Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(metalnetNetwork2), metalnetNetwork2)).To(Satisfy(apierrors.IsNotFound)) }) + + var _ = Describe("NetworkPeeringController", func() { + ns := SetupNamespace(&k8sClient) + metalnetNs := SetupNamespace(&k8sClient) + SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs) + + It("should create metalnet networks for apinet networks without peerings information if NetworkPeeringControllingType is set to None", func(ctx SpecContext) { + By("creating a apinet network-1") + network1 := &apinetv1alpha1.Network{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "network-1", + }, + } + Expect(k8sClient.Create(ctx, network1)).To(Succeed()) + + By("creating a apinet network-2") + network2 := &apinetv1alpha1.Network{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: ns.Name, + Name: "network-2", + }, + } + Expect(k8sClient.Create(ctx, network2)).To(Succeed()) + + By("updating apinet networks spec with peerings") + baseNetwork1 := network1.DeepCopy() + network1.Spec.Peerings = []apinetv1alpha1.NetworkPeering{{ + Name: "peering-1", + Prefixes: []apinetv1alpha1.PeeringPrefix{{ + Name: "my-prefix", + Prefix: net.MustParseNewIPPrefix("10.0.0.0/24")}}, + ID: network2.Spec.ID}} + Expect(k8sClient.Patch(ctx, network1, client.MergeFrom(baseNetwork1))).To(Succeed()) + + baseNetwork2 := network2.DeepCopy() + network2.Spec.Peerings = []apinetv1alpha1.NetworkPeering{{ + Name: "peering-1", + ID: network1.Spec.ID}} + Expect(k8sClient.Patch(ctx, network2, client.MergeFrom(baseNetwork2))).To(Succeed()) + + By("parsing the VNI of network-1") + network1Vni, err := networkid.ParseVNI(network1.Spec.ID) + Expect(err).NotTo(HaveOccurred()) + + By("parsing the VNI of network-2") + network2Vni, err := networkid.ParseVNI(network2.Spec.ID) + Expect(err).NotTo(HaveOccurred()) + + By("waiting for the metalnet networks to be created") + metalnetNetwork1 := &metalnetv1alpha1.Network{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metalnetNs.Name, + Name: string(network1.UID), + }, + } + Eventually(Object(metalnetNetwork1)).Should(SatisfyAll( + HaveField("Spec", metalnetv1alpha1.NetworkSpec{ + ID: network1Vni, + }), + )) + + By("validating metalnet network spec is not updated with peering information") + Consistently(Object(metalnetNetwork1)).Should(SatisfyAll( + HaveField("Spec", metalnetv1alpha1.NetworkSpec{ + ID: network1Vni, + PeeredIDs: nil, + PeeredPrefixes: nil, + }), + )) + + metalnetNetwork2 := &metalnetv1alpha1.Network{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metalnetNs.Name, + Name: string(network2.UID), + }, + } + Eventually(Object(metalnetNetwork2)).Should(SatisfyAll( + HaveField("Spec", metalnetv1alpha1.NetworkSpec{ + ID: network2Vni, + }), + )) + + Consistently(Object(metalnetNetwork2)).Should(SatisfyAll( + HaveField("Spec", metalnetv1alpha1.NetworkSpec{ + ID: network2Vni, + PeeredIDs: nil, + PeeredPrefixes: nil, + }), + )) + + By("updating status of metalnet network peerings") + Eventually(UpdateStatus(metalnetNetwork1, func() { + metalnetNetwork1.Status.Peerings = []metalnetv1alpha1.NetworkPeeringStatus{{ + ID: network2Vni, + State: metalnetv1alpha1.NetworkPeeringStateReady, + }} + })).Should(Succeed()) + + Eventually(UpdateStatus(metalnetNetwork2, func() { + metalnetNetwork2.Status.Peerings = []metalnetv1alpha1.NetworkPeeringStatus{{ + ID: network1Vni, + State: metalnetv1alpha1.NetworkPeeringStateReady, + }} + })).Should(Succeed()) + + By("ensuring apinet network status peerings are not updated") + Consistently(Object(network1)).Should(SatisfyAll( + HaveField("Status.Peerings", BeEmpty()), + )) + + Consistently(Object(network2)).Should(SatisfyAll( + HaveField("Status.Peerings", BeEmpty()), + )) + + By("deleting the networks") + Expect(k8sClient.Delete(ctx, network1)).To(Succeed()) + Expect(k8sClient.Delete(ctx, network2)).To(Succeed()) + + By("waiting for networks to be gone") + Eventually(Get(network1)).Should(Satisfy(apierrors.IsNotFound)) + Eventually(Get(network2)).Should(Satisfy(apierrors.IsNotFound)) + + By("asserting the corresponding apinet network is gone as well") + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(metalnetNetwork1), metalnetNetwork1)).To(Satisfy(apierrors.IsNotFound)) + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(metalnetNetwork2), metalnetNetwork2)).To(Satisfy(apierrors.IsNotFound)) + + }) + }) }) From f969d5ca9b61f605292e08a415f3d5f68cb727a6 Mon Sep 17 00:00:00 2001 From: ushabelgur Date: Tue, 25 Feb 2025 10:48:13 +0530 Subject: [PATCH 2/4] rename flags --- cmd/metalnetlet/main.go | 10 +++++----- metalnetlet/controllers/controllers_suite_test.go | 2 +- metalnetlet/controllers/network_controller.go | 12 ++++++------ metalnetlet/controllers/network_controller_test.go | 3 +-- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/cmd/metalnetlet/main.go b/cmd/metalnetlet/main.go index 13aad8bd..0e8f93d3 100644 --- a/cmd/metalnetlet/main.go +++ b/cmd/metalnetlet/main.go @@ -57,7 +57,7 @@ func main() { var configOptions config.GetConfigOptions var metalnetKubeconfig string var metalnetNamespace string - var networkPeeringControllingBehavior string + var networkPeeringProvider string flag.StringVar(&name, "name", "", "The name of the partition the metalnetlet represents (required).") flag.StringToStringVar(&nodeLabels, "node-label", nodeLabels, "Additional labels to add to the nodes.") @@ -70,10 +70,10 @@ func main() { configOptions.BindFlags(flag.CommandLine) flag.StringVar(&metalnetKubeconfig, "metalnet-kubeconfig", "", "Metalnet kubeconfig to use.") flag.StringVar(&metalnetNamespace, "metalnet-namespace", corev1.NamespaceDefault, "Metalnet namespace to use.") - flag.StringVar(&networkPeeringControllingBehavior, "network-peering-controlling-behavior", "Native", + flag.StringVar(&networkPeeringProvider, "network-peering-provider", "BuiltIn", "Whether to use metalnet for populating the peered prefixes or not. "+ - "If unset or 'Native' is passed metalnetlet will populate the peered prefixes for the lowlevel Network resources."+ - "If 'None' is passed, metalnetlet will not populate any peered prefixes for the metalnet-related Network resources.") + "If unset or 'BuiltIn' is passed metalnetlet will populate the peered prefixes for the lowlevel Network resources."+ + "If 'External' is passed, metalnetlet will not populate any peered prefixes for the metalnet-related Network resources.") opts := zap.Options{ Development: true, @@ -162,7 +162,7 @@ func main() { MetalnetClient: metalnetCluster.GetClient(), PartitionName: name, MetalnetNamespace: metalnetNamespace, - NetworkPeeringControllingType: controllers.NetworkPeeringControllingType(networkPeeringControllingBehavior), + NetworkPeeringControllingType: controllers.NetworkPeeringProviderType(networkPeeringProvider), }).SetupWithManager(mgr, metalnetCluster.GetCache()); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Network") os.Exit(1) diff --git a/metalnetlet/controllers/controllers_suite_test.go b/metalnetlet/controllers/controllers_suite_test.go index 42d36fcb..fa5ad05a 100644 --- a/metalnetlet/controllers/controllers_suite_test.go +++ b/metalnetlet/controllers/controllers_suite_test.go @@ -202,7 +202,7 @@ func SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs *corev1.Namespace MetalnetClient: k8sManager.GetClient(), PartitionName: partitionName, MetalnetNamespace: metalnetNs.Name, - NetworkPeeringControllingType: NetworkPeeringControllingTypeNone, + NetworkPeeringControllingType: ExternalNetworkPeeringProvider, }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) Expect((&MetalnetNodeReconciler{ diff --git a/metalnetlet/controllers/network_controller.go b/metalnetlet/controllers/network_controller.go index 289f40f4..23755fc5 100644 --- a/metalnetlet/controllers/network_controller.go +++ b/metalnetlet/controllers/network_controller.go @@ -26,11 +26,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) -type NetworkPeeringControllingType string +type NetworkPeeringProviderType string const ( - NetworkPeeringControllingTypeNative = "Native" - NetworkPeeringControllingTypeNone = "None" + BuiltInNetworkPeeringProvider = "BuiltIn" + ExternalNetworkPeeringProvider = "External" ) type NetworkReconciler struct { @@ -41,7 +41,7 @@ type NetworkReconciler struct { MetalnetNamespace string - NetworkPeeringControllingType NetworkPeeringControllingType + NetworkPeeringControllingType NetworkPeeringProviderType } //+kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -123,7 +123,7 @@ func (r *NetworkReconciler) delete(ctx context.Context, log logr.Logger, network } func (r *NetworkReconciler) updateApinetNetworkStatus(ctx context.Context, log logr.Logger, network *apinetv1alpha1.Network, metalnetNetwork *metalnetv1alpha1.Network) error { - if r.NetworkPeeringControllingType != NetworkPeeringControllingTypeNone { + if r.NetworkPeeringControllingType != ExternalNetworkPeeringProvider { apinetStatusPeerings := metalnetNetworkPeeringsStatusToNetworkPeeringsStatus(metalnetNetwork.Status.Peerings) if !equality.Semantic.DeepEqual(network.Status.Peerings[r.PartitionName], apinetStatusPeerings) { log.V(1).Info("Patching apinet network status", "status", apinetStatusPeerings) @@ -189,7 +189,7 @@ func (r *NetworkReconciler) reconcile(ctx context.Context, log logr.Logger, netw } var peeredIDs []int32 var peeredPrefixes []metalnetv1alpha1.PeeredPrefix - if r.NetworkPeeringControllingType != NetworkPeeringControllingTypeNone { + if r.NetworkPeeringControllingType != ExternalNetworkPeeringProvider { for _, peering := range network.Spec.Peerings { id, err := networkid.ParseVNI(peering.ID) if err != nil { diff --git a/metalnetlet/controllers/network_controller_test.go b/metalnetlet/controllers/network_controller_test.go index db61ebf7..28ef9065 100644 --- a/metalnetlet/controllers/network_controller_test.go +++ b/metalnetlet/controllers/network_controller_test.go @@ -146,7 +146,7 @@ var _ = Describe("NetworkController", func() { metalnetNs := SetupNamespace(&k8sClient) SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs) - It("should create metalnet networks for apinet networks without peerings information if NetworkPeeringControllingType is set to None", func(ctx SpecContext) { + It("should create metalnet networks for apinet networks without peerings information if NetworkPeeringProviderType is set to External", func(ctx SpecContext) { By("creating a apinet network-1") network1 := &apinetv1alpha1.Network{ ObjectMeta: metav1.ObjectMeta{ @@ -266,7 +266,6 @@ var _ = Describe("NetworkController", func() { By("asserting the corresponding apinet network is gone as well") Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(metalnetNetwork1), metalnetNetwork1)).To(Satisfy(apierrors.IsNotFound)) Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(metalnetNetwork2), metalnetNetwork2)).To(Satisfy(apierrors.IsNotFound)) - }) }) }) From a1e1b5071052d3eb3f61562618c86629c0f92af7 Mon Sep 17 00:00:00 2001 From: ushabelgur Date: Thu, 27 Feb 2025 11:39:33 +0530 Subject: [PATCH 3/4] incorporating review comments --- cmd/metalnetlet/main.go | 17 ++++++++--------- .../controllers/controllers_suite_test.go | 10 +++++----- metalnetlet/controllers/network_controller.go | 13 +++---------- 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/cmd/metalnetlet/main.go b/cmd/metalnetlet/main.go index 0e8f93d3..3117c35f 100644 --- a/cmd/metalnetlet/main.go +++ b/cmd/metalnetlet/main.go @@ -57,7 +57,7 @@ func main() { var configOptions config.GetConfigOptions var metalnetKubeconfig string var metalnetNamespace string - var networkPeeringProvider string + var disableNetworkPeering bool flag.StringVar(&name, "name", "", "The name of the partition the metalnetlet represents (required).") flag.StringToStringVar(&nodeLabels, "node-label", nodeLabels, "Additional labels to add to the nodes.") @@ -70,10 +70,9 @@ func main() { configOptions.BindFlags(flag.CommandLine) flag.StringVar(&metalnetKubeconfig, "metalnet-kubeconfig", "", "Metalnet kubeconfig to use.") flag.StringVar(&metalnetNamespace, "metalnet-namespace", corev1.NamespaceDefault, "Metalnet namespace to use.") - flag.StringVar(&networkPeeringProvider, "network-peering-provider", "BuiltIn", + flag.BoolVar(&disableNetworkPeering, "disable-network-peering", false, "Whether to use metalnet for populating the peered prefixes or not. "+ - "If unset or 'BuiltIn' is passed metalnetlet will populate the peered prefixes for the lowlevel Network resources."+ - "If 'External' is passed, metalnetlet will not populate any peered prefixes for the metalnet-related Network resources.") + "If disabled, metalnetlet will not populate any peered prefixes for the metalnet-related Network resources.") opts := zap.Options{ Development: true, @@ -158,11 +157,11 @@ func main() { } if err := (&controllers.NetworkReconciler{ - Client: mgr.GetClient(), - MetalnetClient: metalnetCluster.GetClient(), - PartitionName: name, - MetalnetNamespace: metalnetNamespace, - NetworkPeeringControllingType: controllers.NetworkPeeringProviderType(networkPeeringProvider), + Client: mgr.GetClient(), + MetalnetClient: metalnetCluster.GetClient(), + PartitionName: name, + MetalnetNamespace: metalnetNamespace, + DisableNetworkPeering: disableNetworkPeering, }).SetupWithManager(mgr, metalnetCluster.GetCache()); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Network") os.Exit(1) diff --git a/metalnetlet/controllers/controllers_suite_test.go b/metalnetlet/controllers/controllers_suite_test.go index fa5ad05a..d86c7aed 100644 --- a/metalnetlet/controllers/controllers_suite_test.go +++ b/metalnetlet/controllers/controllers_suite_test.go @@ -198,11 +198,11 @@ func SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs *corev1.Namespace // register reconciler here Expect((&NetworkReconciler{ - Client: k8sManager.GetClient(), - MetalnetClient: k8sManager.GetClient(), - PartitionName: partitionName, - MetalnetNamespace: metalnetNs.Name, - NetworkPeeringControllingType: ExternalNetworkPeeringProvider, + Client: k8sManager.GetClient(), + MetalnetClient: k8sManager.GetClient(), + PartitionName: partitionName, + MetalnetNamespace: metalnetNs.Name, + DisableNetworkPeering: true, }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) Expect((&MetalnetNodeReconciler{ diff --git a/metalnetlet/controllers/network_controller.go b/metalnetlet/controllers/network_controller.go index 23755fc5..87b4411c 100644 --- a/metalnetlet/controllers/network_controller.go +++ b/metalnetlet/controllers/network_controller.go @@ -26,13 +26,6 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" ) -type NetworkPeeringProviderType string - -const ( - BuiltInNetworkPeeringProvider = "BuiltIn" - ExternalNetworkPeeringProvider = "External" -) - type NetworkReconciler struct { client.Client MetalnetClient client.Client @@ -41,7 +34,7 @@ type NetworkReconciler struct { MetalnetNamespace string - NetworkPeeringControllingType NetworkPeeringProviderType + DisableNetworkPeering bool } //+kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -123,7 +116,7 @@ func (r *NetworkReconciler) delete(ctx context.Context, log logr.Logger, network } func (r *NetworkReconciler) updateApinetNetworkStatus(ctx context.Context, log logr.Logger, network *apinetv1alpha1.Network, metalnetNetwork *metalnetv1alpha1.Network) error { - if r.NetworkPeeringControllingType != ExternalNetworkPeeringProvider { + if !r.DisableNetworkPeering { apinetStatusPeerings := metalnetNetworkPeeringsStatusToNetworkPeeringsStatus(metalnetNetwork.Status.Peerings) if !equality.Semantic.DeepEqual(network.Status.Peerings[r.PartitionName], apinetStatusPeerings) { log.V(1).Info("Patching apinet network status", "status", apinetStatusPeerings) @@ -189,7 +182,7 @@ func (r *NetworkReconciler) reconcile(ctx context.Context, log logr.Logger, netw } var peeredIDs []int32 var peeredPrefixes []metalnetv1alpha1.PeeredPrefix - if r.NetworkPeeringControllingType != ExternalNetworkPeeringProvider { + if !r.DisableNetworkPeering { for _, peering := range network.Spec.Peerings { id, err := networkid.ParseVNI(peering.ID) if err != nil { From cd970ee2e35098fbee32727596b03a6cea0f5613 Mon Sep 17 00:00:00 2001 From: ushabelgur Date: Tue, 4 Mar 2025 10:14:10 +0530 Subject: [PATCH 4/4] incorporating review comments --- cmd/metalnetlet/main.go | 13 ++++++------- metalnetlet/controllers/controllers_suite_test.go | 12 ++++++------ metalnetlet/controllers/network_controller.go | 6 +++--- metalnetlet/controllers/network_controller_test.go | 4 ++-- 4 files changed, 17 insertions(+), 18 deletions(-) diff --git a/cmd/metalnetlet/main.go b/cmd/metalnetlet/main.go index 3117c35f..543aec99 100644 --- a/cmd/metalnetlet/main.go +++ b/cmd/metalnetlet/main.go @@ -71,8 +71,7 @@ func main() { flag.StringVar(&metalnetKubeconfig, "metalnet-kubeconfig", "", "Metalnet kubeconfig to use.") flag.StringVar(&metalnetNamespace, "metalnet-namespace", corev1.NamespaceDefault, "Metalnet namespace to use.") flag.BoolVar(&disableNetworkPeering, "disable-network-peering", false, - "Whether to use metalnet for populating the peered prefixes or not. "+ - "If disabled, metalnetlet will not populate any peered prefixes for the metalnet-related Network resources.") + "Disable the metalnet based network peering. If set to true the network peering is handled externally.") opts := zap.Options{ Development: true, @@ -157,11 +156,11 @@ func main() { } if err := (&controllers.NetworkReconciler{ - Client: mgr.GetClient(), - MetalnetClient: metalnetCluster.GetClient(), - PartitionName: name, - MetalnetNamespace: metalnetNamespace, - DisableNetworkPeering: disableNetworkPeering, + Client: mgr.GetClient(), + MetalnetClient: metalnetCluster.GetClient(), + PartitionName: name, + MetalnetNamespace: metalnetNamespace, + NetworkPeeringDisabled: disableNetworkPeering, }).SetupWithManager(mgr, metalnetCluster.GetCache()); err != nil { setupLog.Error(err, "unable to create controller", "controller", "Network") os.Exit(1) diff --git a/metalnetlet/controllers/controllers_suite_test.go b/metalnetlet/controllers/controllers_suite_test.go index d86c7aed..085acb76 100644 --- a/metalnetlet/controllers/controllers_suite_test.go +++ b/metalnetlet/controllers/controllers_suite_test.go @@ -186,7 +186,7 @@ func SetupTest(metalnetNs *corev1.Namespace) { }) } -func SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs *corev1.Namespace) { +func SetupTestWithNetworkPeeringDisabled(metalnetNs *corev1.Namespace) { BeforeEach(func(ctx SpecContext) { k8sManager, err := ctrl.NewManager(cfg, ctrl.Options{ Scheme: scheme.Scheme, @@ -198,11 +198,11 @@ func SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs *corev1.Namespace // register reconciler here Expect((&NetworkReconciler{ - Client: k8sManager.GetClient(), - MetalnetClient: k8sManager.GetClient(), - PartitionName: partitionName, - MetalnetNamespace: metalnetNs.Name, - DisableNetworkPeering: true, + Client: k8sManager.GetClient(), + MetalnetClient: k8sManager.GetClient(), + PartitionName: partitionName, + MetalnetNamespace: metalnetNs.Name, + NetworkPeeringDisabled: true, }).SetupWithManager(k8sManager, k8sManager.GetCache())).To(Succeed()) Expect((&MetalnetNodeReconciler{ diff --git a/metalnetlet/controllers/network_controller.go b/metalnetlet/controllers/network_controller.go index 87b4411c..145431d1 100644 --- a/metalnetlet/controllers/network_controller.go +++ b/metalnetlet/controllers/network_controller.go @@ -34,7 +34,7 @@ type NetworkReconciler struct { MetalnetNamespace string - DisableNetworkPeering bool + NetworkPeeringDisabled bool } //+kubebuilder:rbac:groups="",resources=events,verbs=create;patch @@ -116,7 +116,7 @@ func (r *NetworkReconciler) delete(ctx context.Context, log logr.Logger, network } func (r *NetworkReconciler) updateApinetNetworkStatus(ctx context.Context, log logr.Logger, network *apinetv1alpha1.Network, metalnetNetwork *metalnetv1alpha1.Network) error { - if !r.DisableNetworkPeering { + if !r.NetworkPeeringDisabled { apinetStatusPeerings := metalnetNetworkPeeringsStatusToNetworkPeeringsStatus(metalnetNetwork.Status.Peerings) if !equality.Semantic.DeepEqual(network.Status.Peerings[r.PartitionName], apinetStatusPeerings) { log.V(1).Info("Patching apinet network status", "status", apinetStatusPeerings) @@ -182,7 +182,7 @@ func (r *NetworkReconciler) reconcile(ctx context.Context, log logr.Logger, netw } var peeredIDs []int32 var peeredPrefixes []metalnetv1alpha1.PeeredPrefix - if !r.DisableNetworkPeering { + if !r.NetworkPeeringDisabled { for _, peering := range network.Spec.Peerings { id, err := networkid.ParseVNI(peering.ID) if err != nil { diff --git a/metalnetlet/controllers/network_controller_test.go b/metalnetlet/controllers/network_controller_test.go index 28ef9065..94d9188f 100644 --- a/metalnetlet/controllers/network_controller_test.go +++ b/metalnetlet/controllers/network_controller_test.go @@ -144,9 +144,9 @@ var _ = Describe("NetworkController", func() { var _ = Describe("NetworkPeeringController", func() { ns := SetupNamespace(&k8sClient) metalnetNs := SetupNamespace(&k8sClient) - SetupTestWithNetworkPeeringControllingTypeNone(metalnetNs) + SetupTestWithNetworkPeeringDisabled(metalnetNs) - It("should create metalnet networks for apinet networks without peerings information if NetworkPeeringProviderType is set to External", func(ctx SpecContext) { + It("should create metalnet networks for apinet networks without peerings information if NetworkPeeringDisabled is set to true", func(ctx SpecContext) { By("creating a apinet network-1") network1 := &apinetv1alpha1.Network{ ObjectMeta: metav1.ObjectMeta{