Move vino Spec ConfigMap to an annotation

This patchset moves vino spec from configMap to annotation.
Also it utilizes vino builder API object. The object is not
yet fully developed.

In next steps will be to update vino builder, but it is in
a separate repository right now, and isn't fully functional.

Change-Id: Ifad74ebafe0e8444161549bab69a4c7f5a8ee58f

config/manager/flavor-templates.yaml

@@ -62,14 +62,12 @@ flavorTemplates:
 
           <controller type="usb" index="0" model="piix3-uhci">
             <alias name="usb"/>
-            <address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x2"/>
           </controller>
           <controller type="pci" index="0" model="pci-root">
             <alias name="pci.0"/>
           </controller>
           <controller type="ide" index="0">
             <alias name="ide"/>
-            <address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x1"/>
           </controller>
 
           # for each interface defined in vino, e.g.
@@ -78,7 +76,6 @@ flavorTemplates:
             <mac address='{{ if_values.macAddress }}'/>
             <source bridge='{{ if_name }}'/>
             <model type='virtio'/>
-            <address type='pci' domain='0x0000' bus='0x00' slot='0x0{{ loop.index0 }}' function='0x0'/>
           </interface>
           {% endfor %}
 
@@ -99,7 +96,6 @@ flavorTemplates:
           <memballoon model="virtio">
             <stats period="10"/>
             <alias name="balloon0"/>
-            <address type="pci" domain="0x0000" bus="0x00" slot="0x06" function="0x0"/>
           </memballoon>
         </devices>
         <seclabel type="dynamic" model="dac" relabel="yes">
@@ -181,14 +177,12 @@ flavorTemplates:
 
           <controller type="usb" index="0" model="piix3-uhci">
             <alias name="usb"/>
-            <address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x2"/>
           </controller>
           <controller type="pci" index="0" model="pci-root">
             <alias name="pci.0"/>
           </controller>
           <controller type="ide" index="0">
             <alias name="ide"/>
-            <address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x1"/>
           </controller>
 
           {% for if_name, if_values in domain.interfaces.items() %}
@@ -196,7 +190,6 @@ flavorTemplates:
             <mac address='{{ if_values.macAddress }}'/>
             <source bridge='{{ if_name }}'/>
             <model type='virtio'/>
-            <address type='pci' domain='0x0000' bus='0x00' slot='0x0{{ loop.index0 }}' function='0x0'/>
           </interface>
           {% endfor %}
 
@@ -217,7 +210,6 @@ flavorTemplates:
           <memballoon model="virtio">
             <stats period="10"/>
             <alias name="balloon0"/>
-            <address type="pci" domain="0x0000" bus="0x00" slot="0x06" function="0x0"/>
           </memballoon>
         </devices>
         <seclabel type="dynamic" model="dac" relabel="yes">

config/manager/flavors.yaml

@@ -1,11 +1,11 @@
 flavors:
   master:
-    vcpus: 4
+    vcpus: 1
     memory: 4
     hugepages: true
     rootSize: 30
   worker:
-    vcpus: 2
+    vcpus: 1
     memory: 2
     hugepages: true
     rootSize: 10

config/samples/vino_cr.yaml

@@ -12,7 +12,7 @@ spec:
     matchLabels:
       beta.kubernetes.io/os: linux
   configuration:
-    cpuExclude: 0-4,54-60
+    cpuExclude: 0-1
     redfishCredentialSecret:
       name: redfishSecret
       namespace: airship-system

docs/api/vino.md

@@ -139,6 +139,31 @@ string
 </tr>
 <tr>
 <td>
+<code>nodes</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.NodeSet">
+[]NodeSet
+</a>
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
+<code>configuration</code><br>
+<em>
+<a href="#airship.airshipit.org/v1.CPUConfiguration">
+CPUConfiguration
+</a>
+</em>
+</td>
+<td>
+<p>(TODO) change json tag to cpuConfiguration when vino-builder has these chanages as well</p>
+</td>
+</tr>
+<tr>
+<td>
 <code>domains</code><br>
 <em>
 <a href="#airship.airshipit.org/v1.BuilderDomain">
@@ -220,6 +245,7 @@ string
 </h3>
 <p>
 (<em>Appears on:</em>
+<a href="#airship.airshipit.org/v1.Builder">Builder</a>, 
 <a href="#airship.airshipit.org/v1.VinoSpec">VinoSpec</a>)
 </p>
 <p>CPUConfiguration CPU node configuration</p>
@@ -890,6 +916,7 @@ map[string]string
 </h3>
 <p>
 (<em>Appears on:</em>
+<a href="#airship.airshipit.org/v1.Builder">Builder</a>, 
 <a href="#airship.airshipit.org/v1.VinoSpec">VinoSpec</a>)
 </p>
 <p>NodeSet node definitions</p>

