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 Jul 10, 2024
1 parent 3d4beeb commit 57ed76b
Show file tree
Hide file tree
Showing 3 changed files with 75 additions and 20 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,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 @@ -25,6 +25,8 @@ import (
virtv1 "kubevirt.io/api/core/v1"

"github.com/kubevirt/ipam-extensions/pkg/config"
"github.com/kubevirt/ipam-extensions/pkg/ipamclaim"
"github.com/kubevirt/ipam-extensions/pkg/udn"
)

const kubevirtVMFinalizer = "kubevirt.io/persistent-ipam"
Expand Down Expand Up @@ -88,7 +90,7 @@ func (r *VirtualMachineInstanceReconciler) Reconcile(
}

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 @@ -177,6 +179,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 All @@ -201,7 +223,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
66 changes: 49 additions & 17 deletions pkg/vminetworkscontroller/vmi_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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 @@ -89,8 +89,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 @@ -135,10 +135,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(nadName),
inputVMI: dummyVMI(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(nadName),
inputVMI: dummyVMI(nadName),
inputNADs: []*nadv1.NetworkAttachmentDefinition{
dummyNAD(nadName),
dummyPrimaryNetworkNAD(nadName),
},
expectedResponse: reconcile.Result{},
expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{
{
Expand All @@ -151,12 +154,25 @@ 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 with an improper config", testConfig{
inputVM: dummyVM(nadName),
inputVMI: dummyVMI(nadName),
inputNAD: dummyNADWrongFormat(nadName),
inputVM: dummyVM(nadName),
inputVMI: dummyVMI(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 @@ -204,7 +220,9 @@ var _ = Describe("vmi IPAM controller", Serial, func() {
Entry("everything is OK but there's already an IPAMClaim with this name", testConfig{
inputVM: dummyVM(nadName),
inputVMI: dummyVMI(nadName),
inputNAD: dummyNAD(nadName),
inputNADs: []*nadv1.NetworkAttachmentDefinition{
dummyNAD(nadName),
},
existingIPAMClaim: &ipamclaimsapi.IPAMClaim{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s.%s", vmName, "randomnet"),
Expand All @@ -217,7 +235,9 @@ var _ = Describe("vmi IPAM controller", Serial, func() {
Entry("found an existing IPAMClaim for the same VM", testConfig{
inputVM: decorateVMWithUID(dummyUID, dummyVM(nadName)),
inputVMI: dummyVMI(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 @@ -259,7 +279,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(nadName)),
inputVMI: dummyVMI(nadName),
inputNAD: dummyNAD(nadName),
inputNADs: []*nadv1.NetworkAttachmentDefinition{
dummyNAD(nadName),
},
existingIPAMClaim: &ipamclaimsapi.IPAMClaim{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("%s.%s", vmName, "randomnet"),
Expand All @@ -280,8 +302,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(nadName),
inputNAD: dummyNAD(nadName),
inputVMI: dummyVMI(nadName),
inputNADs: []*nadv1.NetworkAttachmentDefinition{
dummyNAD(nadName),
},
expectedResponse: reconcile.Result{},
expectedIPAMClaims: []ipamclaimsapi.IPAMClaim{
{
Expand Down Expand Up @@ -342,19 +366,27 @@ func dummyVMISpec(nadName string) virtv1.VirtualMachineInstanceSpec {
}
}

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 57ed76b

Please sign in to comment.