From 71e4891d8b25251f2196e3f371c2ca00f8daf1e6 Mon Sep 17 00:00:00 2001 From: Sam Lucidi Date: Tue, 3 Dec 2024 10:50:35 -0500 Subject: [PATCH 1/2] Look for annotation on NAD to set transfer route The Plan controller will now look for an annotation `forklift.konveyor.io/route` on the NetworkAttachmentDefinition that was specified as the transfer network for the Plan. The annotation is expected to have a single IP address as its value, which will be set as the default route when configuring the secondary network on the importer pods. The annotation is optional, but if it is present then the Plan validator will ensure that its value is a valid IP address. Signed-off-by: Sam Lucidi --- pkg/controller/plan/kubevirt.go | 55 ++++++++++++++++++++++++++++--- pkg/controller/plan/validation.go | 21 ++++++++++-- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/pkg/controller/plan/kubevirt.go b/pkg/controller/plan/kubevirt.go index 0d5be1f30..687f9d378 100644 --- a/pkg/controller/plan/kubevirt.go +++ b/pkg/controller/plan/kubevirt.go @@ -2,11 +2,13 @@ package plan import ( "context" + "encoding/json" "encoding/xml" "errors" "fmt" "io" "math/rand" + "net" "net/http" "os" "path" @@ -15,6 +17,7 @@ import ( "strings" "time" + k8snet "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" planbase "github.com/konveyor/forklift-controller/pkg/controller/plan/adapter/base" "github.com/konveyor/forklift-controller/pkg/controller/plan/util" "github.com/konveyor/forklift-controller/pkg/controller/provider/web" @@ -55,6 +58,9 @@ import ( const ( // Transfer network annotation (value=network-attachment-definition name) AnnTransferNetwork = "k8s.v1.cni.cncf.io/networks" + // Annotation to specify the default route for the transfer network. + // To be set on the transfer network NAD by the end user. + AnnForkliftNetworkRoute = "forklift.konveyor.io/route" // Contains validations for a Kubevirt VM. Needs to be removed when // creating a VM from a template. AnnKubevirtValidations = "vm.kubevirt.io/validations" @@ -1227,9 +1233,12 @@ func (r *KubeVirt) dataVolumes(vm *plan.VMStatus, secret *core.Secret, configMap annotations[planbase.AnnRetainAfterCompletion] = "true" } if r.Plan.Spec.TransferNetwork != nil { - annotations[AnnTransferNetwork] = path.Join( - r.Plan.Spec.TransferNetwork.Namespace, r.Plan.Spec.TransferNetwork.Name) + err = r.setTransferNetwork(annotations) + if err != nil { + return + } } + if r.Plan.Spec.Warm || !r.Destination.Provider.IsHost() || r.Plan.IsSourceProviderOCP() { // Set annotation for WFFC storage classes. Note that we create data volumes while // running a cold migration to the local cluster only when the source is either OpenShift @@ -1757,8 +1766,10 @@ func (r *KubeVirt) guestConversionPod(vm *plan.VMStatus, vmVolumes []cnv.Volume, // pod annotations annotations := map[string]string{} if r.Plan.Spec.TransferNetwork != nil { - annotations[AnnTransferNetwork] = path.Join( - r.Plan.Spec.TransferNetwork.Namespace, r.Plan.Spec.TransferNetwork.Name) + err = r.setTransferNetwork(annotations) + if err != nil { + return + } } // pod pod = &core.Pod{ @@ -2351,6 +2362,42 @@ func (r *KubeVirt) vmAllButMigrationLabels(vmRef ref.Ref) (labels map[string]str return } +func (r *KubeVirt) setTransferNetwork(annotations map[string]string) (err error) { + key := client.ObjectKey{ + Namespace: r.Plan.Spec.TransferNetwork.Namespace, + Name: r.Plan.Spec.TransferNetwork.Name, + } + netAttachDef := &k8snet.NetworkAttachmentDefinition{} + err = r.Get(context.TODO(), key, netAttachDef) + if err != nil { + err = liberr.Wrap(err) + return + } + nse := k8snet.NetworkSelectionElement{ + Namespace: r.Plan.Spec.TransferNetwork.Namespace, + Name: r.Plan.Spec.TransferNetwork.Name, + } + route, found := netAttachDef.Annotations[AnnForkliftNetworkRoute] + if found { + ip := net.ParseIP(route) + if ip != nil { + nse.GatewayRequest = []net.IP{ip} + } else { + err = liberr.New( + "Transfer network default route annotation is not a valid IP address.", + "route", route) + return + } + } + transferNetwork, err := json.Marshal([]k8snet.NetworkSelectionElement{nse}) + if err != nil { + err = liberr.Wrap(err) + return + } + annotations[AnnTransferNetwork] = string(transferNetwork) + return +} + // Represents a CDI DataVolume, its associated PVC, and added behavior. type ExtendedDataVolume struct { *cdi.DataVolume diff --git a/pkg/controller/plan/validation.go b/pkg/controller/plan/validation.go index 49dd1a71f..cbdb61dec 100644 --- a/pkg/controller/plan/validation.go +++ b/pkg/controller/plan/validation.go @@ -6,10 +6,11 @@ import ( "encoding/hex" "errors" "fmt" + "net" "path" "strconv" - net "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" + k8snet "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" api "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1" refapi "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/ref" "github.com/konveyor/forklift-controller/pkg/controller/plan/adapter" @@ -640,11 +641,18 @@ func (r *Reconciler) validateTransferNetwork(plan *api.Plan) (err error) { Reason: NotFound, Message: "Transfer network is not valid.", } + notValid := libcnd.Condition{ + Type: TransferNetNotValid, + Status: True, + Category: Critical, + Reason: NotValid, + Message: "Transfer network default route annotation is not a valid IP address.", + } key := client.ObjectKey{ Namespace: plan.Spec.TransferNetwork.Namespace, Name: plan.Spec.TransferNetwork.Name, } - netAttachDef := &net.NetworkAttachmentDefinition{} + netAttachDef := &k8snet.NetworkAttachmentDefinition{} err = r.Get(context.TODO(), key, netAttachDef) if k8serr.IsNotFound(err) { err = nil @@ -653,6 +661,15 @@ func (r *Reconciler) validateTransferNetwork(plan *api.Plan) (err error) { } if err != nil { err = liberr.Wrap(err) + return + } + route, found := netAttachDef.Annotations[AnnForkliftNetworkRoute] + if !found { + return + } + ip := net.ParseIP(route) + if ip == nil { + plan.Status.SetCondition(notValid) } return From 0defc4743cc0e9b539e46332504a960e6ec80381 Mon Sep 17 00:00:00 2001 From: Sam Lucidi Date: Thu, 5 Dec 2024 11:46:28 -0500 Subject: [PATCH 2/2] Fallback to using multus annotation Signed-off-by: Sam Lucidi --- pkg/controller/plan/kubevirt.go | 34 +++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/pkg/controller/plan/kubevirt.go b/pkg/controller/plan/kubevirt.go index 687f9d378..74dfd8317 100644 --- a/pkg/controller/plan/kubevirt.go +++ b/pkg/controller/plan/kubevirt.go @@ -56,6 +56,10 @@ import ( // Annotations const ( + // Legacy transfer network annotation (value=network-attachment-definition name) + // FIXME: this should be phased out and replaced with the + // k8s.v1.cni.cncf.io/networks annotation. + AnnLegacyTransferNetwork = "v1.multus-cni.io/default-network" // Transfer network annotation (value=network-attachment-definition name) AnnTransferNetwork = "k8s.v1.cni.cncf.io/networks" // Annotation to specify the default route for the transfer network. @@ -2362,6 +2366,12 @@ func (r *KubeVirt) vmAllButMigrationLabels(vmRef ref.Ref) (labels map[string]str return } +// setTransferNetwork sets the transfer network annotation on the DataVolume so +// that it can be used by the importer pod. If the `forklift.konveyor.io/route` annotation +// is present on the referenced NAD, then it will be used with the `k8s.v1.cni.cncf.io/networks` annotation +// to set the default route. If not, this will fall back to setting the `v1.multus-cni.io/default-network` annotation +// with the namespaced name of the NAD. +// FIXME: the codepath using the multus annotation should be phased out. func (r *KubeVirt) setTransferNetwork(annotations map[string]string) (err error) { key := client.ObjectKey{ Namespace: r.Plan.Spec.TransferNetwork.Namespace, @@ -2373,12 +2383,13 @@ func (r *KubeVirt) setTransferNetwork(annotations map[string]string) (err error) err = liberr.Wrap(err) return } - nse := k8snet.NetworkSelectionElement{ - Namespace: r.Plan.Spec.TransferNetwork.Namespace, - Name: r.Plan.Spec.TransferNetwork.Name, - } + route, found := netAttachDef.Annotations[AnnForkliftNetworkRoute] if found { + nse := k8snet.NetworkSelectionElement{ + Namespace: key.Namespace, + Name: key.Name, + } ip := net.ParseIP(route) if ip != nil { nse.GatewayRequest = []net.IP{ip} @@ -2388,13 +2399,16 @@ func (r *KubeVirt) setTransferNetwork(annotations map[string]string) (err error) "route", route) return } + transferNetwork, jErr := json.Marshal([]k8snet.NetworkSelectionElement{nse}) + if jErr != nil { + err = liberr.Wrap(jErr) + return + } + annotations[AnnTransferNetwork] = string(transferNetwork) + } else { + annotations[AnnLegacyTransferNetwork] = path.Join(key.Namespace, key.Name) } - transferNetwork, err := json.Marshal([]k8snet.NetworkSelectionElement{nse}) - if err != nil { - err = liberr.Wrap(err) - return - } - annotations[AnnTransferNetwork] = string(transferNetwork) + return }