From 57ed76b46b3c8baa6fc7ac844716c60a25c65895 Mon Sep 17 00:00:00 2001 From: Enrique Llorente Date: Thu, 4 Jul 2024 17:41:42 +0200 Subject: [PATCH] vm, controller: Create IPAMClaim for primary Signed-off-by: Enrique Llorente --- Makefile | 2 +- pkg/vminetworkscontroller/vmi_controller.go | 27 +++++++- .../vmi_controller_test.go | 66 ++++++++++++++----- 3 files changed, 75 insertions(+), 20 deletions(-) diff --git a/Makefile b/Makefile index ca15afa2..e143da34 100644 --- a/Makefile +++ b/Makefile @@ -65,7 +65,7 @@ vet: ## Run go vet against code. .PHONY: test test: manifests generate fmt vet envtest ## Run tests. - KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out -v -ginkgo.v + KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) --bin-dir $(LOCALBIN) -p path)" go test $$(go list ./... | grep -v /e2e) -coverprofile cover.out -v -ginkgo.v -ginkgo.focus="${WHAT}" # Utilize Kind or modify the e2e tests to load the image locally, enabling compatibility with other vendors. .PHONY: test-e2e # Run the e2e tests against a Kind k8s instance that is spun up. diff --git a/pkg/vminetworkscontroller/vmi_controller.go b/pkg/vminetworkscontroller/vmi_controller.go index c596933c..8b806c09 100644 --- a/pkg/vminetworkscontroller/vmi_controller.go +++ b/pkg/vminetworkscontroller/vmi_controller.go @@ -25,6 +25,8 @@ import ( virtv1 "kubevirt.io/api/core/v1" "github.com/kubevirt/ipam-extensions/pkg/config" + "github.com/kubevirt/ipam-extensions/pkg/ipamclaim" + "github.com/kubevirt/ipam-extensions/pkg/udn" ) const kubevirtVMFinalizer = "kubevirt.io/persistent-ipam" @@ -88,7 +90,7 @@ func (r *VirtualMachineInstanceReconciler) Reconcile( } for logicalNetworkName, netConfigName := range vmiNetworks { - claimKey := fmt.Sprintf("%s.%s", vmi.Name, logicalNetworkName) + claimKey := ipamclaim.GenerateName(vmi.Name, logicalNetworkName) ipamClaim := &ipamclaimsapi.IPAMClaim{ ObjectMeta: controllerruntime.ObjectMeta{ Name: claimKey, @@ -177,6 +179,26 @@ func (r *VirtualMachineInstanceReconciler) vmiNetworksClaimingIPAM( } } } + + //TODO: Do we really need to create the IPAMClaim for primary network + // if the VM is not exposing the default pod network ? + primaryNetworkNAD, err := udn.FindPrimaryNetwork(ctx, r.Client, vmi.Namespace) + if err != nil { + return nil, err + } + r.Log.Info(fmt.Sprintf("DELETEME, primaryNetworkNAD: %+v", primaryNetworkNAD)) + if primaryNetworkNAD != nil { + primaryNetworkConfig, err := config.NewConfig(primaryNetworkNAD.Spec.Config) + if err != nil { + r.Log.Error(err, "failed extracting the relevant NAD configuration", + "NAD name", client.ObjectKeyFromObject(primaryNetworkNAD)) + return nil, fmt.Errorf("failed to extract the relevant NAD information: %w", err) + } + + if primaryNetworkConfig.AllowPersistentIPs { + vmiNets[primaryNetworkConfig.Name] = primaryNetworkConfig.Name + } + } return vmiNets, nil } @@ -201,7 +223,8 @@ func (r *VirtualMachineInstanceReconciler) Cleanup(vmiKey apitypes.NamespacedNam return nil } -func (r *VirtualMachineReconciler) CreateIPAMClaim(ctx context.Context, ipamClaim *ipamclaimsapi.IPAMClaim, ownerInfo *metav1.OwnerReference) error { +func (r *VirtualMachineInstanceReconciler) CreateIPAMClaim(ctx context.Context, + ipamClaim *ipamclaimsapi.IPAMClaim, ownerInfo *metav1.OwnerReference) error { if err := r.Client.Create(ctx, ipamClaim, &client.CreateOptions{}); err != nil { if apierrors.IsAlreadyExists(err) { claimKey := apitypes.NamespacedName{ diff --git a/pkg/vminetworkscontroller/vmi_controller_test.go b/pkg/vminetworkscontroller/vmi_controller_test.go index 690ab882..94158973 100644 --- a/pkg/vminetworkscontroller/vmi_controller_test.go +++ b/pkg/vminetworkscontroller/vmi_controller_test.go @@ -41,7 +41,7 @@ var ( type testConfig struct { inputVM *virtv1.VirtualMachine inputVMI *virtv1.VirtualMachineInstance - inputNAD *nadv1.NetworkAttachmentDefinition + inputNADs []*nadv1.NetworkAttachmentDefinition existingIPAMClaim *ipamclaimsapi.IPAMClaim expectedError error expectedResponse reconcile.Result @@ -89,8 +89,8 @@ var _ = Describe("vmi IPAM controller", Serial, func() { initialObjects = append(initialObjects, config.inputVMI) } - if config.inputNAD != nil { - initialObjects = append(initialObjects, config.inputNAD) + for _, nad := range config.inputNADs { + initialObjects = append(initialObjects, nad) } if config.existingIPAMClaim != nil { @@ -135,10 +135,13 @@ var _ = Describe("vmi IPAM controller", Serial, func() { Expect(ipamClaimsCleaner(ipamClaimList.Items...)).To(ConsistOf(config.expectedIPAMClaims)) } }, - Entry("when the VM has an associated VMI pointing to an existing NAD", testConfig{ - inputVM: dummyVM(nadName), - inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + Entry("when the VM has an associated VMI pointing to an existing NAD with a primary network at namespace", testConfig{ + inputVM: dummyVM(nadName), + inputVMI: dummyVMI(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + dummyPrimaryNetworkNAD(nadName), + }, expectedResponse: reconcile.Result{}, expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{ { @@ -151,12 +154,25 @@ var _ = Describe("vmi IPAM controller", Serial, func() { }, Spec: ipamclaimsapi.IPAMClaimSpec{Network: "goodnet"}, }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s.%s", vmName, "primarynet"), + Namespace: namespace, + Finalizers: []string{kubevirtVMFinalizer}, + Labels: ownedByVMLabel(vmName), + OwnerReferences: []metav1.OwnerReference{{Name: vmName}}, + }, + Spec: ipamclaimsapi.IPAMClaimSpec{Network: "primarynet"}, + }, }, }), + Entry("when the VM has an associated VMI pointing to an existing NAD with an improper config", testConfig{ - inputVM: dummyVM(nadName), - inputVMI: dummyVMI(nadName), - inputNAD: dummyNADWrongFormat(nadName), + inputVM: dummyVM(nadName), + inputVMI: dummyVMI(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNADWrongFormat(nadName), + }, expectedError: fmt.Errorf("failed to extract the relevant NAD information"), }), Entry("the associated VMI exists but points to a NAD that doesn't exist", testConfig{ @@ -204,7 +220,9 @@ var _ = Describe("vmi IPAM controller", Serial, func() { Entry("everything is OK but there's already an IPAMClaim with this name", testConfig{ inputVM: dummyVM(nadName), inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s.%s", vmName, "randomnet"), @@ -217,7 +235,9 @@ var _ = Describe("vmi IPAM controller", Serial, func() { Entry("found an existing IPAMClaim for the same VM", testConfig{ inputVM: decorateVMWithUID(dummyUID, dummyVM(nadName)), inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s.%s", vmName, "randomnet"), @@ -259,7 +279,9 @@ var _ = Describe("vmi IPAM controller", Serial, func() { Entry("found an existing IPAMClaim for a VM with same name but different UID", testConfig{ inputVM: decorateVMWithUID(dummyUID, dummyVM(nadName)), inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s.%s", vmName, "randomnet"), @@ -280,8 +302,10 @@ var _ = Describe("vmi IPAM controller", Serial, func() { expectedError: fmt.Errorf("failed since it found an existing IPAMClaim for \"vm1.randomnet\""), }), Entry("a lonesome VMI (with no corresponding VM) is a valid migration use-case", testConfig{ - inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + inputVMI: dummyVMI(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, expectedResponse: reconcile.Result{}, expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{ { @@ -342,7 +366,7 @@ func dummyVMISpec(nadName string) virtv1.VirtualMachineInstanceSpec { } } -func dummyNAD(nadName string) *nadv1.NetworkAttachmentDefinition { +func dummyNADWithConfig(nadName string, config string) *nadv1.NetworkAttachmentDefinition { namespaceAndName := strings.Split(nadName, "/") return &nadv1.NetworkAttachmentDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -350,11 +374,19 @@ func dummyNAD(nadName string) *nadv1.NetworkAttachmentDefinition { Name: namespaceAndName[1], }, Spec: nadv1.NetworkAttachmentDefinitionSpec{ - Config: `{"name": "goodnet", "allowPersistentIPs": true}`, + Config: config, }, } } +func dummyNAD(nadName string) *nadv1.NetworkAttachmentDefinition { + return dummyNADWithConfig(nadName, `{"name": "goodnet", "allowPersistentIPs": true}`) +} + +func dummyPrimaryNetworkNAD(nadName string) *nadv1.NetworkAttachmentDefinition { + return dummyNADWithConfig(nadName+"primary", `{"name": "primarynet", "role": "primary", "allowPersistentIPs": true}`) +} + func dummyNADWrongFormat(nadName string) *nadv1.NetworkAttachmentDefinition { namespaceAndName := strings.Split(nadName, "/") return &nadv1.NetworkAttachmentDefinition{