Refactor common spec elements into a single struct with common proces… (

#105) * Refactor common spec elements into a single struct with common processing (merging) Signed-off-by: Gary Brown <gary@brownuk.com> * Fix generate issue Signed-off-by: Gary Brown <gary@brownuk.com> * Fix test coverage Signed-off-by: Gary Brown <gary@brownuk.com> * Fix based on review comment Signed-off-by: Gary Brown <gary@brownuk.com>
jaegertracing · Nov 9, 2018 · 06ceca0 · 06ceca0
1 parent 12edd3a
commit 06ceca0
Show file tree

Hide file tree

Showing 8 changed files with 221 additions and 183 deletions.
diff --git a/pkg/apis/io/v1alpha1/types.go b/pkg/apis/io/v1alpha1/types.go
@@ -34,16 +34,21 @@ type Jaeger struct {
 
 // JaegerSpec defines the structure of the Jaeger JSON object from the CR
 type JaegerSpec struct {
-	Strategy     string              `json:"strategy"`
-	AllInOne     JaegerAllInOneSpec  `json:"allInOne"`
-	Query        JaegerQuerySpec     `json:"query"`
-	Collector    JaegerCollectorSpec `json:"collector"`
-	Agent        JaegerAgentSpec     `json:"agent"`
-	Storage      JaegerStorageSpec   `json:"storage"`
-	Ingress      JaegerIngressSpec   `json:"ingress"`
-	Volumes      []v1.Volume         `json:"volumes"`
-	VolumeMounts []v1.VolumeMount    `json:"volumeMounts"`
-	Annotations  map[string]string   `json:"annotations,omitempty"`
+	Strategy  string              `json:"strategy"`
+	AllInOne  JaegerAllInOneSpec  `json:"allInOne"`
+	Query     JaegerQuerySpec     `json:"query"`
+	Collector JaegerCollectorSpec `json:"collector"`
+	Agent     JaegerAgentSpec     `json:"agent"`
+	Storage   JaegerStorageSpec   `json:"storage"`
+	Ingress   JaegerIngressSpec   `json:"ingress"`
+	JaegerCommonSpec
+}
+
+// JaegerCommonSpec defines the common elements used in multiple other spec structs
+type JaegerCommonSpec struct {
+	Volumes      []v1.Volume       `json:"volumes"`
+	VolumeMounts []v1.VolumeMount  `json:"volumeMounts"`
+	Annotations  map[string]string `json:"annotations,omitempty"`
 }
 
 // JaegerStatus defines what is to be returned from a status query
@@ -53,12 +58,10 @@ type JaegerStatus struct {
 
 // JaegerQuerySpec defines the options to be used when deploying the query
 type JaegerQuerySpec struct {
-	Size         int               `json:"size"`
-	Image        string            `json:"image"`
-	Options      Options           `json:"options"`
-	Volumes      []v1.Volume       `json:"volumes"`
-	VolumeMounts []v1.VolumeMount  `json:"volumeMounts"`
-	Annotations  map[string]string `json:"annotations,omitempty"`
+	Size    int     `json:"size"`
+	Image   string  `json:"image"`
+	Options Options `json:"options"`
+	JaegerCommonSpec
 }
 
 // JaegerIngressSpec defines the options to be used when deploying the query ingress
@@ -68,31 +71,25 @@ type JaegerIngressSpec struct {
 
 // JaegerAllInOneSpec defines the options to be used when deploying the query
 type JaegerAllInOneSpec struct {
-	Image        string            `json:"image"`
-	Options      Options           `json:"options"`
-	Volumes      []v1.Volume       `json:"volumes"`
-	VolumeMounts []v1.VolumeMount  `json:"volumeMounts"`
-	Annotations  map[string]string `json:"annotations,omitempty"`
+	Image   string  `json:"image"`
+	Options Options `json:"options"`
+	JaegerCommonSpec
 }
 
 // JaegerCollectorSpec defines the options to be used when deploying the collector
 type JaegerCollectorSpec struct {
-	Size         int               `json:"size"`
-	Image        string            `json:"image"`
-	Options      Options           `json:"options"`
-	Volumes      []v1.Volume       `json:"volumes"`
-	VolumeMounts []v1.VolumeMount  `json:"volumeMounts"`
-	Annotations  map[string]string `json:"annotations,omitempty"`
+	Size    int     `json:"size"`
+	Image   string  `json:"image"`
+	Options Options `json:"options"`
+	JaegerCommonSpec
 }
 
 // JaegerAgentSpec defines the options to be used when deploying the agent
 type JaegerAgentSpec struct {
-	Strategy     string            `json:"strategy"` // can be either 'DaemonSet' or 'Sidecar' (default)
-	Image        string            `json:"image"`
-	Options      Options           `json:"options"`
-	Volumes      []v1.Volume       `json:"volumes"`
-	VolumeMounts []v1.VolumeMount  `json:"volumeMounts"`
-	Annotations  map[string]string `json:"annotations,omitempty"`
+	Strategy string  `json:"strategy"` // can be either 'DaemonSet' or 'Sidecar' (default)
+	Image    string  `json:"image"`
+	Options  Options `json:"options"`
+	JaegerCommonSpec
 }
 
 // JaegerStorageSpec defines the common storage options to be used for the query and collector

diff --git a/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/deployment/agent.go b/pkg/deployment/agent.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
 	"github.com/jaegertracing/jaeger-operator/pkg/service"
+	"github.com/jaegertracing/jaeger-operator/pkg/util"
 )
 
 // Agent builds pods for jaegertracing/jaeger-agent
@@ -42,18 +43,17 @@ func (a *Agent) Get() *appsv1.DaemonSet {
 	args := append(a.jaeger.Spec.Agent.Options.ToArgs(), fmt.Sprintf("--collector.host-port=%s:14267", service.GetNameForCollectorService(a.jaeger)))
 	trueVar := true
 	selector := a.selector()
-	annotations := map[string]string{
-		"prometheus.io/scrape":    "true",
-		"prometheus.io/port":      "5778",
-		"sidecar.istio.io/inject": "false",
-	}
-	for k, v := range a.jaeger.Spec.Annotations {
-		annotations[k] = v
-	}
-	for k, v := range a.jaeger.Spec.Agent.Annotations {
-		annotations[k] = v
+
+	baseCommonSpec := v1alpha1.JaegerCommonSpec{
+		Annotations: map[string]string{
+			"prometheus.io/scrape":    "true",
+			"prometheus.io/port":      "5778",
+			"sidecar.istio.io/inject": "false",
+		},
 	}
 
+	commonSpec := util.Merge([]v1alpha1.JaegerCommonSpec{a.jaeger.Spec.Agent.JaegerCommonSpec, a.jaeger.Spec.JaegerCommonSpec, baseCommonSpec})
+
 	return &appsv1.DaemonSet{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apps/v1",
@@ -79,7 +79,7 @@ func (a *Agent) Get() *appsv1.DaemonSet {
 			Template: v1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:      selector,
-					Annotations: annotations,
+					Annotations: commonSpec.Annotations,
 				},
 				Spec: v1.PodSpec{
 					Containers: []v1.Container{{

diff --git a/pkg/deployment/all-in-one.go b/pkg/deployment/all-in-one.go
@@ -35,18 +35,17 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 	logrus.Debug("Assembling an all-in-one deployment")
 	selector := a.selector()
 	trueVar := true
-	annotations := map[string]string{
-		"prometheus.io/scrape":    "true",
-		"prometheus.io/port":      "16686",
-		"sidecar.istio.io/inject": "false",
-	}
-	for k, v := range a.jaeger.Spec.Annotations {
-		annotations[k] = v
-	}
-	for k, v := range a.jaeger.Spec.AllInOne.Annotations {
-		annotations[k] = v
+
+	baseCommonSpec := v1alpha1.JaegerCommonSpec{
+		Annotations: map[string]string{
+			"prometheus.io/scrape":    "true",
+			"prometheus.io/port":      "16686",
+			"sidecar.istio.io/inject": "false",
+		},
 	}
 
+	commonSpec := util.Merge([]v1alpha1.JaegerCommonSpec{a.jaeger.Spec.AllInOne.JaegerCommonSpec, a.jaeger.Spec.JaegerCommonSpec, baseCommonSpec})
+
 	return &appsv1.Deployment{
 		TypeMeta: metav1.TypeMeta{
 			APIVersion: "apps/v1",
@@ -72,7 +71,7 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 			Template: v1.PodTemplateSpec{
 				ObjectMeta: metav1.ObjectMeta{
 					Labels:      selector,
-					Annotations: annotations,
+					Annotations: commonSpec.Annotations,
 				},
 				Spec: v1.PodSpec{
 					Containers: []v1.Container{{
@@ -90,7 +89,7 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 								Value: "9411",
 							},
 						},
-						VolumeMounts: util.RemoveDuplicatedVolumeMounts(append(a.jaeger.Spec.AllInOne.VolumeMounts, a.jaeger.Spec.VolumeMounts...)),
+						VolumeMounts: commonSpec.VolumeMounts,
 						Ports: []v1.ContainerPort{
 							{
 								ContainerPort: 5775,
@@ -138,7 +137,7 @@ func (a *AllInOne) Get() *appsv1.Deployment {
 							InitialDelaySeconds: 1,
 						},
 					}},
-					Volumes: util.RemoveDuplicatedVolumes(append(a.jaeger.Spec.AllInOne.Volumes, a.jaeger.Spec.Volumes...)),
+					Volumes: commonSpec.Volumes,
 				},
 			},
 		},