diff --git a/Makefile b/Makefile index c6b3b6e2..82504190 100644 --- a/Makefile +++ b/Makefile @@ -69,7 +69,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 -ginkgo.focus "${WHAT}" + 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 3681b4f3..9696123e 100644 --- a/pkg/vminetworkscontroller/vmi_controller.go +++ b/pkg/vminetworkscontroller/vmi_controller.go @@ -27,6 +27,7 @@ import ( "github.com/kubevirt/ipam-extensions/pkg/claims" "github.com/kubevirt/ipam-extensions/pkg/config" + "github.com/kubevirt/ipam-extensions/pkg/udn" ) // VirtualMachineInstanceReconciler reconciles a VirtualMachineInstance object @@ -84,7 +85,7 @@ func (r *VirtualMachineInstanceReconciler) Reconcile( ownerInfo := ownerReferenceFor(vmi, vm) for logicalNetworkName, netConfigName := range vmiNetworks { - claimKey := fmt.Sprintf("%s.%s", vmi.Name, logicalNetworkName) + claimKey := claims.GenerateName(vmi.Name, logicalNetworkName) ipamClaim := &ipamclaimsapi.IPAMClaim{ ObjectMeta: controllerruntime.ObjectMeta{ Name: claimKey, @@ -158,10 +159,6 @@ func (r *VirtualMachineInstanceReconciler) vmiNetworksClaimingIPAM( ) (map[string]string, error) { vmiNets := make(map[string]string) for _, net := range vmi.Spec.Networks { - if net.Pod != nil { - continue - } - if net.Multus != nil && !net.Multus.Default { nadName := net.Multus.NetworkName namespace := vmi.Namespace @@ -193,8 +190,27 @@ func (r *VirtualMachineInstanceReconciler) vmiNetworksClaimingIPAM( if nadConfig.AllowPersistentIPs { vmiNets[net.Name] = nadConfig.Name } + } else if net.Pod != nil { + primaryNetworkNAD, err := udn.FindPrimaryNetwork(ctx, r.Client, vmi.Namespace) + if err != nil { + return nil, err + } + if primaryNetworkNAD == nil { + continue + } + 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[net.Name] = primaryNetworkConfig.Name + } } } + return vmiNets, nil } diff --git a/pkg/vminetworkscontroller/vmi_controller_test.go b/pkg/vminetworkscontroller/vmi_controller_test.go index d9af04c2..35ffc2ea 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,39 @@ var _ = Describe("VMI IPAM controller", Serial, func() { }, Spec: ipamclaimsapi.IPAMClaimSpec{Network: "goodnet"}, }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("%s.%s", vmName, "podnet"), + Namespace: namespace, + Finalizers: []string{claims.KubevirtVMFinalizer}, + Labels: claims.OwnedByVMLabel(vmName), + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: "kubevirt.io/v1", + Kind: "VirtualMachine", + Name: vmName, + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true)}, + }, + }, + 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{ @@ -267,8 +290,10 @@ var _ = Describe("VMI IPAM controller", Serial, func() { }, }), Entry("standalone VMI which is marked for deletion, with active pods, should keep IPAMClaims finalizers", testConfig{ - inputVMI: dummyMarkedForDeletionVMIWithActivePods(nadName), - inputNAD: dummyNAD(nadName), + inputVMI: dummyMarkedForDeletionVMIWithActivePods(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, expectedResponse: reconcile.Result{}, existingIPAMClaim: &ipamclaimsapi.IPAMClaim{ ObjectMeta: metav1.ObjectMeta{ @@ -331,7 +356,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 +371,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 +415,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 +438,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 +532,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 +540,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{