Merge "Move vino Spec ConfigMap to an annotation"

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)
 }