From f78eb995c60ee370c45ec9d002388802c0e77db2 Mon Sep 17 00:00:00 2001 From: Miguel Duarte Barroso Date: Mon, 26 Aug 2024 17:00:43 +0200 Subject: [PATCH] udn: let OVN-K know the claim name when UDN is used Signed-off-by: Miguel Duarte Barroso --- pkg/config/types.go | 1 + pkg/ipamclaimswebhook/podmutator.go | 66 +++++++++-- pkg/vminetworkscontroller/vmi_controller.go | 125 ++++++++++++++------ 3 files changed, 149 insertions(+), 43 deletions(-) diff --git a/pkg/config/types.go b/pkg/config/types.go index 9b12ec5a..afbe7462 100644 --- a/pkg/config/types.go +++ b/pkg/config/types.go @@ -8,6 +8,7 @@ import ( type RelevantConfig struct { Name string `json:"name"` AllowPersistentIPs bool `json:"allowPersistentIPs,omitempty"` + Role string `json:"role,omitempty"` } func NewConfig(nadSpec string) (*RelevantConfig, error) { diff --git a/pkg/ipamclaimswebhook/podmutator.go b/pkg/ipamclaimswebhook/podmutator.go index a9b2d3eb..845b4de2 100644 --- a/pkg/ipamclaimswebhook/podmutator.go +++ b/pkg/ipamclaimswebhook/podmutator.go @@ -65,14 +65,28 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) return admission.Errored(http.StatusBadRequest, err) } - log.Info("webhook handling event") + vmName, hasVMAnnotation := pod.Annotations["kubevirt.io/domain"] + log.Info("webhook handling event - checking primary UDN flow for", "VM", vmName, "namespace", pod.Namespace) + primaryUDNNetwork, err := a.vmiPrimaryUDN(ctx, pod.Namespace) + if err != nil { + // TODO: figure out what to do. Probably fail + return admission.Errored(http.StatusInternalServerError, err) + } + + if primaryUDNNetwork != nil { + log.Info("found primary UDN for", "vmName", vmName, "namespace", pod.Namespace, "primary UDN name", primaryUDNNetwork.Name) + annotatePodWithUDN(pod, vmName, primaryUDNNetwork.Name) + } + + log.Info("webhook handling event - checking secondary networks flow for", "pod", pod.Name, "namespace", pod.Namespace) networkSelectionElements, err := netutils.ParsePodNetworkAnnotation(pod) if err != nil { var goodTypeOfError *v1.NoK8sNetworkError - if errors.As(err, &goodTypeOfError) { + if errors.As(err, &goodTypeOfError) && primaryUDNNetwork == nil { return admission.Allowed("no secondary networks requested") + } else if primaryUDNNetwork == nil { + return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to parse pod network selection elements")) } - return admission.Errored(http.StatusBadRequest, fmt.Errorf("failed to parse pod network selection elements")) } var ( @@ -112,7 +126,6 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) "NAD", nadName, "network", pluginConfig.Name, ) - vmName, hasVMAnnotation := pod.Annotations["kubevirt.io/domain"] if !hasVMAnnotation { log.Info( "does not have the kubevirt VM annotation", @@ -154,13 +167,14 @@ func (a *IPAMClaimsValet) Handle(ctx context.Context, request admission.Request) podNetworkSelectionElements = append(podNetworkSelectionElements, *networkSelectionElement) } - if len(podNetworkSelectionElements) > 0 { + if len(podNetworkSelectionElements) > 0 || primaryUDNNetwork != nil { + log.Info("WILL PATCH THE POD") + newPod, err := podWithUpdatedSelectionElements(pod, podNetworkSelectionElements) if err != nil { return admission.Errored(http.StatusInternalServerError, err) } - - if reflect.DeepEqual(newPod, pod) || !hasChangedNetworkSelectionElements { + if primaryUDNNetwork == nil && (reflect.DeepEqual(newPod, pod) || !hasChangedNetworkSelectionElements) { return admission.Allowed("mutation not needed") } @@ -202,6 +216,42 @@ func podWithUpdatedSelectionElements(pod *corev1.Pod, networks []v1.NetworkSelec if err != nil { return nil, err } - newPod.Annotations[v1.NetworkAttachmentAnnot] = string(newNets) + if string(newNets) != "[]" { + newPod.Annotations[v1.NetworkAttachmentAnnot] = string(newNets) + } return newPod, nil } + +func annotatePodWithUDN(pod *corev1.Pod, vmName string, primaryUDNName string) { + const ovnUDNIPAMClaimName = "k8s.ovn.org/ovn-udn-ipamclaim-reference" + udnAnnotations := map[string]string{ + ovnUDNIPAMClaimName: fmt.Sprintf("%s.%s-primary-udn", vmName, primaryUDNName), + } + pod.SetAnnotations(udnAnnotations) +} + +func (a *IPAMClaimsValet) vmiPrimaryUDN(ctx context.Context, namespace string) (*config.RelevantConfig, error) { + const ( + NetworkRolePrimary = "primary" + NetworkRoleSecondary = "secondary" + ) + + log := logf.FromContext(ctx) + var namespaceNads v1.NetworkAttachmentDefinitionList + if err := a.List(ctx, &namespaceNads, &client.ListOptions{}); err != nil { + return nil, fmt.Errorf("failed to list the NADs on namespace %q: %v", namespace, err) + } + + for _, nad := range namespaceNads.Items { + networkConfig, err := config.NewConfig(nad.Spec.Config) + if err != nil { + log.Error(err, "failed extracting the relevant NAD configuration", "NAD name", nad.Name, "NAD namespace", nad.Namespace) + return nil, fmt.Errorf("failed to extract the relevant NAD information") + } + + if networkConfig.Role == NetworkRolePrimary { + return networkConfig, nil + } + } + return nil, nil +} diff --git a/pkg/vminetworkscontroller/vmi_controller.go b/pkg/vminetworkscontroller/vmi_controller.go index 3681b4f3..2c7870b8 100644 --- a/pkg/vminetworkscontroller/vmi_controller.go +++ b/pkg/vminetworkscontroller/vmi_controller.go @@ -83,44 +83,27 @@ func (r *VirtualMachineInstanceReconciler) Reconcile( } ownerInfo := ownerReferenceFor(vmi, vm) - for logicalNetworkName, netConfigName := range vmiNetworks { - claimKey := fmt.Sprintf("%s.%s", vmi.Name, logicalNetworkName) - ipamClaim := &ipamclaimsapi.IPAMClaim{ - ObjectMeta: controllerruntime.ObjectMeta{ - Name: claimKey, - Namespace: vmi.Namespace, - OwnerReferences: []metav1.OwnerReference{ownerInfo}, - Finalizers: []string{claims.KubevirtVMFinalizer}, - Labels: claims.OwnedByVMLabel(vmi.Name), - }, - Spec: ipamclaimsapi.IPAMClaimSpec{ - Network: netConfigName, - }, - } - if err := r.Client.Create(ctx, ipamClaim, &client.CreateOptions{}); err != nil { - if apierrors.IsAlreadyExists(err) { - claimKey := apitypes.NamespacedName{ - Namespace: vmi.Namespace, - Name: claimKey, - } + // UDN code block + primaryUDN, err := r.vmiPrimaryUDN(ctx, vmi) + if err != nil { + return controllerruntime.Result{}, err + } + if primaryUDN != nil { + claimKey := fmt.Sprintf("%s.%s-primary-udn", vmi.Name, primaryUDN.Name) + udnIPAMClaim := newIPAMClaim(claimKey, vmi, ownerInfo, primaryUDN.Name) + if err := r.ensureIPAMClaim(ctx, udnIPAMClaim, vmi, ownerInfo); err != nil { + return controllerruntime.Result{}, fmt.Errorf("failed ensuring IPAM claim for primary UDN network %q: %w", primaryUDN.Name, err) + } + } + // UDN code block END - existingIPAMClaim := &ipamclaimsapi.IPAMClaim{} - if err := r.Client.Get(ctx, claimKey, existingIPAMClaim); err != nil { - return controllerruntime.Result{}, fmt.Errorf("let us be on the safe side and retry later") - } + for logicalNetworkName, netConfigName := range vmiNetworks { + claimKey := fmt.Sprintf("%s.%s", vmi.Name, logicalNetworkName) + ipamClaim := newIPAMClaim(claimKey, vmi, ownerInfo, netConfigName) - if len(existingIPAMClaim.OwnerReferences) == 1 && existingIPAMClaim.OwnerReferences[0].UID == ownerInfo.UID { - r.Log.Info("found existing IPAMClaim belonging to this VM/VMI, nothing to do", "UID", ownerInfo.UID) - continue - } else { - err := fmt.Errorf("failed since it found an existing IPAMClaim for %q", claimKey.Name) - r.Log.Error(err, "leaked IPAMClaim found", "existing owner", existingIPAMClaim.UID) - return controllerruntime.Result{}, err - } - } - r.Log.Error(err, "failed to create the IPAMClaim") - return controllerruntime.Result{}, err + if err := r.ensureIPAMClaim(ctx, ipamClaim, vmi, ownerInfo); err != nil { + return controllerruntime.Result{}, fmt.Errorf("failed ensuring IPAM claim: %w", err) } } @@ -242,3 +225,75 @@ func getOwningVM(ctx context.Context, c client.Client, name apitypes.NamespacedN return nil, fmt.Errorf("failed getting VM %q: %w", name, err) } } + +func (r *VirtualMachineInstanceReconciler) vmiPrimaryUDN( + ctx context.Context, + vmi *virtv1.VirtualMachineInstance, +) (*config.RelevantConfig, error) { + const ( + NetworkRolePrimary = "primary" + NetworkRoleSecondary = "secondary" + ) + + var namespaceNads nadv1.NetworkAttachmentDefinitionList + if err := r.List(ctx, &namespaceNads, &client.ListOptions{}); err != nil { + return nil, fmt.Errorf("failed to list the NADs on namespace %q: %v", vmi.Namespace, err) + } + + for _, nad := range namespaceNads.Items { + networkConfig, err := config.NewConfig(nad.Spec.Config) + if err != nil { + r.Log.Error(err, "failed extracting the relevant NAD configuration", "NAD name", nad.Name, "NAD namespace", nad.Namespace) + return nil, fmt.Errorf("failed to extract the relevant NAD information") + } + + if networkConfig.Role == NetworkRolePrimary { + return networkConfig, nil + } + } + return nil, nil +} + +func newIPAMClaim(claimKey string, vmi *virtv1.VirtualMachineInstance, ownerInfo metav1.OwnerReference, netConfigName string) *ipamclaimsapi.IPAMClaim { + return &ipamclaimsapi.IPAMClaim{ + ObjectMeta: controllerruntime.ObjectMeta{ + Name: claimKey, + Namespace: vmi.Namespace, + OwnerReferences: []metav1.OwnerReference{ownerInfo}, + Finalizers: []string{claims.KubevirtVMFinalizer}, + Labels: claims.OwnedByVMLabel(vmi.Name), + }, + Spec: ipamclaimsapi.IPAMClaimSpec{ + Network: netConfigName, + }, + } +} + +func (r *VirtualMachineInstanceReconciler) ensureIPAMClaim(ctx context.Context, ipamClaim *ipamclaimsapi.IPAMClaim, vmi *virtv1.VirtualMachineInstance, ownerInfo metav1.OwnerReference) error { + claimKey := ipamClaim.Name + if err := r.Client.Create(ctx, ipamClaim, &client.CreateOptions{}); err != nil { + if apierrors.IsAlreadyExists(err) { + claimKey := apitypes.NamespacedName{ + Namespace: vmi.Namespace, + Name: claimKey, + } + + existingIPAMClaim := &ipamclaimsapi.IPAMClaim{} + if err := r.Client.Get(ctx, claimKey, existingIPAMClaim); err != nil { + return fmt.Errorf("let us be on the safe side and retry later") + } + + if len(existingIPAMClaim.OwnerReferences) == 1 && existingIPAMClaim.OwnerReferences[0].UID == ownerInfo.UID { + r.Log.Info("found existing IPAMClaim belonging to this VM/VMI, nothing to do", "UID", ownerInfo.UID) + return nil + } else { + err := fmt.Errorf("failed since it found an existing IPAMClaim for %q", claimKey.Name) + r.Log.Error(err, "leaked IPAMClaim found", "existing owner", existingIPAMClaim.UID) + return err + } + } + r.Log.Error(err, "failed to create the IPAMClaim") + return err + } + return nil +}