From bccf6d68c5412398a6413954518c4267f3b3e7c4 Mon Sep 17 00:00:00 2001 From: Kostiantyn Kalynovskyi Date: Fri, 29 Jan 2021 21:23:29 +0000 Subject: [PATCH] Adding ability to override VM interface Change-Id: I35cc4368c4f7a0e0642c16d089bda910412a832c --- .../bases/airship.airshipit.org_vinoes.yaml | 4 + docs/api/vino.md | 11 ++ go.mod | 1 + pkg/api/v1/vino_types.go | 6 +- pkg/api/v1/zz_generated.deepcopy.go | 6 +- pkg/controllers/vino_controller.go | 32 ++++ pkg/controllers/vino_controller_test.go | 180 ++++++++++++++++++ 7 files changed, 234 insertions(+), 6 deletions(-) create mode 100644 pkg/controllers/vino_controller_test.go diff --git a/config/crd/bases/airship.airshipit.org_vinoes.yaml b/config/crd/bases/airship.airshipit.org_vinoes.yaml index 98f4b9c..fd14e3d 100644 --- a/config/crd/bases/airship.airshipit.org_vinoes.yaml +++ b/config/crd/bases/airship.airshipit.org_vinoes.yaml @@ -88,6 +88,10 @@ spec: type: object subnet: type: string + vmInterfaceName: + description: VMinterfaceName defines the interface name to be used + as bridge for virtual machines + type: string type: object nodeSelector: description: Define nodelabel parameters diff --git a/docs/api/vino.md b/docs/api/vino.md index 2c674de..f6e79da 100644 --- a/docs/api/vino.md +++ b/docs/api/vino.md @@ -340,6 +340,17 @@ VMRoutes + + +vmInterfaceName
+ +string + + + +

VMinterfaceName defines the interface name to be used as bridge for virtual machines

