Factor out ingress from all-in-one and query, as common to both but i… (

#91)

* Factor out ingress from all-in-one and query, as common to both but is independent of either deployment strategy

Signed-off-by: Gary Brown <[email protected]>

* Update docs

Signed-off-by: Gary Brown <[email protected]>

deploy/examples/disable-ingress.yaml

@@ -3,6 +3,5 @@ kind: Jaeger
 metadata:
   name: disable-ingress
 spec:
-  all-in-one:
-    ingress:
-      enabled: false
+  ingress:
+    enabled: false

pkg/apis/io/v1alpha1/types.go

@@ -31,6 +31,7 @@ type JaegerSpec struct {
 	Collector JaegerCollectorSpec `json:"collector"`
 	Agent     JaegerAgentSpec     `json:"agent"`
 	Storage   JaegerStorageSpec   `json:"storage"`
+	Ingress   JaegerIngressSpec   `json:"ingress"`
 }
 
 // JaegerStatus defines what is to be returned from a status query
@@ -40,7 +41,6 @@ type JaegerStatus struct {
 
 // JaegerQuerySpec defines the options to be used when deploying the query
 type JaegerQuerySpec struct {
-	Ingress     JaegerIngressSpec `json:"ingress"`
 	Size        int               `json:"size"`
 	Image       string            `json:"image"`
 	Options     Options           `json:"options"`
@@ -54,7 +54,6 @@ type JaegerIngressSpec struct {
 
 // JaegerAllInOneSpec defines the options to be used when deploying the query
 type JaegerAllInOneSpec struct {
-	Ingress     JaegerIngressSpec `json:"ingress"`
 	Image       string            `json:"image"`
 	Options     Options           `json:"options"`
 	Annotations map[string]string `json:"annotations,omitempty"`

pkg/controller/all-in-one.go

@@ -3,6 +3,8 @@ package controller
 import (
 	"context"
 
+	"github.com/jaegertracing/jaeger-operator/pkg/ingress"
+
 	"github.com/operator-framework/operator-sdk/pkg/sdk"
 	"github.com/sirupsen/logrus"
 	batchv1 "k8s.io/api/batch/v1"
@@ -38,8 +40,10 @@ func (c *allInOneController) Create() []sdk.Object {
 	for _, svc := range dep.Services() {
 		os = append(os, svc)
 	}
-	for _, ingress := range dep.Ingresses() {
-		os = append(os, ingress)
+
+	qi := ingress.NewQueryIngress(c.jaeger).Get()
+	if nil != qi {
+		os = append(os, qi)
 	}
 
 	return os

pkg/controller/production.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
 	"github.com/jaegertracing/jaeger-operator/pkg/deployment"
+	"github.com/jaegertracing/jaeger-operator/pkg/ingress"
 	"github.com/jaegertracing/jaeger-operator/pkg/storage"
 )
 
@@ -47,8 +48,9 @@ func (c *productionController) Create() []sdk.Object {
 		components = append(components, svc)
 	}
 
-	for _, ingress := range query.Ingresses() {
-		components = append(components, ingress)
+	qi := ingress.NewQueryIngress(c.jaeger).Get()
+	if nil != qi {
+		components = append(components, qi)
 	}
 
 	return components

pkg/deployment/all-in-one.go

@@ -7,12 +7,10 @@ import (
 	"github.com/spf13/viper"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/api/core/v1"
-	"k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
-	"github.com/jaegertracing/jaeger-operator/pkg/ingress"
 	"github.com/jaegertracing/jaeger-operator/pkg/service"
 )
 
@@ -149,17 +147,6 @@ func (a *AllInOne) Services() []*v1.Service {
 	}
 }
 
-// Ingresses returns a list of ingress rules to be deployed along with the all-in-one deployment
-func (a *AllInOne) Ingresses() []*v1beta1.Ingress {
-	if a.jaeger.Spec.AllInOne.Ingress.Enabled == nil || *a.jaeger.Spec.AllInOne.Ingress.Enabled == true {
-		return []*v1beta1.Ingress{
-			ingress.NewQueryIngress(a.jaeger),
-		}
-	}
-
-	return []*v1beta1.Ingress{}
-}
-
 func (a *AllInOne) selector() map[string]string {
 	return map[string]string{"app": "jaeger", "jaeger": a.jaeger.Name}
 }

pkg/deployment/all-in-one_test.go

@@ -69,41 +69,3 @@ func TestAllInOneNumberOfServices(t *testing.T) {
 		assert.Equal(t, name, owners[0].Name)
 	}
 }
-
-func TestAllInOneNumberOfIngresses(t *testing.T) {
-	name := "TestAllInOneNumberOfIngresses"
-	newBool := func(value bool) *bool {
-		return &value
-	}
-
-	subTestCases := []struct {
-		name                   string
-		ingressSpec            v1alpha1.JaegerIngressSpec
-		expectedIngressesCount int
-	}{
-		{
-			name:                   "IngressEnabledDefault",
-			ingressSpec:            v1alpha1.JaegerIngressSpec{},
-			expectedIngressesCount: 1,
-		},
-		{
-			name:                   "IngressEnabledFalse",
-			ingressSpec:            v1alpha1.JaegerIngressSpec{Enabled: newBool(false)},
-			expectedIngressesCount: 0,
-		},
-		{
-			name:                   "IngressEnabledTrue",
-			ingressSpec:            v1alpha1.JaegerIngressSpec{Enabled: newBool(true)},
-			expectedIngressesCount: 1,
-		},
-	}
-
-	for _, stc := range subTestCases {
-		t.Run(stc.name, func(t *testing.T) {
-			jaeger := v1alpha1.NewJaeger(name)
-			jaeger.Spec.AllInOne.Ingress = stc.ingressSpec
-			ingresses := NewAllInOne(jaeger).Ingresses()
-			assert.Len(t, ingresses, stc.expectedIngressesCount)
-		})
-	}
-}

pkg/deployment/query.go

@@ -7,12 +7,10 @@ import (
 	"github.com/spf13/viper"
 	appsv1 "k8s.io/api/apps/v1"
 	"k8s.io/api/core/v1"
-	"k8s.io/api/extensions/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 
 	"github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1"
-	"github.com/jaegertracing/jaeger-operator/pkg/ingress"
 	"github.com/jaegertracing/jaeger-operator/pkg/service"
 )
 
@@ -126,17 +124,6 @@ func (q *Query) Services() []*v1.Service {
 	}
 }
 
-// Ingresses returns a list of ingress rules to be deployed along with the all-in-one deployment
-func (q *Query) Ingresses() []*v1beta1.Ingress {
-	if q.jaeger.Spec.Query.Ingress.Enabled == nil || *q.jaeger.Spec.Query.Ingress.Enabled == true {
-		return []*v1beta1.Ingress{
-			ingress.NewQueryIngress(q.jaeger),
-		}
-	}
-
-	return []*v1beta1.Ingress{}
-}
-
 func (q *Query) selector() map[string]string {
 	return map[string]string{"app": "jaeger", "jaeger": q.jaeger.Name, "jaeger-component": "query"}
 }

pkg/deployment/query_test.go

@@ -75,41 +75,3 @@ func TestQueryServices(t *testing.T) {
 
 	assert.Len(t, svcs, 1)
 }
-
-func TestQueryIngresses(t *testing.T) {
-	newBool := func(value bool) *bool {
-		return &value
-	}
-
-	subTestCases := []struct {
-		name                   string
-		ingressSpec            v1alpha1.JaegerIngressSpec
-		expectedIngressesCount int
-	}{
-		{
-			name:                   "IngressEnabledDefault",
-			ingressSpec:            v1alpha1.JaegerIngressSpec{},
-			expectedIngressesCount: 1,
-		},
-		{
-			name:                   "IngressEnabledFalse",
-			ingressSpec:            v1alpha1.JaegerIngressSpec{Enabled: newBool(false)},
-			expectedIngressesCount: 0,
-		},
-		{
-			name:                   "IngressEnabledTrue",
-			ingressSpec:            v1alpha1.JaegerIngressSpec{Enabled: newBool(true)},
-			expectedIngressesCount: 1,
-		},
-	}
-
-	for _, stc := range subTestCases {
-		t.Run(stc.name, func(t *testing.T) {
-			query := NewQuery(v1alpha1.NewJaeger("TestQueryIngresses"))
-			query.jaeger.Spec.Query.Ingress = stc.ingressSpec
-			ingresses := query.Ingresses()
-
-			assert.Len(t, ingresses, stc.expectedIngressesCount)
-		})
-	}
-}

pkg/ingress/query.go

@@ -11,8 +11,22 @@ import (
 	"github.com/jaegertracing/jaeger-operator/pkg/service"
 )
 
-// NewQueryIngress returns a new ingress object for the Query service
-func NewQueryIngress(jaeger *v1alpha1.Jaeger) *v1beta1.Ingress {
+// QueryIngress builds pods for jaegertracing/jaeger-query
+type QueryIngress struct {
+	jaeger *v1alpha1.Jaeger
+}
+
+// NewQueryIngress builds a new QueryIngress struct based on the given spec
+func NewQueryIngress(jaeger *v1alpha1.Jaeger) *QueryIngress {
+	return &QueryIngress{jaeger: jaeger}
+}
+
+// Get returns an ingress specification for the current instance
+func (i *QueryIngress) Get() *v1beta1.Ingress {
+	if i.jaeger.Spec.Ingress.Enabled != nil && *i.jaeger.Spec.Ingress.Enabled == false {
+		return nil
+	}
+
 	trueVar := true
 
 	return &v1beta1.Ingress{
@@ -21,22 +35,22 @@ func NewQueryIngress(jaeger *v1alpha1.Jaeger) *v1beta1.Ingress {
 			APIVersion: "extensions/v1beta1",
 		},
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      fmt.Sprintf("%s-query", jaeger.Name),
-			Namespace: jaeger.Namespace,
+			Name:      fmt.Sprintf("%s-query", i.jaeger.Name),
+			Namespace: i.jaeger.Namespace,
 			OwnerReferences: []metav1.OwnerReference{
 				metav1.OwnerReference{
-					APIVersion: jaeger.APIVersion,
-					Kind:       jaeger.Kind,
-					Name:       jaeger.Name,
-					UID:        jaeger.UID,
+					APIVersion: i.jaeger.APIVersion,
+					Kind:       i.jaeger.Kind,
+					Name:       i.jaeger.Name,
+					UID:        i.jaeger.UID,
 					Controller: &trueVar,
 				},
 			},
 		},
 		Spec: v1beta1.IngressSpec{
 			Backend: &v1beta1.IngressBackend{
-				ServiceName: service.GetNameForQueryService(jaeger),
-				ServicePort: intstr.FromInt(service.GetPortForQueryService(jaeger)),
+				ServiceName: service.GetNameForQueryService(i.jaeger),
+				ServicePort: intstr.FromInt(service.GetPortForQueryService(i.jaeger)),
 			},
 		},
 	}

pkg/ingress/query_test.go

@@ -1,6 +1,7 @@
 package ingress
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -9,7 +10,35 @@ import (
 )
 
 func TestQueryIngress(t *testing.T) {
-	jaeger := v1alpha1.NewJaeger("TestQueryIngress")
+	name := "TestQueryIngress"
+	jaeger := v1alpha1.NewJaeger(name)
 	ingress := NewQueryIngress(jaeger)
-	assert.Contains(t, ingress.Spec.Backend.ServiceName, "query")
+
+	dep := ingress.Get()
+
+	assert.Contains(t, dep.Spec.Backend.ServiceName, fmt.Sprintf("%s-query", name))
+}
+
+func TestQueryIngressDisabled(t *testing.T) {
+	enabled := false
+	name := "TestQueryIngressDisabled"
+	jaeger := v1alpha1.NewJaeger(name)
+	jaeger.Spec.Ingress.Enabled = &enabled
+	ingress := NewQueryIngress(jaeger)
+
+	dep := ingress.Get()
+
+	assert.Nil(t, dep)
+}
+
+func TestQueryIngressEnabled(t *testing.T) {
+	enabled := true
+	name := "TestQueryIngressEnabled"
+	jaeger := v1alpha1.NewJaeger(name)
+	jaeger.Spec.Ingress.Enabled = &enabled
+	ingress := NewQueryIngress(jaeger)
+
+	dep := ingress.Get()
+
+	assert.NotNil(t, dep)
 }