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 4, 2024
1 parent 6084f92 commit 0884ee4
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 28 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 21 additions & 5 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.GenerateName(vmi.Name, logicalNetworkName)
ipamClaim := &ipamclaimsapi.IPAMClaim{
ObjectMeta: controllerruntime.ObjectMeta{
Name: claimKey,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

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 0884ee4

Please sign in to comment.