+ + diff --git a/go.mod b/go.mod index 9556140..9b5f833 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.13 require ( github.com/evanphx/json-patch v4.9.0+incompatible github.com/go-logr/logr v0.3.0 + github.com/go-logr/zapr v0.2.0 github.com/metal3-io/baremetal-operator v0.0.0-20210111093319-93a6fd209b9a github.com/onsi/ginkgo v1.14.2 github.com/onsi/gomega v1.10.2 diff --git a/pkg/api/v1/vino_types.go b/pkg/api/v1/vino_types.go index eaad9cf..2392201 100644 --- a/pkg/api/v1/vino_types.go +++ b/pkg/api/v1/vino_types.go @@ -31,6 +31,8 @@ const ( VinoLabelDSNamespaceSelector = VinoLabel + "/" + "cr-namespace" // VinoFinalizer constant VinoFinalizer = "vino.airshipit.org" + // EnvVarVMInterfaceName environment variable that is used to find VM interface to use for vms + EnvVarVMInterfaceName = "VM_BRIDGE_INTERFACE" ) // VinoSpec defines the desired state of Vino @@ -40,7 +42,7 @@ type VinoSpec struct { // Define CPU configuration CPUConfiguration *CPUConfiguration `json:"configuration,omitempty"` // Define network parameters - Network *Network `json:"networks,omitempty"` + Network Network `json:"networks,omitempty"` // Define node details Node []NodeSet `json:"nodes,omitempty"` // DaemonSetOptions defines how vino will spawn daemonset on nodes @@ -68,6 +70,8 @@ type Network struct { AllocationStop string `json:"allocationStop,omitempty"` DNSServers []string `json:"dns_servers,omitempty"` Routes *VMRoutes `json:"routes,omitempty"` + // VMinterfaceName defines the interface name to be used as bridge for virtual machines + VMInterfaceName string `json:"vmInterfaceName,omitempty"` } // VMRoutes defined diff --git a/pkg/api/v1/zz_generated.deepcopy.go b/pkg/api/v1/zz_generated.deepcopy.go index d58e562..eb3c595 100644 --- a/pkg/api/v1/zz_generated.deepcopy.go +++ b/pkg/api/v1/zz_generated.deepcopy.go @@ -315,11 +315,7 @@ func (in *VinoSpec) DeepCopyInto(out *VinoSpec) { *out = new(CPUConfiguration) **out = **in } - if in.Network != nil { - in, out := &in.Network, &out.Network - *out = new(Network) - (*in).DeepCopyInto(*out) - } + in.Network.DeepCopyInto(&out.Network) if in.Node != nil { in, out := &in.Node, &out.Node *out = make([]NodeSet, len(*in)) diff --git a/pkg/controllers/vino_controller.go b/pkg/controllers/vino_controller.go index caeb240..6378bfc 100644 --- a/pkg/controllers/vino_controller.go +++ b/pkg/controllers/vino_controller.go @@ -514,6 +514,12 @@ func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.Daemo } } + // TODO develop logic to derive all required ENV variables from VINO CR, and pass them + // to setENV function instead + if vino.Spec.Network.VMInterfaceName != "" { + setEnv(ctx, ds, vino) + } + // this will help avoid colisions if we have two vino CRs in the same namespace ds.Spec.Selector.MatchLabels[vinov1.VinoLabelDSNameSelector] = vino.Name ds.Spec.Template.ObjectMeta.Labels[vinov1.VinoLabelDSNameSelector] = vino.Name @@ -522,6 +528,32 @@ func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.Daemo ds.Spec.Template.ObjectMeta.Labels[vinov1.VinoLabelDSNamespaceSelector] = vino.Namespace } +func setEnv(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) { + for i, c := range ds.Spec.Template.Spec.Containers { + var set bool + for j, envVar := range c.Env { + if envVar.Name == vinov1.EnvVarVMInterfaceName { + logr.FromContext(ctx).Info("found env variable with vm interface name on daemonset template, overriding it", + "vino instance", vino.Namespace+"/"+vino.Name, + "container name", c.Name, + "value", envVar.Value, + ) + ds.Spec.Template.Spec.Containers[i].Env[j].Value = vino.Spec.Network.VMInterfaceName + set = true + break + } + } + if !set { + ds.Spec.Template.Spec.Containers[i].Env = append( + ds.Spec.Template.Spec.Containers[i].Env, corev1.EnvVar{ + Name: vinov1.EnvVarVMInterfaceName, + Value: vino.Spec.Network.VMInterfaceName, + }, + ) + } + } +} + func (r *VinoReconciler) waitDaemonSet(ctx context.Context, ds *appsv1.DaemonSet) error { logger := logr.FromContext(ctx).WithValues( "daemonset", ds.Namespace+"/"+ds.Name) diff --git a/pkg/controllers/vino_controller_test.go b/pkg/controllers/vino_controller_test.go new file mode 100644 index 0000000..a265bf4 --- /dev/null +++ b/pkg/controllers/vino_controller_test.go @@ -0,0 +1,180 @@ +package controllers + +import ( + "context" + + "github.com/go-logr/logr" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + vinov1 "vino/pkg/api/v1" +) + +func testDS() *appsv1.DaemonSet { + return &appsv1.DaemonSet{Spec: appsv1.DaemonSetSpec{ + Template: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{}}}}} +} + +func testVINO() *vinov1.Vino { + return &vinov1.Vino{ + ObjectMeta: v1.ObjectMeta{}, + Spec: vinov1.VinoSpec{ + Network: vinov1.Network{}}} +} + +var _ = Describe("Test Setting Env variables", func() { + Context("when daemonset is created", func() { + l := logr.Discard() + ctx := logr.NewContext(context.Background(), l) + Context("no containers defined in damonset", func() { + It("does nothing", func() { + ds := testDS() + setEnv(ctx, ds, testVINO()) + Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(0)) + }) + + }) + Context("when daemonset has containers", func() { + It("sets env interface variable to every container", func() { + vino := testVINO() + ifName := "eth0" + vino.Spec.Network.VMInterfaceName = ifName + ds := testDS() + ds.Spec.Template.Spec.Containers = make([]corev1.Container, 3) + + setEnv(ctx, ds, vino) + + for _, container := range ds.Spec.Template.Spec.Containers { + Expect(container.Env).To(HaveLen(1)) + Expect(container.Env[0].Name).To(Equal(vinov1.EnvVarVMInterfaceName)) + Expect(container.Env[0].Value).To(Equal(ifName)) + } + }) + + }) + Context("when daemonset has container with wrong env var values", func() { + It("overrides that variable in the container", func() { + vino := testVINO() + ifName := "eth0" + vino.Spec.Network.VMInterfaceName = ifName + ds := testDS() + ds.Spec.Template.Spec.Containers = []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: vinov1.EnvVarVMInterfaceName, + Value: "wrong-value", + }, + }, + }, + } + + setEnv(ctx, ds, vino) + Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(1)) + container := ds.Spec.Template.Spec.Containers[0] + Expect(container.Env).To(HaveLen(1)) + Expect(container.Env[0].Name).To(Equal(vinov1.EnvVarVMInterfaceName)) + Expect(container.Env[0].Value).To(Equal(ifName)) + }) + }) + Context("when daemonset containers don't have required variable", func() { + It("overrides that variable in the container", func() { + vino := testVINO() + ifName := "eth0" + vino.Spec.Network.VMInterfaceName = ifName + ds := testDS() + ds.Spec.Template.Spec.Containers = []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: "bar", + Value: "wrong-value", + }, + { + Name: "foo", + Value: "wrong-value", + }, + }, + }, + { + Env: []corev1.EnvVar{ + { + Name: "foo", + Value: "wrong-value", + }, + { + Name: "bar", + Value: "wrong-value", + }, + }, + }, + } + + setEnv(ctx, ds, vino) + + Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(2)) + for _, container := range ds.Spec.Template.Spec.Containers { + Expect(container.Env).To(HaveLen(3)) + for i, env := range container.Env { + if i == len(container.Env)-1 { + // only last env holds the correct values + Expect(env.Name).To(Equal(vinov1.EnvVarVMInterfaceName)) + Expect(env.Value).To(Equal(ifName)) + } else { + Expect(env.Name).NotTo(Equal(vinov1.EnvVarVMInterfaceName)) + Expect(env.Value).NotTo(Equal(ifName)) + } + } + } + + }) + }) + Context("when daemonset container has many variables", func() { + It("it sets required variable only single time", func() { + vino := testVINO() + ifName := "eth0" + vino.Spec.Network.VMInterfaceName = ifName + ds := testDS() + ds.Spec.Template.Spec.Containers = []corev1.Container{ + { + Env: []corev1.EnvVar{ + { + Name: "foo", + Value: "wrong-value", + }, + { + Name: vinov1.EnvVarVMInterfaceName, + Value: "wrong-value", + }, + + { + Name: "bar", + Value: "wrong-value", + }, + }, + }, + } + + setEnv(ctx, ds, vino) + Expect(ds.Spec.Template.Spec.Containers).To(HaveLen(1)) + container := ds.Spec.Template.Spec.Containers[0] + Expect(container.Env).To(HaveLen(3)) + for i, env := range container.Env { + if i == 1 { + // only env var with index 1 holds the correct values + Expect(env.Name).To(Equal(vinov1.EnvVarVMInterfaceName)) + Expect(env.Value).To(Equal(ifName)) + } else { + Expect(env.Name).NotTo(Equal(vinov1.EnvVarVMInterfaceName)) + Expect(env.Value).NotTo(Equal(ifName)) + } + } + }) + }) + }) +})