From dfb6dbf44d809e3b575741693833d60882ba83e5 Mon Sep 17 00:00:00 2001 From: Enrique Llorente Date: Thu, 4 Jul 2024 17:08:09 +0200 Subject: [PATCH] webhook: Annotate with primary network ipamclaim Signed-off-by: Enrique Llorente --- Makefile | 2 +- pkg/ipamclaimswebhook/podmutator.go | 84 +++++++++++--- pkg/ipamclaimswebhook/podmutator_test.go | 142 ++++++++++++++++------- 3 files changed, 172 insertions(+), 56 deletions(-) diff --git a/Makefile b/Makefile index 6a60e676..c6b3b6e2 100644 --- a/Makefile +++ b/Makefile @@ -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 + 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. diff --git a/pkg/ipamclaimswebhook/podmutator.go b/pkg/ipamclaimswebhook/podmutator.go index 7f2a3112..e8732e64 100644 --- a/pkg/ipamclaimswebhook/podmutator.go +++ b/pkg/ipamclaimswebhook/podmutator.go @@ -39,7 +39,9 @@ import ( virtv1 "kubevirt.io/api/core/v1" + "github.com/kubevirt/ipam-extensions/pkg/claims" "github.com/kubevirt/ipam-extensions/pkg/config" + "github.com/kubevirt/ipam-extensions/pkg/udn" ) // +kubebuilder:webhook:path=/mutate-v1-pod,mutating=true,failurePolicy=fail,groups="",resources=pods,verbs=create;update,versions=v1,name=ipam-claims.k8s.cni.cncf.io,admissionReviewVersions=v1,sideEffects=None @@ -65,6 +67,15 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) return admission.Errored(http.StatusBadRequest, err) } + log.Info("webhook handling event") + networkSelectionElements, err := netutils.ParsePodNetworkAnnotation(pod) + if err != nil { + var goodTypeOfError *v1.NoK8sNetworkError + if !errors.As(err, &goodTypeOfError) { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to parse pod network selection elements")) + } + } + vmName, hasVMAnnotation := pod.Annotations["kubevirt.io/domain"] if !hasVMAnnotation { log.Info( @@ -73,20 +84,45 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) return admission.Allowed("not a VM") } - log.Info("webhook handling event") - networkSelectionElements, err := netutils.ParsePodNetworkAnnotation(pod) - if err != nil { - var goodTypeOfError *v1.NoK8sNetworkError - if errors.As(err, &goodTypeOfError) { - return admission.Allowed("no secondary networks requested") - } - return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to parse pod network selection elements")) - } - var ( hasChangedNetworkSelectionElements bool podNetworkSelectionElements = make([]v1.NetworkSelectionElement, 0, len(networkSelectionElements)) + primaryNetworkIpamClaimName = "" ) + primaryNetworkNAD, err := udn.FindPrimaryNetwork(ctx, a.Client, pod.Namespace) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + if primaryNetworkNAD != nil { + pluginConfig, err := config.NewConfig(primaryNetworkNAD.Spec.Config) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + + if pluginConfig.AllowPersistentIPs { + log.Info( + "will request primary network persistent IPs", + "NAD", client.ObjectKeyFromObject(primaryNetworkNAD), + "network", pluginConfig.Name, + ) + + 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) + } + + networkName := vmiPodNetworkName(vmi) + if networkName == "" { + log.Info("vmi has no pod network primary UDN ipam claim will not be created") + } else { + primaryNetworkIpamClaimName = claims.GenerateName(vmi.Name, networkName) + hasChangedNetworkSelectionElements = true + } + } + } + for _, networkSelectionElement := range networkSelectionElements { nadName := types.NamespacedName{ Namespace: networkSelectionElement.Namespace, @@ -139,7 +175,7 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) continue } - networkSelectionElement.IPAMClaimReference = fmt.Sprintf("%s.%s", vmName, networkName) + networkSelectionElement.IPAMClaimReference = claims.GenerateName(vmName, networkName) log.Info( "requesting claim", "NAD", nadName, @@ -153,10 +189,17 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) podNetworkSelectionElements = append(podNetworkSelectionElements, *networkSelectionElement) } - if len(podNetworkSelectionElements) > 0 { - newPod, err := podWithUpdatedSelectionElements(pod, podNetworkSelectionElements) - if err != nil { - return admission.Errored(http.StatusInternalServerError, err) + if len(podNetworkSelectionElements) > 0 || primaryNetworkIpamClaimName != "" { + newPod := pod.DeepCopy() + if len(podNetworkSelectionElements) > 0 { + var err error + newPod, err = podWithUpdatedSelectionElements(newPod, podNetworkSelectionElements) + if err != nil { + return admission.Errored(http.StatusInternalServerError, err) + } + } + if primaryNetworkIpamClaimName != "" { + newPod.Annotations[config.OVNPrimaryNetworkIPAMClaimAnnotation] = primaryNetworkIpamClaimName } if reflect.DeepEqual(newPod, pod) || !hasChangedNetworkSelectionElements { @@ -171,6 +214,7 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) log.Info("new pod annotations", "pod", newPod.Annotations) return admission.PatchResponseFromRaw(request.Object.Raw, marshaledPod) } + return admission.Allowed("carry on") } @@ -195,6 +239,16 @@ func vmiSecondaryNetworks(vmi *virtv1.VirtualMachineInstance) map[string]string return indexedSecondaryNetworks } +// returns the name of the kubevirt VM pod network +func vmiPodNetworkName(vmi *virtv1.VirtualMachineInstance) string { + for _, network := range vmi.Spec.Networks { + if network.Pod != nil { + return network.Name + } + } + return "" +} + func podWithUpdatedSelectionElements(pod *corev1.Pod, networks []v1.NetworkSelectionElement) (*corev1.Pod, error) { newPod := pod.DeepCopy() newNets, err := json.Marshal(networks) diff --git a/pkg/ipamclaimswebhook/podmutator_test.go b/pkg/ipamclaimswebhook/podmutator_test.go index b204fa9c..8996054b 100644 --- a/pkg/ipamclaimswebhook/podmutator_test.go +++ b/pkg/ipamclaimswebhook/podmutator_test.go @@ -36,7 +36,7 @@ import ( type testConfig struct { inputVM *virtv1.VirtualMachine inputVMI *virtv1.VirtualMachineInstance - inputNAD *nadv1.NetworkAttachmentDefinition + inputNADs []*nadv1.NetworkAttachmentDefinition inputPod *corev1.Pod expectedAdmissionResponse admission.Response } @@ -86,8 +86,8 @@ var _ = Describe("KubeVirt IPAM launcher pod mutato machine", 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) } ctrlOptions := controllerruntime.Options{ @@ -111,44 +111,101 @@ var _ = Describe("KubeVirt IPAM launcher pod mutato machine", Serial, func() { Equal(config.expectedAdmissionResponse), ) }, - Entry("pod not requesting secondary attachments is accepted", testConfig{ + Entry("pod not requesting secondary attachments and no primary user defined network is accepted", testConfig{ inputVM: dummyVM(nadName), inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, inputPod: &corev1.Pod{}, expectedAdmissionResponse: admission.Response{ AdmissionResponse: admissionv1.AdmissionResponse{ Allowed: true, Result: &metav1.Status{ - Message: "no secondary networks requested", + Message: "not a VM", Code: http.StatusOK, }, }, }, }), - Entry("vm launcher pod with an attachment to a network with persistent IPs enabled requests an IPAMClaim", testConfig{ - inputVM: dummyVM(nadName), - inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), - inputPod: dummyPodForVM(nadName, vmName), - expectedAdmissionResponse: admission.Response{ - AdmissionResponse: admissionv1.AdmissionResponse{ - Allowed: true, - PatchType: &patchType, + Entry("vm launcher pod with an attachment to a primary and ", + "secondary user defined network with persistent IPs enabled requests an IPAMClaim", testConfig{ + inputVM: dummyVM(nadName), + inputVMI: dummyVMI(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + dummyPrimaryNetworkNAD(nadName), }, - Patches: []jsonpatch.JsonPatchOperation{ - { - Operation: "replace", - Path: "/metadata/annotations/k8s.v1.cni.cncf.io~1networks", - Value: "[{\"name\":\"supadupanet\",\"namespace\":\"ns1\",\"ipam-claim-reference\":\"vm1.randomnet\"}]", + inputPod: dummyPodForVM(nadName, vmName), + expectedAdmissionResponse: admission.Response{ + AdmissionResponse: admissionv1.AdmissionResponse{ + Allowed: true, + PatchType: &patchType, + }, + Patches: []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: "/metadata/annotations/k8s.ovn.org~1ovn-udn-ipamclaim-reference", + Value: "vm1.podnet", + }, + { + Operation: "replace", + Path: "/metadata/annotations/k8s.v1.cni.cncf.io~1networks", + Value: "[{\"name\":\"supadupanet\",\"namespace\":\"ns1\",\"ipam-claim-reference\":\"vm1.randomnet\"}]", + }, }, }, - }, - }), + }), + Entry("vm launcher pod with with primary user defined network defined at", + " namespace with persistent IPs enabled requests an IPAMClaim", testConfig{ + inputVM: dummyVM(nadName), + inputVMI: dummyVMI(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyPrimaryNetworkNAD(nadName), + }, + inputPod: dummyPodForVM("" /*without network selection element*/, vmName), + expectedAdmissionResponse: admission.Response{ + AdmissionResponse: admissionv1.AdmissionResponse{ + Allowed: true, + PatchType: &patchType, + }, + Patches: []jsonpatch.JsonPatchOperation{ + { + Operation: "add", + Path: "/metadata/annotations/k8s.ovn.org~1ovn-udn-ipamclaim-reference", + Value: "vm1.podnet", + }, + }, + }, + }), + Entry("vm launcher pod with an attachment to a secondary user defined network ", + "with persistent IPs enabled requests an IPAMClaim", testConfig{ + inputVM: dummyVM(nadName), + inputVMI: dummyVMI(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, + inputPod: dummyPodForVM(nadName, vmName), + expectedAdmissionResponse: admission.Response{ + AdmissionResponse: admissionv1.AdmissionResponse{ + Allowed: true, + PatchType: &patchType, + }, + Patches: []jsonpatch.JsonPatchOperation{ + { + Operation: "replace", + Path: "/metadata/annotations/k8s.v1.cni.cncf.io~1networks", + Value: "[{\"name\":\"supadupanet\",\"namespace\":\"ns1\",\"ipam-claim-reference\":\"vm1.randomnet\"}]", + }, + }, + }, + }), Entry("vm launcher pod with an attachment to a network *without* persistentIPs is accepted", testConfig{ inputVM: dummyVM(nadName), inputVMI: dummyVMI(nadName), - inputNAD: dummyNADWithoutPersistentIPs(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNADWithoutPersistentIPs(nadName), + }, inputPod: dummyPodForVM(nadName, vmName), expectedAdmissionResponse: admission.Response{ AdmissionResponse: admissionv1.AdmissionResponse{ @@ -161,7 +218,9 @@ var _ = Describe("KubeVirt IPAM launcher pod mutato machine", Serial, func() { }, }), Entry("pod not belonging to a VM with an attachment to a network with persistent IPs enabled is accepted", testConfig{ - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, inputPod: dummyPod(nadName), expectedAdmissionResponse: admission.Response{ AdmissionResponse: admissionv1.AdmissionResponse{ @@ -176,7 +235,9 @@ var _ = Describe("KubeVirt IPAM launcher pod mutato machine", Serial, func() { Entry("pod requesting an attachment via a NAD with an invalid configuration throws a BAD REQUEST", testConfig{ inputVM: dummyVM(nadName), inputVMI: dummyVMI(nadName), - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, inputPod: pod("{not json}", nil), expectedAdmissionResponse: admission.Response{ AdmissionResponse: admissionv1.AdmissionResponse{ @@ -203,7 +264,9 @@ var _ = Describe("KubeVirt IPAM launcher pod mutato machine", Serial, func() { }, }), Entry("launcher pod whose VMI is not found throws a server error", testConfig{ - inputNAD: dummyNAD(nadName), + inputNADs: []*nadv1.NetworkAttachmentDefinition{ + dummyNAD(nadName), + }, inputPod: dummyPodForVM(nadName, vmName), expectedAdmissionResponse: admission.Response{ AdmissionResponse: admissionv1.AdmissionResponse{ @@ -261,7 +324,7 @@ 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{ @@ -269,22 +332,20 @@ func dummyNAD(nadName string) *nadv1.NetworkAttachmentDefinition { 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 dummyNADWithoutPersistentIPs(nadName 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"}`, - }, - } + return dummyNADWithConfig(nadName, `{"name": "goodnet"}`) } func podAdmissionRequest(pod *corev1.Pod) admission.Request { @@ -312,8 +373,9 @@ func dummyPod(nadName string) *corev1.Pod { } func pod(nadName string, annotations map[string]string) *corev1.Pod { - baseAnnotations := map[string]string{ - nadv1.NetworkAttachmentAnnot: nadName, + baseAnnotations := map[string]string{} + if nadName != "" { + baseAnnotations[nadv1.NetworkAttachmentAnnot] = nadName } for k, v := range annotations { baseAnnotations[k] = v