Skip to content

Commit

Permalink
vm, controller: Create IPAMClaim for primary
Browse files Browse the repository at this point in the history 
Signed-off-by: Enrique Llorente <[email protected]>
  • Loading branch information
@qinqon
qinqon committed Sep 5, 2024
1 parent d008090 commit bdabec2
Show file tree
Hide file tree
Showing 2 changed files with 120 additions and 54 deletions.
89 changes: 57 additions & 32 deletions pkg/vminetworkscontroller/vmi_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.ComposeKey(vmi.Name, logicalNetworkName)
ipamClaim := &ipamclaimsapi.IPAMClaim{
ObjectMeta: controllerruntime.ObjectMeta{
Name: claimKey,
Expand Down Expand Up @@ -158,44 +159,68 @@ 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
namespaceAndName := strings.Split(nadName, "/")
if len(namespaceAndName) == 2 {
namespace = namespaceAndName[0]
nadName = namespaceAndName[1]
if err := r.ensureVMINetworksWithSecondaryUDN(ctx, vmi.Namespace, net, vmiNets); err != nil {
return nil, err
}

contextWithTimeout, cancel := context.WithTimeout(ctx, time.Second)
defer cancel()
nad := &nadv1.NetworkAttachmentDefinition{}
if err := r.Client.Get(
contextWithTimeout,
apitypes.NamespacedName{Namespace: namespace, Name: nadName},
nad,
); err != nil {
if apierrors.IsNotFound(err) {
return nil, err
}
} else if net.Pod != nil {
if err := r.ensureVMINetworksWithPrimaryUDN(ctx, vmi.Namespace, net, vmiNets); err != nil {
return nil, err
}
}
}

nadConfig, err := config.NewConfig(nad.Spec.Config)
if err != nil {
r.Log.Error(err, "failed extracting the relevant NAD configuration", "NAD name", nadName)
return nil, fmt.Errorf("failed to extract the relevant NAD information")
}
return vmiNets, nil
}

if nadConfig.AllowPersistentIPs {
vmiNets[net.Name] = nadConfig.Name
}
func (r *VirtualMachineInstanceReconciler) ensureVMINetworksWithSecondaryUDN(ctx context.Context,
namespace string, network virtv1.Network, vmiNets map[string]string) error {
nadName := network.Multus.NetworkName
namespaceAndName := strings.Split(nadName, "/")
if len(namespaceAndName) == 2 {
namespace = namespaceAndName[0]
nadName = namespaceAndName[1]
}

contextWithTimeout, cancel := context.WithTimeout(ctx, time.Second)
defer cancel()
nad := &nadv1.NetworkAttachmentDefinition{}
if err := r.Client.Get(
contextWithTimeout,
apitypes.NamespacedName{Namespace: namespace, Name: nadName},
nad,
); err != nil {
if apierrors.IsNotFound(err) {
return err
}
}
return vmiNets, nil
return r.ensureVMINetworkWithUDN(network, nad, vmiNets)
}

func (r *VirtualMachineInstanceReconciler) ensureVMINetworksWithPrimaryUDN(ctx context.Context,
namespace string, network virtv1.Network, vmiNets map[string]string) error {
primaryNetworkNAD, err := udn.FindPrimaryNetwork(ctx, r.Client, namespace)
if err != nil {
return err
}
if primaryNetworkNAD == nil {
return nil
}
return r.ensureVMINetworkWithUDN(network, primaryNetworkNAD, vmiNets)
}

func (r *VirtualMachineInstanceReconciler) ensureVMINetworkWithUDN(network virtv1.Network,
nad *nadv1.NetworkAttachmentDefinition, vmiNets map[string]string) error {
nadConfig, err := config.NewConfig(nad.Spec.Config)
if err != nil {
r.Log.Error(err, "failed extracting the relevant NAD configuration", "NAD name", nad.Name)
return fmt.Errorf("failed to extract the relevant NAD information")
}

if nadConfig.AllowPersistentIPs {
vmiNets[network.Name] = nadConfig.Name
}
return nil
}

func shouldCleanFinalizers(vmi *virtv1.VirtualMachineInstance, vm *virtv1.VirtualMachine) bool {
Expand Down
85 changes: 63 additions & 22 deletions pkg/vminetworkscontroller/vmi_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
{
Expand All @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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"),
Expand All @@ -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"),
Expand Down Expand Up @@ -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"),
Expand All @@ -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{
{
Expand Down Expand Up @@ -499,19 +532,27 @@ 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{
Namespace: namespaceAndName[0],
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{
Expand Down

0 comments on commit bdabec2

Please sign in to comment.