From 5d0f585d1041e545040fc716f8422e05bc3255bc 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 | 73 ++++++++++++++----- 3 files changed, 79 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index f0c95663..c853f919 100644 --- a/Makefile +++ b/Makefile @@ -67,7 +67,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 91843976..6e7eda55 100644 --- a/pkg/vminetworkscontroller/vmi_controller.go +++ b/pkg/vminetworkscontroller/vmi_controller.go @@ -28,6 +28,8 @@ import ( "github.com/kubevirt/ipam-extensions/pkg/claims" "github.com/kubevirt/ipam-extensions/pkg/config" + "github.com/kubevirt/ipam-extensions/pkg/ipamclaim" + "github.com/kubevirt/ipam-extensions/pkg/udn" ) // VirtualMachineInstanceReconciler reconciles a VirtualMachineInstance object @@ -85,7 +87,7 @@ func (r *VirtualMachineInstanceReconciler) Reconcile( ownerInfo := ownerReferenceFor(vmi, vm) 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, @@ -174,6 +176,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 } @@ -244,7 +266,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 d9af04c2..4523f08c 100644 --- a/pkg/vminetworkscontroller/vmi_controller_test.go +++ b/pkg/vminetworkscontroller/vmi_controller_test.go @@ -45,7 +45,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 @@ -94,8 +94,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 { @@ -140,10 +140,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(dummyVMISpec(nadName)), - inputVMI: dummyVMI(dummyVMISpec(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(dummyVMISpec(nadName)), + inputVMI: dummyVMI(dummyVMISpec(nadName)), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + dummyPrimaryNetworkNAD(nadName), + }, expectedResponse: reconcile.Result{}, expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{ { @@ -162,19 +165,33 @@ 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 but as multus default network", testConfig{ - inputVM: dummyVM(dummyVMIWithMultusDefaultNetworkSpec(nadName)), - inputVMI: dummyVMI(dummyVMIWithMultusDefaultNetworkSpec(nadName)), - inputNAD: dummyNAD(nadName), + inputVM: dummyVM(dummyVMIWithMultusDefaultNetworkSpec(nadName)), + inputVMI: dummyVMI(dummyVMIWithMultusDefaultNetworkSpec(nadName)), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, expectedResponse: reconcile.Result{}, expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{}, }), Entry("when the VM has an associated VMI pointing to an existing NAD with an improper config", testConfig{ - inputVM: dummyVM(dummyVMISpec(nadName)), - inputVMI: dummyVMI(dummyVMISpec(nadName)), - inputNAD: dummyNADWrongFormat(nadName), + inputVM: dummyVM(dummyVMISpec(nadName)), + inputVMI: dummyVMI(dummyVMISpec(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{ @@ -331,7 +348,9 @@ var _ = Describe("VMI IPAM controller", Serial, func() { Entry("everything is OK but there's already an IPAMClaim with this name", testConfig{ inputVM: dummyVM(dummyVMISpec(nadName)), inputVMI: dummyVMI(dummyVMISpec(nadName)), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s.%s", vmName, "randomnet"), @@ -344,7 +363,9 @@ var _ = Describe("VMI IPAM controller", Serial, func() { Entry("found an existing IPAMClaim for the same VM", testConfig{ inputVM: decorateVMWithUID(dummyUID, dummyVM(dummyVMISpec(nadName))), inputVMI: dummyVMI(dummyVMISpec(nadName)), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s.%s", vmName, "randomnet"), @@ -386,7 +407,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(dummyVMISpec(nadName))), inputVMI: dummyVMI(dummyVMISpec(nadName)), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s.%s", vmName, "randomnet"), @@ -407,8 +430,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(dummyVMISpec(nadName)), - inputNAD: dummyNAD(nadName), + inputVMI: dummyVMI(dummyVMISpec(nadName)), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, expectedResponse: reconcile.Result{}, expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{ { @@ -499,7 +524,7 @@ func dummyVMIWithMultusDefaultNetworkSpec(nadName string) virtv1.VirtualMachineI } } -func dummyNAD(nadName string) *nadv1.NetworkAttachmentDefinition { +func dummyNADWithConfig(nadName string, config string) *nadv1.NetworkAttachmentDefinition { namespaceAndName := strings.Split(nadName, "/") return &nadv1.NetworkAttachmentDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -507,11 +532,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{