[CDAP-19300] Added Containers Injection, added Dockerfile.test and updated the README

Author: mariogiuffrida

.gitignore

@@ -63,6 +63,7 @@ anaconda-mode/
 *.dylib
 # Test binary, build with 'go test -c'
 *.test
+!Dockerfile.test
 # Output of the go coverage tool, specifically when used with LiteIDE
 *.out
 ### Vim ###

Dockerfile.test

@@ -0,0 +1,25 @@
+# Build the manager binary
+FROM golang:1.16 as tester
+
+ENV version 1.0.8
+ENV arch amd64
+
+# Copy everything in the go src
+WORKDIR /go/src/cdap.io/cdap-operator
+COPY ./ ./
+
+# Install Kubebuilder
+RUN curl -L -O "https://github.com/kubernetes-sigs/kubebuilder/releases/download/v${version}/kubebuilder_${version}_linux_${arch}.tar.gz" && \
+    tar -zxvf kubebuilder_${version}_linux_${arch}.tar.gz && \
+    mv kubebuilder_${version}_linux_${arch} /usr/local/kubebuilder && \
+    cp /usr/local/kubebuilder/bin/* /usr/local/bin
+
+# Install setup-envtest
+RUN go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
+
+# download envtest 1.19.x for kubebuilder and to set KUBEBUILDER_ASSETS environment variable
+RUN $(go env GOPATH)/bin/setup-envtest use -p env 1.19.x > /tmp/setup_envtest.sh && \
+    eval `$(go env GOPATH)/bin/setup-envtest use -p env 1.19.x` && \
+    rm /tmp/setup_envtest.sh
+
+CMD make test

README.md

@@ -82,3 +82,14 @@ rm /tmp/setup_envtest.sh
 ```
 
 4. Run `make test`
+
+#### Running Unit Tests in a Docker image
+
+From the project root folder build the test image by running the following
+```
+docker build -f Dockerfile.test . -t test
+```
+Execute the image with
+```
+docker run test
+```

api/v1alpha1/cdapmaster_types.go

@@ -172,6 +172,10 @@ type CDAPScalableServiceSpec struct {
 	CDAPServiceSpec `json:",inline"`
 	// Replicas is number of replicas for the service.
 	Replicas *int32 `json:"replicas,omitempty"`
+	// Containers define any additional containers a service has
+	// This is a list of containers and can be left blank
+	// A typical use is to add sidecars for a deployment
+	Containers []*corev1.Container `json:"containers,omitempty"`
 }
 
 // CDAPExternalServiceSpec defines the base specification for master services that expose to outside of the cluster.
@@ -194,6 +198,10 @@ type CDAPStatefulServiceSpec struct {
 	StorageSize string `json:"storageSize,omitempty"`
 	// StorageClassName is the name of the StorageClass for the persistent volume used by the service.
 	StorageClassName *string `json:"storageClassName,omitempty"`
+	// Containers define any additional containers a service has
+	// This is a list of containers and can be left blank
+	// A typical use is to add sidecars for a stateful set
+	Containers []*corev1.Container `json:"containers,omitempty"`
 }
 
 // AppFabricSpec defines the specification for the AppFabric service.

api/v1alpha1/zz_generated.deepcopy.go

@@ -127,9 +127,43 @@ func ApplyDefaults(resource interface{}) {
 	}
 
 	// Set the configMapCConf entry for the router and UI service and ports
-	spec.Config[confRouterServerAddress] = fmt.Sprintf("cdap-%s-%s", r.Name, strings.ToLower(string(serviceRouter)))
-	spec.Config[confRouterBindPort] = strconv.Itoa(int(*spec.Router.ServicePort))
-	spec.Config[confUserInterfaceBindPort] = strconv.Itoa(int(*spec.UserInterface.ServicePort))
+	if spec.Config[confRouterServerAddress] == "" {
+		spec.Config[confRouterServerAddress] = fmt.Sprintf("cdap-%s-%s", r.Name, strings.ToLower(string(serviceRouter)))
+	}
+
+	if spec.Config[confRouterBindPort] == "" {
+		spec.Config[confRouterBindPort] = strconv.Itoa(int(*spec.Router.ServicePort))
+	}
+
+	if spec.Config[confUserInterfaceBindPort] == "" {
+		spec.Config[confUserInterfaceBindPort] = strconv.Itoa(int(*spec.UserInterface.ServicePort))
+	}
+
+	// Set the default local data directory if it is not set in cdap-cr.
+	if _, ok := spec.Config[confLocalDataDirKey]; !ok {
+		spec.Config[confLocalDataDirKey] = confLocalDataDirVal
+	}
+
+	// Set security secret disk names to be consistent with securitySecret if not overwritten.
+	if _, ok := spec.Config[confTwillSecurityMasterSecretDiskName]; !ok && spec.SecuritySecret != "" {
+		spec.Config[confTwillSecurityMasterSecretDiskName] = spec.SecuritySecret
+	}
+	if _, ok := spec.Config[confTwillSecurityMasterSecretDiskPath]; !ok && spec.SecuritySecret != "" {
+		spec.Config[confTwillSecurityMasterSecretDiskPath] = defaultSecuritySecretPath
+	}
+	// This configuration makes the default securitySecret available to the workers by default.
+	// TODO: Add support for secure-by-default configurations.
+	if _, ok := spec.Config[confTwillSecurityWorkerSecretDiskName]; !ok && spec.SecuritySecret != "" {
+		spec.Config[confTwillSecurityWorkerSecretDiskName] = spec.SecuritySecret
+	}
+	if _, ok := spec.Config[confTwillSecurityWorkerSecretDiskPath]; !ok && spec.SecuritySecret != "" {
+		spec.Config[confTwillSecurityWorkerSecretDiskPath] = defaultSecuritySecretPath
+	}
+
+	// Set the default JMX server port if not set and system metrics exporter sidecar is enabled
+	if _, ok := spec.Config[confJMXServerPort]; spec.SystemMetricsExporter != nil && !ok {
+		spec.Config[confJMXServerPort] = fmt.Sprint(defaultJMXport)
+	}
 
 	// Set the default local data directory if it is not set in cdap-cr.
 	if _, ok := spec.Config[confLocalDataDirKey]; !ok {

controllers/cdapmaster_controller.go

@@ -177,33 +177,47 @@ func buildStatefulSets(master *v1alpha1.CDAPMaster, name string, services Servic
 
 	// Add each service as a container
 	for _, s := range services {
-		ss, err := getCDAPServiceSpec(master, s)
+		statefulSet, err := getCDAPStatefulServiceSpec(master, s)
 		if err != nil {
 			return nil, err
 		}
 		// This happens when the service is optional and disabled in CR
 		// (i.e. service spec is set to nil)
-		if ss == nil {
+		if statefulSet == nil {
 			continue
 		}
 
-		c, err := serviceContainerSpec(ss, master, dataDir, s)
+		c, err := serviceContainerSpec(&statefulSet.CDAPServiceSpec, master, dataDir, s)
 		if err != nil {
 			return nil, err
 		}
 		spec = spec.withContainer(c)
-		if err := addSystemMetricsServiceIfEnabled(spec, master, ss, dataDir, c); err != nil {
+
+		if statefulSet.Containers != nil {
+			for _, container := range statefulSet.Containers {
+				additionalContainer := containerSpecFromContainer(container, dataDir)
+				spec = spec.withContainer(additionalContainer)
+			}
+		}
+
+		if err := addSystemMetricsServiceIfEnabled(spec, master, &statefulSet.CDAPServiceSpec, dataDir, c); err != nil {
 			return nil, err
 		}
 
 		// Adding a label to allow NodePort service selector to find the pod
 		spec = spec.addLabel(labelContainerKeyPrefix+s, master.Name)
 
 		// Mount extra volumes from ConfigMap and Secret
-		if _, err := spec.addConfigMapVolumes(ss.ConfigMapVolumes); err != nil {
+		if _, err := spec.addConfigMapVolumes(statefulSet.CDAPServiceSpec.ConfigMapVolumes); err != nil {
+			return nil, err
+		}
+		if _, err := spec.addSecretVolumes(statefulSet.CDAPServiceSpec.SecretVolumes); err != nil {
+			return nil, err
+		}
+		if _, err := spec.addAdditionalVolumes(statefulSet.CDAPServiceSpec.AdditionalVolumes); err != nil {
 			return nil, err
 		}
-		if _, err := spec.addSecretVolumes(ss.SecretVolumes); err != nil {
+		if _, err := spec.addAdditionalVolumeMounts(statefulSet.CDAPServiceSpec.AdditionalVolumeMounts); err != nil {
 			return nil, err
 		}
 		if _, err := spec.addAdditionalVolumes(ss.AdditionalVolumes); err != nil {
@@ -313,30 +327,43 @@ func buildDeployment(master *v1alpha1.CDAPMaster, name string, services ServiceG
 
 	// Add each service as a container
 	for _, s := range services {
-		ss, err := getCDAPServiceSpec(master, s)
+		scalableSpec, err := getCDAPScalableServiceSpec(master, s)
 		if err != nil {
 			return nil, err
 		}
 		// This happens when the service is optional and disabled in CR
 		// (i.e. service spec is set to nil)
-		if ss == nil {
+		if scalableSpec == nil {
 			continue
 		}
 
-		c, err := serviceContainerSpec(ss, master, dataDir, s)
+		c, err := serviceContainerSpec(&scalableSpec.CDAPServiceSpec, master, dataDir, s)
 		if err != nil {
 			return nil, err
 		}
 		spec = spec.withContainer(c)
 
+		if scalableSpec.Containers != nil {
+			for _, container := range scalableSpec.Containers {
+				additionalContainer := containerSpecFromContainer(container, dataDir)
+				spec = spec.withContainer(additionalContainer)
+			}
+		}
+
 		// Adding a label to allow k8s service selector to easily find the pod
 		spec = spec.addLabel(labelContainerKeyPrefix+s, master.Name)
 
 		// Mount extra volumes from ConfigMap and Secret
-		if _, err := spec.addConfigMapVolumes(ss.ConfigMapVolumes); err != nil {
+		if _, err := spec.addConfigMapVolumes(scalableSpec.CDAPServiceSpec.ConfigMapVolumes); err != nil {
+			return nil, err
+		}
+		if _, err := spec.addSecretVolumes(scalableSpec.CDAPServiceSpec.SecretVolumes); err != nil {
+			return nil, err
+		}
+		if _, err := spec.addAdditionalVolumes(scalableSpec.CDAPServiceSpec.AdditionalVolumes); err != nil {
 			return nil, err
 		}
-		if _, err := spec.addSecretVolumes(ss.SecretVolumes); err != nil {
+		if _, err := spec.addAdditionalVolumeMounts(scalableSpec.CDAPServiceSpec.AdditionalVolumeMounts); err != nil {
 			return nil, err
 		}
 		if _, err := spec.addAdditionalVolumes(ss.AdditionalVolumes); err != nil {

controllers/deployment.go

@@ -256,6 +256,131 @@ var _ = Describe("Controller Suite", func() {
 			}
 		})
 	})
+
+	Describe("Add additional generic container", func() {
+		readMaster := func(fileName string) *v1alpha1.CDAPMaster {
+			master := &v1alpha1.CDAPMaster{}
+			err := fromJson(fileName, master)
+			Expect(err).To(BeNil())
+			return master
+		}
+		readExpectedJson := func(fileName string) []byte {
+			json, err := ioutil.ReadFile("testdata/" + fileName)
+			Expect(err).To(BeNil())
+			return json
+		}
+		diffJson := func(expected, actual []byte) {
+			opts := jsondiff.DefaultConsoleOptions()
+			diff, text := jsondiff.Compare(expected, actual, &opts)
+			Expect(diff.String()).To(Equal(jsondiff.SupersetMatch.String()), text)
+		}
+		diffAdditionalContainers := func(containers []corev1.Container, containerName, expectedJsonFilename string) {
+			var additionalContainer *corev1.Container
+			for _, c := range containers {
+				if c.Name == containerName {
+					additionalContainer = &c
+					break
+				}
+			}
+
+			actualContainerJson, _ := json.Marshal(additionalContainer)
+			expectedContainerJson := readExpectedJson(expectedJsonFilename)
+			diffJson(actualContainerJson, expectedContainerJson)
+		}
+		It("Multiple Additional containers for Router", func() {
+			master := readMaster("testdata/cdap_master_cr_multi_additional_containers.json")
+			emptyLabels := make(map[string]string)
+			spec, err := buildDeploymentPlanSpec(master, emptyLabels)
+			Expect(err).To(BeNil())
+			objs, err := buildObjectsForDeploymentPlan(spec)
+			Expect(err).To(BeNil())
+
+			var strategyHandler DeploymentPlan
+			strategyHandler.Init()
+
+			for _, obj := range objs {
+				if o, ok := obj.Obj.(*k8s.Object).Obj.(*appsv1.Deployment); ok {
+
+					if o.Name == getObjName(master, "router") {
+						containers := o.Spec.Template.Spec.Containers
+
+						Expect(len(containers)).To(BeIdenticalTo(3))
+
+						diffAdditionalContainers(containers, "test-router-container-a", "additional_router_container_a.json")
+						diffAdditionalContainers(containers, "test-router-container-b", "additional_router_container_b.json")
+					}
+				}
+			}
+		})
+		It("Multiple Additional container for AppFabric", func() {
+			master := readMaster("testdata/cdap_master_cr_multi_additional_containers.json")
+			emptyLabels := make(map[string]string)
+			spec, err := buildDeploymentPlanSpec(master, emptyLabels)
+			Expect(err).To(BeNil())
+			objs, err := buildObjectsForDeploymentPlan(spec)
+			Expect(err).To(BeNil())
+
+			var strategyHandler DeploymentPlan
+			strategyHandler.Init()
+
+			for _, obj := range objs {
+				if o, ok := obj.Obj.(*k8s.Object).Obj.(*appsv1.StatefulSet); ok {
+					if o.Name == getObjName(master, "appFabric") {
+						containers := o.Spec.Template.Spec.Containers
+
+						Expect(len(containers)).To(BeIdenticalTo(3))
+
+						diffAdditionalContainers(containers, "test-appfabric-container-a", "additional_appfabric_container_a.json")
+						diffAdditionalContainers(containers, "test-appfabric-container-b", "additional_appfabric_container_b.json")
+					}
+				}
+			}
+		})
+		It("No Additional container for Router", func() {
+			master := readMaster("testdata/cdap_master_cr.json")
+			emptyLabels := make(map[string]string)
+			spec, err := buildDeploymentPlanSpec(master, emptyLabels)
+			Expect(err).To(BeNil())
+			objs, err := buildObjectsForDeploymentPlan(spec)
+			Expect(err).To(BeNil())
+
+			var strategyHandler DeploymentPlan
+			strategyHandler.Init()
+
+			for _, obj := range objs {
+				if o, ok := obj.Obj.(*k8s.Object).Obj.(*appsv1.Deployment); ok {
+					if o.Name == getObjName(master, "router") {
+						containers := o.Spec.Template.Spec.Containers
+
+						Expect(len(containers)).To(BeIdenticalTo(1))
+						Expect(containers[0].Name).To(BeIdenticalTo("router"))
+					}
+				}
+			}
+		})
+		It("No Additional container for AppFabric", func() {
+			master := readMaster("testdata/cdap_master_cr.json")
+			emptyLabels := make(map[string]string)
+			spec, err := buildDeploymentPlanSpec(master, emptyLabels)
+			Expect(err).To(BeNil())
+			objs, err := buildObjectsForDeploymentPlan(spec)
+			Expect(err).To(BeNil())
+
+			var strategyHandler DeploymentPlan
+			strategyHandler.Init()
+
+			for _, obj := range objs {
+				if o, ok := obj.Obj.(*k8s.Object).Obj.(*appsv1.StatefulSet); ok {
+					if o.Name == getObjName(master, "appFabric") {
+						containers := o.Spec.Template.Spec.Containers
+
+						Expect(len(containers)).To(BeIdenticalTo(1))
+						Expect(containers[0].Name).To(BeIdenticalTo("appfabric"))
+					}
+				}
+			}
+		})
+	})
 })
 
 func TestMergeEnvVars(t *testing.T) {