Merge pull request #7 from maiqueb/assorted-alpha-release-improvements

Assorted alpha release improvements
kubevirt · May 10, 2024 · f648b76 · f648b76
2 parents 2e41698 + e95a6df
commit f648b76
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 47 deletions.
diff --git a/Makefile b/Makefile
@@ -46,7 +46,7 @@ help: ## Display this help.
 
 .PHONY: manifests
 manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects.
-	$(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases
+	$(CONTROLLER_GEN) rbac:roleName=manager-role webhook paths="./..."
 
 .PHONY: generate
 generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.

diff --git a/pkg/ipamclaimswebhook/podmutator.go b/pkg/ipamclaimswebhook/podmutator.go
@@ -23,6 +23,7 @@ import (
 	"fmt"
 	"net/http"
 	"reflect"
+	"strings"
 
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -63,6 +64,8 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request)
 	if err := a.decoder.Decode(request, pod); err != nil {
 		return admission.Errored(http.StatusBadRequest, err)
 	}
+
+	log.Info("webhook handling event")
 	networkSelectionElements, err := netutils.ParsePodNetworkAnnotation(pod)
 	if err != nil {
 		var goodTypeOfError *v1.NoK8sNetworkError
@@ -71,11 +74,20 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request)
 		}
 		return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to parse pod network selection elements"))
 	}
+
 	var (
 		hasChangedNetworkSelectionElements bool
 		podNetworkSelectionElements        = make([]v1.NetworkSelectionElement, 0, len(networkSelectionElements))
 	)
 	for _, networkSelectionElement := range networkSelectionElements {
+		nadName := types.NamespacedName{
+			Namespace: networkSelectionElement.Namespace,
+			Name:      networkSelectionElement.Name,
+		}.String()
+		log.Info(
+			"iterating network selection elements",
+			"NAD", nadName,
+		)
 		nadKey := types.NamespacedName{
 			Namespace: networkSelectionElement.Namespace,
 			Name:      networkSelectionElement.Name,
@@ -95,15 +107,22 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request)
 		}
 
 		if pluginConfig.AllowPersistentIPs {
+			log.Info(
+				"will request persistent IPs",
+				"NAD", nadName,
+				"network", pluginConfig.Name,
+			)
 			vmName, hasVMAnnotation := pod.Annotations["kubevirt.io/domain"]
 			if !hasVMAnnotation {
+				log.Info(
+					"does not have the kubevirt VM annotation",
+					"NAD", nadName,
+					"network", pluginConfig.Name,
+				)
 				return admission.Allowed("not a VM")
 			}
-			vmKey := types.NamespacedName{
-				Namespace: pod.Namespace,
-				Name:      vmName,
-			}
 
+			vmKey := types.NamespacedName{Namespace: pod.Namespace, Name: vmName}
 			vmi := &virtv1.VirtualMachineInstance{}
 			if err := a.Client.Get(context.Background(), vmKey, vmi); err != nil {
 				return admission.Errored(http.StatusInternalServerError, err)
@@ -112,12 +131,22 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request)
 			vmiNets := vmiSecondaryNetworks(vmi)
 			networkName, foundNetworkName := vmiNets[nadKey.String()]
 			if !foundNetworkName {
-				log.V(5).Info("network name not found", "network name", networkName)
+				log.Info(
+					"network name not found",
+					"NAD", nadName,
+					"network", networkName,
+				)
 				podNetworkSelectionElements = append(podNetworkSelectionElements, *networkSelectionElement)
 				continue
 			}
 
 			networkSelectionElement.IPAMClaimReference = fmt.Sprintf("%s.%s", vmName, networkName)
+			log.Info(
+				"requesting claim",
+				"NAD", nadName,
+				"network", pluginConfig.Name,
+				"claim", networkSelectionElement.IPAMClaimReference,
+			)
 			podNetworkSelectionElements = append(podNetworkSelectionElements, *networkSelectionElement)
 			hasChangedNetworkSelectionElements = true
 			continue
@@ -156,7 +185,12 @@ func vmiSecondaryNetworks(vmi *virtv1.VirtualMachineInstance) map[string]string
 		if network.Multus.Default {
 			continue
 		}
-		indexedSecondaryNetworks[network.Multus.NetworkName] = network.Name
+
+		nadName := network.Multus.NetworkName // NAD name must be formatted in <ns>/<name> format
+		if !strings.Contains(network.Multus.NetworkName, "/") {
+			nadName = fmt.Sprintf("%s/%s", vmi.Namespace, network.Multus.NetworkName)
+		}
+		indexedSecondaryNetworks[nadName] = network.Name
 	}
 
 	return indexedSecondaryNetworks

diff --git a/pkg/vmnetworkscontroller/controller_test.go b/pkg/vmnetworkscontroller/controller_test.go
@@ -235,46 +235,6 @@ var _ = Describe("vm IPAM controller", Serial, func() {
 			},
 		}),
 	)
-
-	// TODO: delete this test; it's duplicated in the table
-	XIt("", func() {
-		key := apitypes.NamespacedName{Namespace: "ns1", Name: "vm1"}
-
-		fakeClient := fake.NewClientBuilder().
-			WithScheme(scheme.Scheme).
-			WithObjects(
-				dummyVM(nadName),
-				dummyVMI(nadName),
-				dummyNAD(nadName),
-			)
-		ctrlOptions := controllerruntime.Options{
-			Scheme: scheme.Scheme,
-			NewClient: func(_ *rest.Config, _ client.Options) (client.Client, error) {
-				return fakeClient.Build(), nil
-			},
-		}
-
-		mgr, err := controllerruntime.NewManager(controllerruntime.GetConfigOrDie(), ctrlOptions)
-		Expect(err).NotTo(HaveOccurred())
-
-		reconcileMachine := NewVMReconciler(mgr)
-
-		Expect(
-			reconcileMachine.Reconcile(context.Background(), controllerruntime.Request{NamespacedName: key}),
-		).To(Equal(reconcile.Result{}))
-
-		ipamClaim := &ipamclaimsapi.IPAMClaim{}
-		claimKey := apitypes.NamespacedName{
-			Namespace: namespace,
-			Name:      fmt.Sprintf("%s.%s", vmName, "randomnet"),
-		}
-		allRoundClient := mgr.GetClient()
-		Expect(allRoundClient.Get(context.Background(), claimKey, ipamClaim)).To(Succeed())
-		Expect(ipamClaim.Spec).To(
-			Equal(ipamclaimsapi.IPAMClaimSpec{
-				Network: "goodnet",
-			}))
-	})
 })
 
 func dummyVM(nadName string) *virtv1.VirtualMachine {