diff --git a/pkg/controller/plan/adapter/ocp/BUILD.bazel b/pkg/controller/plan/adapter/ocp/BUILD.bazel index d097e5931..87ff7d9d5 100644 --- a/pkg/controller/plan/adapter/ocp/BUILD.bazel +++ b/pkg/controller/plan/adapter/ocp/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +load("@io_bazel_rules_go//go:def.bzl", "go_library") go_library( name = "ocp", @@ -7,7 +7,6 @@ go_library( "builder.go", "client.go", "destinationclient.go", - "util.go", "validator.go", ], importpath = "github.com/konveyor/forklift-controller/pkg/controller/plan/adapter/ocp", @@ -38,10 +37,3 @@ go_library( "//vendor/sigs.k8s.io/controller-runtime/pkg/client/config", ], ) - -go_test( - name = "ocp_test", - srcs = ["util_test.go"], - embed = [":ocp"], - deps = ["//pkg/apis/forklift/v1beta1/ref"], -) diff --git a/pkg/controller/plan/adapter/ocp/builder.go b/pkg/controller/plan/adapter/ocp/builder.go index d11d85bf7..dc142fdaf 100644 --- a/pkg/controller/plan/adapter/ocp/builder.go +++ b/pkg/controller/plan/adapter/ocp/builder.go @@ -17,6 +17,7 @@ import ( "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/ref" planbase "github.com/konveyor/forklift-controller/pkg/controller/plan/adapter/base" plancontext "github.com/konveyor/forklift-controller/pkg/controller/plan/context" + ocpclient "github.com/konveyor/forklift-controller/pkg/lib/client/openshift" core "k8s.io/api/core/v1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" @@ -477,7 +478,7 @@ func (r *Builder) mapNetworks(sourceVm *cnv.VirtualMachine, targetVmSpec *cnv.Vi switch { case network.Multus != nil: - name, namespace := GetNetworkNameAndNamespace(network.Multus.NetworkName, &ref.Ref{Name: sourceVm.Name, Namespace: sourceVm.Namespace}) + name, namespace := ocpclient.GetNetworkNameAndNamespace(network.Multus.NetworkName, &ref.Ref{Name: sourceVm.Name, Namespace: sourceVm.Namespace}) pair, found := r.Map.Network.FindNetworkByNameAndNamespace(namespace, name) if !found { r.Log.Info("Network not found", "namespace", namespace, "name", name) diff --git a/pkg/controller/plan/adapter/ocp/validator.go b/pkg/controller/plan/adapter/ocp/validator.go index 0e75cc2d1..ed31ae4f2 100644 --- a/pkg/controller/plan/adapter/ocp/validator.go +++ b/pkg/controller/plan/adapter/ocp/validator.go @@ -10,6 +10,7 @@ import ( "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/ref" "github.com/konveyor/forklift-controller/pkg/controller/provider/web" + ocpclient "github.com/konveyor/forklift-controller/pkg/lib/client/openshift" core "k8s.io/api/core/v1" cnv "kubevirt.io/api/core/v1" k8sclient "sigs.k8s.io/controller-runtime/pkg/client" @@ -164,7 +165,7 @@ func (r *Validator) NetworksMapped(vmRef ref.Ref) (ok bool, err error) { return false, err } } else if net.Multus != nil { - name, namespace := GetNetworkNameAndNamespace(net.Multus.NetworkName, &vmRef) + name, namespace := ocpclient.GetNetworkNameAndNamespace(net.Multus.NetworkName, &vmRef) _, found := r.plan.Referenced.Map.Network.FindNetworkByNameAndNamespace(namespace, name) if !found { err = liberr.Wrap( diff --git a/pkg/forklift-api/webhooks/mutating-webhook/mutators/BUILD.bazel b/pkg/forklift-api/webhooks/mutating-webhook/mutators/BUILD.bazel index 6d6cb64f5..792bb324c 100644 --- a/pkg/forklift-api/webhooks/mutating-webhook/mutators/BUILD.bazel +++ b/pkg/forklift-api/webhooks/mutating-webhook/mutators/BUILD.bazel @@ -11,6 +11,7 @@ go_library( visibility = ["//visibility:public"], deps = [ "//pkg/apis/forklift/v1beta1", + "//pkg/apis/forklift/v1beta1/ref", "//pkg/forklift-api/webhooks/util", "//pkg/lib/client/openshift", "//pkg/lib/error", diff --git a/pkg/forklift-api/webhooks/mutating-webhook/mutators/plan-mutator.go b/pkg/forklift-api/webhooks/mutating-webhook/mutators/plan-mutator.go index c8efd47e9..14a8cc980 100644 --- a/pkg/forklift-api/webhooks/mutating-webhook/mutators/plan-mutator.go +++ b/pkg/forklift-api/webhooks/mutating-webhook/mutators/plan-mutator.go @@ -7,8 +7,10 @@ import ( net "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1" api "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1" + "github.com/konveyor/forklift-controller/pkg/apis/forklift/v1beta1/ref" "github.com/konveyor/forklift-controller/pkg/forklift-api/webhooks/util" ocp "github.com/konveyor/forklift-controller/pkg/lib/client/openshift" + ocpclient "github.com/konveyor/forklift-controller/pkg/lib/client/openshift" admissionv1 "k8s.io/api/admission/v1beta1" core "k8s.io/api/core/v1" k8serr "k8s.io/apimachinery/pkg/api/errors" @@ -78,9 +80,10 @@ func (mutator *PlanMutator) setTransferNetworkIfNotSet() (bool, error) { } if network := targetProvider.Annotations["forklift.konveyor.io/defaultTransferNetwork"]; network != "" { + name, namespace := ocpclient.GetNetworkNameAndNamespace(network, &ref.Ref{Name: network, Namespace: mutator.plan.Spec.TargetNamespace}) key := client.ObjectKey{ - Namespace: mutator.plan.Spec.TargetNamespace, - Name: network, + Namespace: namespace, + Name: name, } var tcl client.Client // target client, i.e., client to a possibly remote cluster @@ -111,8 +114,8 @@ func (mutator *PlanMutator) setTransferNetworkIfNotSet() (bool, error) { if err = tcl.Get(context.TODO(), key, netAttachDef); err == nil { log.Info("Patching the plan's transfer network") mutator.plan.Spec.TransferNetwork = &core.ObjectReference{ - Name: network, - Namespace: mutator.plan.Spec.TargetNamespace, + Namespace: key.Namespace, + Name: key.Name, } planChanged = true } else if !k8serr.IsNotFound(err) { // TODO: else if !NotFound ... diff --git a/pkg/lib/client/openshift/BUILD.bazel b/pkg/lib/client/openshift/BUILD.bazel index b6492b1af..586c04f58 100644 --- a/pkg/lib/client/openshift/BUILD.bazel +++ b/pkg/lib/client/openshift/BUILD.bazel @@ -2,11 +2,15 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "openshift", - srcs = ["client.go"], + srcs = [ + "client.go", + "util.go", + ], importpath = "github.com/konveyor/forklift-controller/pkg/lib/client/openshift", visibility = ["//visibility:public"], deps = [ "//pkg/apis/forklift/v1beta1", + "//pkg/apis/forklift/v1beta1/ref", "//pkg/lib/error", "//vendor/k8s.io/api/core/v1:core", "//vendor/k8s.io/client-go/kubernetes/scheme", @@ -19,10 +23,14 @@ go_library( go_test( name = "openshift_test", - srcs = ["client_test.go"], + srcs = [ + "client_test.go", + "util_test.go", + ], embed = [":openshift"], deps = [ "//pkg/apis/forklift/v1beta1", + "//pkg/apis/forklift/v1beta1/ref", "//vendor/k8s.io/api/core/v1:core", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:meta", ], diff --git a/pkg/controller/plan/adapter/ocp/util.go b/pkg/lib/client/openshift/util.go similarity index 100% rename from pkg/controller/plan/adapter/ocp/util.go rename to pkg/lib/client/openshift/util.go diff --git a/pkg/controller/plan/adapter/ocp/util_test.go b/pkg/lib/client/openshift/util_test.go similarity index 100% rename from pkg/controller/plan/adapter/ocp/util_test.go rename to pkg/lib/client/openshift/util_test.go diff --git a/tests/suit/forklift_test.go b/tests/suit/forklift_test.go index 2f1c4efce..741ea04ad 100644 --- a/tests/suit/forklift_test.go +++ b/tests/suit/forklift_test.go @@ -140,4 +140,75 @@ var _ = Describe("Forklift", func() { Expect(plan.Annotations[AnnPopulatorLabels]).To(Equal("True")) }) }) + + Context("Plan with provider that is set with transfer network that includes namespace", func() { + providerNetworkNamespace := "default" + providerNetworkName := "my-network" + providerNetwork := providerNetworkNamespace + "/" + providerNetworkName + + var provider *forkliftv1.Provider + var targetNS *v1.Namespace + var err error + + BeforeEach(func() { + targetNS, err = f.CreateNamespace("target", map[string]string{}) + Expect(err).ToNot(HaveOccurred()) + targetNS, err = f.CreateNamespace(providerNetworkNamespace, map[string]string{}) + Expect(err).ToNot(HaveOccurred()) + By("Create target Openshift provider") + annotations := map[string]string{"forklift.konveyor.io/defaultTransferNetwork": providerNetwork} + target := utils.NewProvider(utils.TargetProviderName, forkliftv1.OpenShift, namespace, annotations, map[string]string{}, "", nil) + err = utils.CreateProviderFromDefinition(f.CrClient, target) + Expect(err).ToNot(HaveOccurred()) + _, err = utils.WaitForProviderReadyWithTimeout(f.CrClient, namespace, utils.TargetProviderName, 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + By("Create oVirt provider") + pr := utils.NewProvider(ovirtProviderName, forkliftv1.OVirt, namespace, map[string]string{}, map[string]string{}, f.OvirtClient.OvirtURL, secret) + err = utils.CreateProviderFromDefinition(f.CrClient, pr) + Expect(err).ToNot(HaveOccurred()) + provider, err = utils.WaitForProviderReadyWithTimeout(f.CrClient, namespace, ovirtProviderName, 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + By("Create Network Map") + networkMapDef := utils.NewNetworkMap(namespace, *provider, networkMapName, vmData.GetVMNics()[0]) + err = utils.CreateNetworkMapFromDefinition(f.CrClient, networkMapDef) + Expect(err).ToNot(HaveOccurred()) + err = utils.WaitForNetworkMapReadyWithTimeout(f.CrClient, namespace, networkMapName, 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + By("Create Storage Map") + storageMapDef := utils.NewStorageMap(namespace, *provider, test_storage_map_name, vmData.GetVMSDs(), ovirtStorageClass) + err = utils.CreateStorageMapFromDefinition(f.CrClient, storageMapDef) + Expect(err).ToNot(HaveOccurred()) + err = utils.WaitForStorageMapReadyWithTimeout(f.CrClient, namespace, test_storage_map_name, 30*time.Second) + Expect(err).ToNot(HaveOccurred()) + }) + + It("Namespaced transfer network should not be set to that of the Provider when it doesn't exist", func() { + By("Create plan for provider with defaultTransferNetwork") + planDef := utils.NewPlanWithVmId(*provider, namespace, test_plan_name, test_storage_map_name, networkMapName, targetNS.Name, []string{vmData.GetTestVMId()}) + err = utils.CreatePlanFromDefinition(f.CrClient, planDef) + Expect(err).ToNot(HaveOccurred()) + err, plan := utils.WaitForPlanReadyWithTimeout(f.CrClient, namespace, planDef.Name, 15*time.Second) + Expect(err).ToNot(HaveOccurred()) + By("Verify created plan") + Expect(plan).ToNot(BeNil()) + Expect(plan.Spec.TransferNetwork).To(BeNil()) + }) + + It("Namespaced transfer network should be set to that of the Provider when it exists", func() { + By("Create namespaced Network Attachment Definition") + err, _ = utils.CreateNetworkAttachmentDefinition(f.CrClient, providerNetworkName, providerNetworkNamespace) + Expect(err).ToNot(HaveOccurred()) + By("Create plan for provider with defaultTransferNetwork") + planDef := utils.NewPlanWithVmId(*provider, namespace, test_plan_name, test_storage_map_name, networkMapName, targetNS.Name, []string{vmData.GetTestVMId()}) + err = utils.CreatePlanFromDefinition(f.CrClient, planDef) + Expect(err).ToNot(HaveOccurred()) + err, plan := utils.WaitForPlanReadyWithTimeout(f.CrClient, namespace, planDef.Name, 15*time.Second) + Expect(err).ToNot(HaveOccurred()) + By("Verify created plan for provider with defaultTransferNetwork") + Expect(plan).ToNot(BeNil()) + Expect(plan.Spec.TransferNetwork).ToNot(BeNil()) + Expect(plan.Spec.TransferNetwork.Name).To(Equal(providerNetworkName)) + Expect(plan.Spec.TransferNetwork.Namespace).To(Equal(providerNetworkNamespace)) + }) + }) })