From 2fdcf96d0da8fa782e7eef82cdc91b9f16d56163 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Sat, 10 Nov 2018 12:28:21 +0000 Subject: [PATCH 1/7] Query base path should be used to configure correct path in ingress Signed-off-by: Gary Brown --- deploy/examples/all-in-one-with-options.yaml | 7 +++- pkg/ingress/query.go | 39 +++++++++++++++++--- pkg/ingress/query_test.go | 39 ++++++++++++++++++++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/deploy/examples/all-in-one-with-options.yaml b/deploy/examples/all-in-one-with-options.yaml index c0897a80a..83481eb72 100644 --- a/deploy/examples/all-in-one-with-options.yaml +++ b/deploy/examples/all-in-one-with-options.yaml @@ -8,4 +8,9 @@ spec: image: jaegertracing/all-in-one:1.7 options: log-level: debug - memory.max-traces: 100000 + query: + base-path: /jaeger + storage: + options: + memory: + max-traces: 100000 diff --git a/pkg/ingress/query.go b/pkg/ingress/query.go index 866cbf803..e342b2f13 100644 --- a/pkg/ingress/query.go +++ b/pkg/ingress/query.go @@ -2,6 +2,7 @@ package ingress import ( "fmt" + "strings" "k8s.io/api/extensions/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -29,6 +30,37 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { trueVar := true + spec := v1beta1.IngressSpec{} + backend := &v1beta1.IngressBackend{ + ServiceName: service.GetNameForQueryService(i.jaeger), + ServicePort: intstr.FromInt(service.GetPortForQueryService(i.jaeger)), + } + if _, ok := i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"]; ok && strings.ToLower(i.jaeger.Spec.Strategy) == "allinone" { + rule := &v1beta1.IngressRule{} + rule.HTTP = &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + v1beta1.HTTPIngressPath{ + Path: i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"], + Backend: *backend, + }, + }, + } + spec.Rules = append(spec.Rules, *rule) + } else if _, ok := i.jaeger.Spec.Query.Options.Map()["query.base-path"]; ok && strings.ToLower(i.jaeger.Spec.Strategy) == "production" { + rule := &v1beta1.IngressRule{} + rule.HTTP = &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + v1beta1.HTTPIngressPath{ + Path: i.jaeger.Spec.Query.Options.Map()["query.base-path"], + Backend: *backend, + }, + }, + } + spec.Rules = append(spec.Rules, *rule) + } else { + spec.Backend = backend + } + return &v1beta1.Ingress{ TypeMeta: metav1.TypeMeta{ Kind: "Ingress", @@ -47,11 +79,6 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { }, }, }, - Spec: v1beta1.IngressSpec{ - Backend: &v1beta1.IngressBackend{ - ServiceName: service.GetNameForQueryService(i.jaeger), - ServicePort: intstr.FromInt(service.GetPortForQueryService(i.jaeger)), - }, - }, + Spec: spec, } } diff --git a/pkg/ingress/query_test.go b/pkg/ingress/query_test.go index 9dcee912b..106e267a7 100644 --- a/pkg/ingress/query_test.go +++ b/pkg/ingress/query_test.go @@ -41,4 +41,43 @@ func TestQueryIngressEnabled(t *testing.T) { dep := ingress.Get() assert.NotNil(t, dep) + assert.NotNil(t, dep.Spec.Backend) +} + +func TestQueryIngressAllInOneBasePath(t *testing.T) { + enabled := true + name := "TestQueryIngressAllInOneBasePath" + jaeger := v1alpha1.NewJaeger(name) + jaeger.Spec.Ingress.Enabled = &enabled + jaeger.Spec.Strategy = "allInOne" + jaeger.Spec.AllInOne.Options = v1alpha1.NewOptions(map[string]interface{}{"query.base-path": "/jaeger"}) + ingress := NewQueryIngress(jaeger) + + dep := ingress.Get() + + assert.NotNil(t, dep) + assert.Nil(t, dep.Spec.Backend) + assert.Len(t, dep.Spec.Rules, 1) + assert.Len(t, dep.Spec.Rules[0].HTTP.Paths, 1) + assert.Equal(t, "/jaeger", dep.Spec.Rules[0].HTTP.Paths[0].Path) + assert.NotNil(t, dep.Spec.Rules[0].HTTP.Paths[0].Backend) +} + +func TestQueryIngressQueryBasePath(t *testing.T) { + enabled := true + name := "TestQueryIngressQueryBasePath" + jaeger := v1alpha1.NewJaeger(name) + jaeger.Spec.Ingress.Enabled = &enabled + jaeger.Spec.Strategy = "production" + jaeger.Spec.Query.Options = v1alpha1.NewOptions(map[string]interface{}{"query.base-path": "/jaeger"}) + ingress := NewQueryIngress(jaeger) + + dep := ingress.Get() + + assert.NotNil(t, dep) + assert.Nil(t, dep.Spec.Backend) + assert.Len(t, dep.Spec.Rules, 1) + assert.Len(t, dep.Spec.Rules[0].HTTP.Paths, 1) + assert.Equal(t, "/jaeger", dep.Spec.Rules[0].HTTP.Paths[0].Path) + assert.NotNil(t, dep.Spec.Rules[0].HTTP.Paths[0].Backend) } From 7daabecfb23e0751cb1982c2887a66c227ff194b Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 11:52:15 +0000 Subject: [PATCH 2/7] Update following review comment and added initial e2e test to check ingress uses existing URI without base path Signed-off-by: Gary Brown --- pkg/ingress/query.go | 35 +++++++--------- test/e2e/all_in_one_test.go | 80 +++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 20 deletions(-) diff --git a/pkg/ingress/query.go b/pkg/ingress/query.go index e342b2f13..27a7f9201 100644 --- a/pkg/ingress/query.go +++ b/pkg/ingress/query.go @@ -36,27 +36,9 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { ServicePort: intstr.FromInt(service.GetPortForQueryService(i.jaeger)), } if _, ok := i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"]; ok && strings.ToLower(i.jaeger.Spec.Strategy) == "allinone" { - rule := &v1beta1.IngressRule{} - rule.HTTP = &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ - v1beta1.HTTPIngressPath{ - Path: i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"], - Backend: *backend, - }, - }, - } - spec.Rules = append(spec.Rules, *rule) + spec.Rules = append(spec.Rules, *getRule(i.jaeger.Spec.AllInOne.Options, backend)) } else if _, ok := i.jaeger.Spec.Query.Options.Map()["query.base-path"]; ok && strings.ToLower(i.jaeger.Spec.Strategy) == "production" { - rule := &v1beta1.IngressRule{} - rule.HTTP = &v1beta1.HTTPIngressRuleValue{ - Paths: []v1beta1.HTTPIngressPath{ - v1beta1.HTTPIngressPath{ - Path: i.jaeger.Spec.Query.Options.Map()["query.base-path"], - Backend: *backend, - }, - }, - } - spec.Rules = append(spec.Rules, *rule) + spec.Rules = append(spec.Rules, *getRule(i.jaeger.Spec.Query.Options, backend)) } else { spec.Backend = backend } @@ -82,3 +64,16 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { Spec: spec, } } + +func getRule(options v1alpha1.Options, backend *v1beta1.IngressBackend) *v1beta1.IngressRule { + rule := &v1beta1.IngressRule{} + rule.HTTP = &v1beta1.HTTPIngressRuleValue{ + Paths: []v1beta1.HTTPIngressPath{ + v1beta1.HTTPIngressPath{ + Path: options.Map()["query.base-path"], + Backend: *backend, + }, + }, + } + return rule +} diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index bee5775fc..43bbbe152 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -2,14 +2,19 @@ package e2e import ( goctx "context" + "encoding/json" "fmt" + "io/ioutil" + "net/http" "testing" + "time" "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" framework "github.com/operator-framework/operator-sdk/pkg/test" "github.com/operator-framework/operator-sdk/pkg/test/e2eutil" "github.com/sirupsen/logrus" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" ) func JaegerAllInOne(t *testing.T) { @@ -57,3 +62,78 @@ func allInOneTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) return e2eutil.WaitForDeployment(t, f.KubeClient, namespace, "my-jaeger", 1, retryInterval, timeout) } + +func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) error { + cleanupOptions := &framework.CleanupOptions{TestContext: ctx, Timeout: timeout, RetryInterval: retryInterval} + namespace, err := ctx.GetNamespace() + if err != nil { + return fmt.Errorf("could not get namespace: %v", err) + } + + j := &v1alpha1.Jaeger{ + TypeMeta: metav1.TypeMeta{ + Kind: "Jaeger", + APIVersion: "io.jaegertracing/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "all-in-one-with-base-path", + Namespace: namespace, + }, + Spec: v1alpha1.JaegerSpec{ + Strategy: "allInOne", + AllInOne: v1alpha1.JaegerAllInOneSpec{ + Options: v1alpha1.NewOptions(map[string]interface{}{ + "query.base-path": "/jaeger", + }), + }, + }, + } + + err = f.Client.Create(goctx.TODO(), j, cleanupOptions) + if err != nil { + return err + } + + err = WaitForIngress(t, f.KubeClient, namespace, "all-in-one-with-base-path-query", retryInterval, timeout) + if err != nil { + return err + } + + i, err := f.KubeClient.ExtensionsV1beta1().Ingresses(namespace).Get("all-in-one-with-base-path-query", metav1.GetOptions{}) + if err != nil { + return err + } + + if len(i.Status.LoadBalancer.Ingress) != 1 { + return fmt.Errorf("Wrong number of ingresses. Expected 1, was %v", len(i.Status.LoadBalancer.Ingress)) + } + + address := i.Status.LoadBalancer.Ingress[0].IP + url := fmt.Sprintf("http://%s/api/traces?service=order", address) + c := http.Client{Timeout: time.Second} + + req, err := http.NewRequest(http.MethodGet, url, nil) + if err != nil { + return err + } + + return wait.Poll(retryInterval, timeout, func() (done bool, err error) { + res, err := c.Do(req) + if err != nil { + return false, err + } + + body, err := ioutil.ReadAll(res.Body) + if err != nil { + return false, err + } + + resp := &resp{} + err = json.Unmarshal(body, &resp) + if err != nil { + return false, err + } + + return len(resp.Data) > 0, nil + }) +} From 77b38634068186f7aa02b64333416992030d52f9 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 12:48:02 +0000 Subject: [PATCH 3/7] Hooked in current e2e test Signed-off-by: Gary Brown --- test/e2e/all_in_one_test.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index 43bbbe152..776d0193d 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -2,7 +2,6 @@ package e2e import ( goctx "context" - "encoding/json" "fmt" "io/ioutil" "net/http" @@ -25,6 +24,10 @@ func JaegerAllInOne(t *testing.T) { if err := allInOneTest(t, framework.Global, ctx); err != nil { t.Fatal(err) } + + if err := allInOneWithUIBasePathTest(t, framework.Global, ctx); err != nil { + t.Fatal(err) + } } func allInOneTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) error { @@ -109,7 +112,7 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame } address := i.Status.LoadBalancer.Ingress[0].IP - url := fmt.Sprintf("http://%s/api/traces?service=order", address) + url := fmt.Sprintf("http://%s/search", address) c := http.Client{Timeout: time.Second} req, err := http.NewRequest(http.MethodGet, url, nil) @@ -128,12 +131,6 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame return false, err } - resp := &resp{} - err = json.Unmarshal(body, &resp) - if err != nil { - return false, err - } - - return len(resp.Data) > 0, nil + return len(body) > 0, nil }) } From 02c3c55ec997f22f1e2f266c92bd7b2a4f49b7ab Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 14:18:32 +0000 Subject: [PATCH 4/7] Update test to check status code Signed-off-by: Gary Brown --- test/e2e/all_in_one_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index 776d0193d..ba281eb41 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -73,6 +73,8 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame return fmt.Errorf("could not get namespace: %v", err) } + basePath := "/jaeger" + j := &v1alpha1.Jaeger{ TypeMeta: metav1.TypeMeta{ Kind: "Jaeger", @@ -86,7 +88,7 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame Strategy: "allInOne", AllInOne: v1alpha1.JaegerAllInOneSpec{ Options: v1alpha1.NewOptions(map[string]interface{}{ - "query.base-path": "/jaeger", + "query.base-path": basePath, }), }, }, @@ -112,7 +114,7 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame } address := i.Status.LoadBalancer.Ingress[0].IP - url := fmt.Sprintf("http://%s/search", address) + url := fmt.Sprintf("http://%s%s/search", address, basePath) c := http.Client{Timeout: time.Second} req, err := http.NewRequest(http.MethodGet, url, nil) @@ -126,6 +128,10 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame return false, err } + if res.StatusCode != 200 { + return false, fmt.Errorf("unexpected status code %d", res.StatusCode) + } + body, err := ioutil.ReadAll(res.Body) if err != nil { return false, err From c4b9855a16f55fe7af02299c68c41ab3cb3cbee1 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 16:05:50 +0000 Subject: [PATCH 5/7] Add ingress annotation support to fix e2e test Signed-off-by: Gary Brown --- pkg/apis/io/v1alpha1/types.go | 1 + pkg/ingress/query.go | 4 ++++ pkg/ingress/query_test.go | 19 +++++++++++++++++++ test/e2e/all_in_one_test.go | 4 ++++ 4 files changed, 28 insertions(+) diff --git a/pkg/apis/io/v1alpha1/types.go b/pkg/apis/io/v1alpha1/types.go index 9ae69f34f..044d91702 100644 --- a/pkg/apis/io/v1alpha1/types.go +++ b/pkg/apis/io/v1alpha1/types.go @@ -80,6 +80,7 @@ type JaegerQuerySpec struct { type JaegerIngressSpec struct { Enabled *bool `json:"enabled"` Security IngressSecurityType `json:"security"` + JaegerCommonSpec } // JaegerAllInOneSpec defines the options to be used when deploying the query diff --git a/pkg/ingress/query.go b/pkg/ingress/query.go index 27a7f9201..aa0cc985b 100644 --- a/pkg/ingress/query.go +++ b/pkg/ingress/query.go @@ -10,6 +10,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" ) // QueryIngress builds pods for jaegertracing/jaeger-query @@ -30,6 +31,8 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { trueVar := true + commonSpec := util.Merge([]v1alpha1.JaegerCommonSpec{i.jaeger.Spec.Ingress.JaegerCommonSpec, i.jaeger.Spec.JaegerCommonSpec}) + spec := v1beta1.IngressSpec{} backend := &v1beta1.IngressBackend{ ServiceName: service.GetNameForQueryService(i.jaeger), @@ -60,6 +63,7 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { Controller: &trueVar, }, }, + Annotations: commonSpec.Annotations, }, Spec: spec, } diff --git a/pkg/ingress/query_test.go b/pkg/ingress/query_test.go index 106e267a7..a1610388e 100644 --- a/pkg/ingress/query_test.go +++ b/pkg/ingress/query_test.go @@ -81,3 +81,22 @@ func TestQueryIngressQueryBasePath(t *testing.T) { assert.Equal(t, "/jaeger", dep.Spec.Rules[0].HTTP.Paths[0].Path) assert.NotNil(t, dep.Spec.Rules[0].HTTP.Paths[0].Backend) } + +func TestQueryIngressAnnotations(t *testing.T) { + jaeger := v1alpha1.NewJaeger("TestQueryIngressAnnotations") + jaeger.Spec.Annotations = map[string]string{ + "name": "operator", + "hello": "jaeger", + } + jaeger.Spec.Ingress.Annotations = map[string]string{ + "hello": "world", // Override top level annotation + "prometheus.io/scrape": "false", + } + + ingress := NewQueryIngress(jaeger) + dep := ingress.Get() + + assert.Equal(t, "operator", dep.Annotations["name"]) + assert.Equal(t, "world", dep.Annotations["hello"]) + assert.Equal(t, "false", dep.Annotations["prometheus.io/scrape"]) +} diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index ba281eb41..52ac34d32 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -94,6 +94,10 @@ func allInOneWithUIBasePathTest(t *testing.T, f *framework.Framework, ctx *frame }, } + j.Spec.Annotations = map[string]string{ + "nginx.ingress.kubernetes.io/ssl-redirect": "false", + } + err = f.Client.Create(goctx.TODO(), j, cleanupOptions) if err != nil { return err From a4c04d32d01772531a4362fb62df59f89f77377f Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 16:09:56 +0000 Subject: [PATCH 6/7] Generate fix Signed-off-by: Gary Brown --- pkg/apis/io/v1alpha1/zz_generated.deepcopy.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go index 901366089..345a1dd10 100644 --- a/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go @@ -157,6 +157,7 @@ func (in *JaegerIngressSpec) DeepCopyInto(out *JaegerIngressSpec) { *out = new(bool) **out = **in } + in.JaegerCommonSpec.DeepCopyInto(&out.JaegerCommonSpec) return } From 0819962332b04611c28c46148a899d719512b161 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 17:21:28 +0000 Subject: [PATCH 7/7] Update based on comments Signed-off-by: Gary Brown --- pkg/ingress/query.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/ingress/query.go b/pkg/ingress/query.go index aa0cc985b..2d25fddef 100644 --- a/pkg/ingress/query.go +++ b/pkg/ingress/query.go @@ -34,16 +34,16 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { commonSpec := util.Merge([]v1alpha1.JaegerCommonSpec{i.jaeger.Spec.Ingress.JaegerCommonSpec, i.jaeger.Spec.JaegerCommonSpec}) spec := v1beta1.IngressSpec{} - backend := &v1beta1.IngressBackend{ + backend := v1beta1.IngressBackend{ ServiceName: service.GetNameForQueryService(i.jaeger), ServicePort: intstr.FromInt(service.GetPortForQueryService(i.jaeger)), } if _, ok := i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"]; ok && strings.ToLower(i.jaeger.Spec.Strategy) == "allinone" { - spec.Rules = append(spec.Rules, *getRule(i.jaeger.Spec.AllInOne.Options, backend)) + spec.Rules = append(spec.Rules, getRule(i.jaeger.Spec.AllInOne.Options, backend)) } else if _, ok := i.jaeger.Spec.Query.Options.Map()["query.base-path"]; ok && strings.ToLower(i.jaeger.Spec.Strategy) == "production" { - spec.Rules = append(spec.Rules, *getRule(i.jaeger.Spec.Query.Options, backend)) + spec.Rules = append(spec.Rules, getRule(i.jaeger.Spec.Query.Options, backend)) } else { - spec.Backend = backend + spec.Backend = &backend } return &v1beta1.Ingress{ @@ -69,13 +69,13 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { } } -func getRule(options v1alpha1.Options, backend *v1beta1.IngressBackend) *v1beta1.IngressRule { - rule := &v1beta1.IngressRule{} +func getRule(options v1alpha1.Options, backend v1beta1.IngressBackend) v1beta1.IngressRule { + rule := v1beta1.IngressRule{} rule.HTTP = &v1beta1.HTTPIngressRuleValue{ Paths: []v1beta1.HTTPIngressPath{ v1beta1.HTTPIngressPath{ Path: options.Map()["query.base-path"], - Backend: *backend, + Backend: backend, }, }, }