pkg/api/v1/vino_builder.go

@@ -18,9 +18,12 @@ package v1
 
 // TODO (kkalynovskyi) create an API object for this, and refactor vino-builder to read it from kubernetes.
 type Builder struct {
-	GWIPBridge string                   `json:"gwIPBridge,omitempty"`
-	Networks   []Network                `json:"networks,omitempty"`
-	Domains    map[string]BuilderDomain `json:"domains,omitempty"`
+	GWIPBridge string    `json:"gwIPBridge,omitempty"`
+	Networks   []Network `json:"networks,omitempty"`
+	Nodes      []NodeSet `json:"nodes,omitempty"`
+	// (TODO) change json tag to cpuConfiguration when vino-builder has these chanages as well
+	CPUConfiguration CPUConfiguration         `json:"configuration,omitempty"`
+	Domains          map[string]BuilderDomain `json:"domains,omitempty"`
 }
 
 type BuilderNetworkInterface struct {

pkg/api/v1/vino_types.go

@@ -44,7 +44,7 @@ type VinoSpec struct {
 	// Define nodelabel parameters
 	NodeSelector *NodeSelector `json:"nodeSelector,omitempty"`
 	// Define CPU configuration
-	CPUConfiguration *CPUConfiguration `json:"configuration,omitempty"`
+	CPUConfiguration CPUConfiguration `json:"configuration,omitempty"`
 	// Define network parameters
 	Networks []Network `json:"networks,omitempty"`
 	// Define node details

pkg/api/v1/zz_generated.deepcopy.go

@@ -65,6 +65,14 @@ func (in *Builder) DeepCopyInto(out *Builder) {
 			(*in)[i].DeepCopyInto(&(*out)[i])
 		}
 	}
+	if in.Nodes != nil {
+		in, out := &in.Nodes, &out.Nodes
+		*out = make([]NodeSet, len(*in))
+		for i := range *in {
+			(*in)[i].DeepCopyInto(&(*out)[i])
+		}
+	}
+	out.CPUConfiguration = in.CPUConfiguration
 	if in.Domains != nil {
 		in, out := &in.Domains, &out.Domains
 		*out = make(map[string]BuilderDomain, len(*in))
@@ -505,11 +513,7 @@ func (in *VinoSpec) DeepCopyInto(out *VinoSpec) {
 		*out = new(NodeSelector)
 		(*in).DeepCopyInto(*out)
 	}
-	if in.CPUConfiguration != nil {
-		in, out := &in.CPUConfiguration, &out.CPUConfiguration
-		*out = new(CPUConfiguration)
-		**out = **in
-	}
+	out.CPUConfiguration = in.CPUConfiguration
 	if in.Networks != nil {
 		in, out := &in.Networks, &out.Networks
 		*out = make([]Network, len(*in))

pkg/controllers/bmh.go

@@ -222,7 +222,7 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 	}
 
 	logger.Info("annotating node", "node", k8sNode.Name)
-	if err = r.annotateNode(ctx, k8sNode, nodeNetworkValues, nodeNetworks); err != nil {
+	if err = r.annotateNode(ctx, k8sNode, nodeNetworkValues, vino); err != nil {
 		return err
 	}
 	return nil
@@ -298,11 +298,13 @@ func (r *VinoReconciler) domainSpecificNetValues(
 func (r *VinoReconciler) annotateNode(ctx context.Context,
 	k8sNode *corev1.Node,
 	domainInterfaceValues map[string]generatedValues,
-	networks []vinov1.Network) error {
+	vino *vinov1.Vino) error {
 	logr.FromContext(ctx).Info("Getting GW bridge IP from node", "node", k8sNode.Name)
 	builderValues := vinov1.Builder{
-		Domains:  make(map[string]vinov1.BuilderDomain),
-		Networks: networks,
+		Domains:          make(map[string]vinov1.BuilderDomain),
+		Networks:         vino.Spec.Networks,
+		Nodes:            vino.Spec.Nodes,
+		CPUConfiguration: vino.Spec.CPUConfiguration,
 	}
 	for domainName, domain := range domainInterfaceValues {
 		builderDomain := vinov1.BuilderDomain{

pkg/controllers/bmh_test.go

@@ -105,8 +105,9 @@ var _ = Describe("Test BMH reconciliation", func() {
 			rack1 := "r1"
 			server1 := "s1"
 			node1Labels := map[string]string{
-				rackLabel:   rack1,
-				serverLabel: server1,
+				rackLabel:                            rack1,
+				serverLabel:                          server1,
+				vinov1.VinoDefaultGatewayBridgeLabel: "127.0.0.1",
 			}
 			node1 := &corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{
@@ -126,7 +127,10 @@ var _ = Describe("Test BMH reconciliation", func() {
 			node2 := &corev1.Node{
 				ObjectMeta: metav1.ObjectMeta{
 					Name:        "node02",
-					Annotations: make(map[string]string),
+					Annotations: map[string]string{},
+					Labels: map[string]string{
+						vinov1.VinoDefaultGatewayBridgeLabel: "127.0.0.1",
+					},
 				},
 				Status: corev1.NodeStatus{
 					Addresses: []corev1.NodeAddress{

pkg/controllers/vino_controller.go

@@ -99,11 +99,6 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 		}
 	}
 
-	err = r.reconcileConfigMap(ctx, vino)
-	if err != nil {
-		return ctrl.Result{Requeue: true}, err
-	}
-
 	err = r.reconcileDaemonSet(ctx, vino)
 	if err != nil {
 		return ctrl.Result{Requeue: true}, err
@@ -118,122 +113,10 @@ func (r *VinoReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.
 	return ctrl.Result{}, nil
 }
 
-func (r *VinoReconciler) reconcileConfigMap(ctx context.Context, vino *vinov1.Vino) error {
-	err := r.ensureConfigMap(ctx, vino)
-	if err != nil {
-		err = fmt.Errorf("could not reconcile ConfigMap: %w", err)
-		apimeta.SetStatusCondition(&vino.Status.Conditions, metav1.Condition{
-			Status:             metav1.ConditionFalse,
-			Reason:             vinov1.ReconciliationFailedReason,
-			Message:            err.Error(),
-			Type:               vinov1.ConditionTypeReady,
-			ObservedGeneration: vino.GetGeneration(),
-		})
-		apimeta.SetStatusCondition(&vino.Status.Conditions, metav1.Condition{
-			Status:             metav1.ConditionFalse,
-			Reason:             vinov1.ReconciliationFailedReason,
-			Message:            err.Error(),
-			Type:               vinov1.ConditionTypeConfigMapReady,
-			ObservedGeneration: vino.GetGeneration(),
-		})
-		if patchStatusErr := r.patchStatus(ctx, vino); patchStatusErr != nil {
-			err = kerror.NewAggregate([]error{err, patchStatusErr})
-			err = fmt.Errorf("unable to patch status after ConfigMap reconciliation failed: %w", err)
-		}
-		return err
-	}
-	apimeta.SetStatusCondition(&vino.Status.Conditions, metav1.Condition{
-		Status:             metav1.ConditionTrue,
-		Reason:             vinov1.ReconciliationSucceededReason,
-		Message:            "ConfigMap reconciled",
-		Type:               vinov1.ConditionTypeConfigMapReady,
-		ObservedGeneration: vino.GetGeneration(),
-	})
-	if err = r.patchStatus(ctx, vino); err != nil {
-		err = fmt.Errorf("unable to patch status after ConfigMap reconciliation succeeded: %w", err)
-		return err
-	}
-
-	return nil
-}
-
-func (r *VinoReconciler) ensureConfigMap(ctx context.Context, vino *vinov1.Vino) error {
-	logger := logr.FromContext(ctx)
-
-	generatedCM, err := r.buildConfigMap(ctx, vino)
-	if err != nil {
-		return err
-	}
-	logger.Info("successfully built config map", "new config map data", generatedCM.Data)
-
-	currentCM, err := r.getCurrentConfigMap(ctx, vino)
-	if err != nil {
-		return err
-	}
-
-	if currentCM == nil {
-		logger.Info("current config map is not present in a cluster creating newly generated one")
-		return applyRuntimeObject(
-			ctx,
-			types.NamespacedName{Name: generatedCM.Name, Namespace: generatedCM.Namespace},
-			generatedCM,
-			r.Client)
-	}
-
-	logger.Info("generated config map", "current config map data", currentCM.Data)
-
-	if needsUpdate(generatedCM, currentCM) {
-		logger.Info("current config map needs an update, trying to update it")
-		return r.Client.Update(ctx, generatedCM)
-	}
-	return nil
-}
-
-func (r *VinoReconciler) buildConfigMap(ctx context.Context, vino *vinov1.Vino) (
-	*corev1.ConfigMap, error) {
-	logr.FromContext(ctx).Info("Generating new config map for vino object")
-
-	data, err := yaml.Marshal(vino.Spec)
-	if err != nil {
-		return nil, err
-	}
-
-	return &corev1.ConfigMap{
-		ObjectMeta: metav1.ObjectMeta{
-			Namespace: getRuntimeNamespace(),
-			Name:      r.getConfigMapName(vino),
-		},
-		Data: map[string]string{
-			ConfigMapKeyVinoSpec: string(data),
-		},
-	}, nil
-}
-
-func (r *VinoReconciler) getConfigMapName(vino *vinov1.Vino) string {
-	return fmt.Sprintf("%s-%s", vino.Namespace, vino.Name)
-}
-
 func (r *VinoReconciler) getDaemonSetName(vino *vinov1.Vino) string {
 	return fmt.Sprintf("%s-%s", vino.Namespace, vino.Name)
 }
 
-func (r *VinoReconciler) getCurrentConfigMap(ctx context.Context, vino *vinov1.Vino) (*corev1.ConfigMap, error) {
-	logr.FromContext(ctx).Info("Getting current config map for vino object")
-	cm := &corev1.ConfigMap{}
-	err := r.Get(ctx, types.NamespacedName{
-		Name:      vino.Name,
-		Namespace: vino.Namespace,
-	}, cm)
-	if err != nil {
-		if !apierror.IsNotFound(err) {
-			return cm, err
-		}
-		return nil, nil
-	}
-
-	return cm, nil
-}
-
 func (r *VinoReconciler) patchStatus(ctx context.Context, vino *vinov1.Vino) error {
 	key := client.ObjectKeyFromObject(vino)
 	latest := &vinov1.Vino{}
@@ -243,15 +126,6 @@ func (r *VinoReconciler) patchStatus(ctx context.Context, vino *vinov1.Vino) err
 	return r.Client.Status().Patch(ctx, vino, client.MergeFrom(latest))
 }
 
-func needsUpdate(generated, current *corev1.ConfigMap) bool {
-	for key, value := range generated.Data {
-		if current.Data[key] != value {
-			return true
-		}
-	}
-	return false
-}
-
 func (r *VinoReconciler) reconcileDaemonSet(ctx context.Context, vino *vinov1.Vino) error {
 	err := r.ensureDaemonSet(ctx, vino)
 	if err != nil {
@@ -336,53 +210,10 @@ func (r *VinoReconciler) ensureDaemonSet(ctx context.Context, vino *vinov1.Vino)
 }
 
 func (r *VinoReconciler) decorateDaemonSet(ctx context.Context, ds *appsv1.DaemonSet, vino *vinov1.Vino) {
-	volume := "vino-spec"
-
 	ds.Spec.Template.Spec.NodeSelector = vino.Spec.NodeSelector.MatchLabels
 	ds.Namespace = getRuntimeNamespace()
 	ds.Name = r.getDaemonSetName(vino)
 
-	found := false
-	for _, vol := range ds.Spec.Template.Spec.Volumes {
-		if vol.Name == "vino-spec" {
-			found = true
-			break
-		}
-	}
-	if !found {
-		ds.Spec.Template.Spec.Volumes = append(ds.Spec.Template.Spec.Volumes, corev1.Volume{
-			Name: volume,
-			VolumeSource: corev1.VolumeSource{
-				ConfigMap: &corev1.ConfigMapVolumeSource{
-					LocalObjectReference: corev1.LocalObjectReference{Name: r.getConfigMapName(vino)},
-				},
-			},
-		})
-	}
-
-	// add vino spec to each container
-	for i, c := range ds.Spec.Template.Spec.Containers {
-		found = false
-		for _, mount := range c.VolumeMounts {
-			if mount.Name == volume {
-				found = true
-			}
-		}
-		if !found {
-			logr.FromContext(ctx).Info("volume mount with vino spec is not found",
-				"vino instance", vino.Namespace+"/"+vino.Name,
-				"container name", c.Name,
-			)
-			ds.Spec.Template.Spec.Containers[i].VolumeMounts = append(
-				ds.Spec.Template.Spec.Containers[i].VolumeMounts, corev1.VolumeMount{
-					MountPath: "/vino/spec",
-					Name:      volume,
-					ReadOnly:  true,
-					SubPath:   ConfigMapKeyVinoSpec,
-				})
-		}
-	}
-
 	// TODO develop logic to derive all required ENV variables from VINO CR, and pass them
 	// to setENV function instead
 	if vino.Spec.VMBridge != "" {
@@ -508,14 +339,6 @@ func (r *VinoReconciler) finalize(ctx context.Context, vino *vinov1.Vino) error
 		}); err != nil {
 		return err
 	}
-	if err := r.Delete(ctx,
-		&corev1.ConfigMap{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: r.getConfigMapName(vino), Namespace: getRuntimeNamespace(),
-			},
-		}); err != nil {
-		return err
-	}
 	controllerutil.RemoveFinalizer(vino, vinov1.VinoFinalizer)
 	return r.Update(ctx, vino)
 }