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 3, 2024
1 parent a7ca839 commit 5d0f585
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 23 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
27 changes: 25 additions & 2 deletions pkg/vminetworkscontroller/vmi_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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{
Expand Down
73 changes: 53 additions & 20 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,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{
Expand Down Expand Up @@ -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"),
Expand All @@ -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"),
Expand Down Expand Up @@ -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"),
Expand All @@ -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{
{
Expand Down Expand Up @@ -499,19 +524,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 5d0f585

Please sign in to comment.