Merge pull request #2189 from facchettos/operator-deploy-redo

operator deployment improvement

pkg/operator/deploy/deploy.go

@@ -4,15 +4,16 @@ package deploy
 // Licensed under the Apache License 2.0.
 
 import (
+	"bytes"
 	"context"
 	"embed"
 	"errors"
 	"fmt"
 	"strings"
+	"text/template"
 	"time"
 
 	"github.com/sirupsen/logrus"
-	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	extensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
 	extensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
@@ -33,7 +34,6 @@ import (
 	aroclient "github.com/Azure/ARO-RP/pkg/operator/clientset/versioned"
 	"github.com/Azure/ARO-RP/pkg/operator/controllers/genevalogging"
 	"github.com/Azure/ARO-RP/pkg/util/dynamichelper"
-	utilembed "github.com/Azure/ARO-RP/pkg/util/embed"
 	"github.com/Azure/ARO-RP/pkg/util/pullsecret"
 	"github.com/Azure/ARO-RP/pkg/util/ready"
 	"github.com/Azure/ARO-RP/pkg/util/restconfig"
@@ -83,49 +83,82 @@ func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli
 	}, nil
 }
 
-func (o *operator) staticResources() ([]kruntime.Object, error) {
-	results := []kruntime.Object{}
-	for _, fileBytes := range utilembed.ReadDirRecursive(embeddedFiles, "staticresources") {
-		obj, _, err := scheme.Codecs.UniversalDeserializer().Decode(fileBytes, nil, nil)
+type deploymentData struct {
+	Image              string
+	Version            string
+	GitCommit          string
+	IsLocalDevelopment bool
+	HasVersion         bool
+}
+
+func templateManifests(data deploymentData) ([][]byte, error) {
+	templatesRoot, err := template.ParseFS(embeddedFiles, "staticresources/*.yaml")
+	if err != nil {
+		return nil, err
+	}
+	templatesMaster, err := template.ParseFS(embeddedFiles, "staticresources/master/*")
+	if err != nil {
+		return nil, err
+	}
+	templatesWorker, err := template.ParseFS(embeddedFiles, "staticresources/worker/*")
 	if err != nil {
 		return nil, err
 	}
 
-		// set the image for the deployments
-		if d, ok := obj.(*appsv1.Deployment); ok {
-			if d.Labels == nil {
-				d.Labels = map[string]string{}
-			}
-			var image string
+	templatedFiles := make([][]byte, 0)
+	templatesArray := []*template.Template{templatesMaster, templatesRoot, templatesWorker}
 
+	for _, templates := range templatesArray {
+		for _, templ := range templates.Templates() {
+			buff := &bytes.Buffer{}
+			if err := templ.Execute(buff, data); err != nil {
+				return nil, err
+			}
+			templatedFiles = append(templatedFiles, buff.Bytes())
+		}
+	}
+	return templatedFiles, nil
+}
+
+func (o *operator) createDeploymentData() deploymentData {
+	image := o.env.AROOperatorImage()
+	hasVersion := false
 	if o.oc.Properties.OperatorVersion != "" {
-				image = fmt.Sprintf("%s/aro:%s", o.env.ACRDomain(), o.oc.Properties.OperatorVersion)
-				d.Labels["version"] = o.oc.Properties.OperatorVersion
-			} else {
-				image = o.env.AROOperatorImage()
-				d.Labels["version"] = version.GitCommit
+		image = fmt.Sprintf("%s/aro", o.env.ACRDomain())
+		hasVersion = true
 	}
 
-			for i := range d.Spec.Template.Spec.Containers {
-				d.Spec.Template.Spec.Containers[i].Image = image
+	return deploymentData{
+		IsLocalDevelopment: o.env.IsLocalDevelopmentMode(),
+		Image:              image,
+		Version:            o.oc.Properties.OperatorVersion,
+		GitCommit:          version.GitCommit,
+		HasVersion:         hasVersion,
+	}
+}
 
-				if o.env.IsLocalDevelopmentMode() {
-					d.Spec.Template.Spec.Containers[i].Env = append(d.Spec.Template.Spec.Containers[i].Env, corev1.EnvVar{
-						Name:  "RP_MODE",
-						Value: "development",
-					})
+func (o *operator) createObjects() ([]kruntime.Object, error) {
+	deploymentData := o.createDeploymentData()
+	templated, err := templateManifests(deploymentData)
+	if err != nil {
+		return nil, err
 	}
+	objects := make([]kruntime.Object, 0, len(templated))
+	for _, v := range templated {
+		obj, _, err := scheme.Codecs.UniversalDeserializer().Decode(v, nil, nil)
+		if err != nil {
+			return nil, err
 		}
+		objects = append(objects, obj)
 	}
 
-		results = append(results, obj)
-	}
-	return results, nil
+	return objects, nil
 }
 
 func (o *operator) resources() ([]kruntime.Object, error) {
 	// first static resources from Assets
-	results, err := o.staticResources()
+
+	results, err := o.createObjects()
 	if err != nil {
 		return nil, err
 	}

pkg/operator/deploy/deploy_test.go

@@ -174,7 +174,7 @@ func TestOperatorVersion(t *testing.T) {
 				env: _env,
 			}
 
-			staticResources, err := o.staticResources()
+			staticResources, err := o.createObjects()
 			if err != nil {
 				t.Error(err)
 			}
@@ -192,7 +192,7 @@ func TestOperatorVersion(t *testing.T) {
 
 			for _, d := range deployments {
 				if d.Labels["version"] != tt.wantVersion {
-					t.Errorf("Got %q, not %q", d.Labels["version"], tt.wantVersion)
+					t.Errorf("Got %q, not %q for label \"version\"", d.Labels["version"], tt.wantVersion)
 				}
 
 				if len(d.Spec.Template.Spec.Containers) != 1 {
@@ -201,7 +201,7 @@ func TestOperatorVersion(t *testing.T) {
 
 				image := d.Spec.Template.Spec.Containers[0].Image
 				if image != tt.wantPullspec {
-					t.Errorf("Got %q, not %q", image, tt.wantPullspec)
+					t.Errorf("Got %q, not %q for the image", image, tt.wantPullspec)
 				}
 			}
 		})

pkg/operator/deploy/staticresources/master/deployment.yaml.tmpl

@@ -3,6 +3,11 @@ kind: Deployment
 metadata:
   labels:
     app: aro-operator-master 
+    {{ if .HasVersion}}
+    version: {{ .Version }} 
+    {{ else }}
+    version: unknown
+    {{end}}
   name: aro-operator-master
   namespace: openshift-azure-operator
 spec:
@@ -25,8 +30,13 @@ spec:
         args:
         - operator
         - master
-        image: GENERATED
+        image: "{{ .Image }}{{ if .HasVersion}}:{{ if .IsLocalDevelopment }}{{ .GitCommit }}{{ else }}{{ .Version }}{{end}}{{end}}"
         name: aro-operator
+        {{ if .IsLocalDevelopment}}
+        env: 
+        - name: "RP_MODE"
+          value: "development" 
+        {{ end }}
         ports:
         - containerPort: 8080
           name: http

pkg/operator/deploy/staticresources/worker/deployment.yaml.tmpl

@@ -3,6 +3,11 @@ kind: Deployment
 metadata:
   labels:
     app: aro-operator-worker 
+    {{ if .HasVersion}}
+    version: {{ .Version }} 
+    {{ else }}
+    version: unknown
+    {{end}}
   name: aro-operator-worker
   namespace: openshift-azure-operator
 spec:
@@ -25,7 +30,12 @@ spec:
         args:
         - operator
         - worker
-        image: GENERATED
+        image: "{{ .Image }}{{ if .HasVersion}}:{{ if .IsLocalDevelopment }}{{ .GitCommit }}{{ else }}{{ .Version }}{{end}}{{end}}"
+        {{ if .IsLocalDevelopment}}
+        env: 
+        - name: "RP_MODE"
+          value: "development" 
+        {{ end }}
         name: aro-operator
         livenessProbe:
           httpGet: