From 4601dc766b57f799f0567a453884be0edbbb77fc Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Mon, 12 Nov 2018 17:29:43 +0000 Subject: [PATCH] Query base path should be used to configure correct path in ingress (#108) * Query base path should be used to configure correct path in ingress Signed-off-by: Gary Brown * Update following review comment and added initial e2e test to check ingress uses existing URI without base path Signed-off-by: Gary Brown * Hooked in current e2e test Signed-off-by: Gary Brown * Update test to check status code Signed-off-by: Gary Brown * Add ingress annotation support to fix e2e test Signed-off-by: Gary Brown * Generate fix Signed-off-by: Gary Brown * Update based on comments Signed-off-by: Gary Brown --- deploy/examples/all-in-one-with-options.yaml | 7 +- pkg/apis/io/v1alpha1/types.go | 1 + pkg/apis/io/v1alpha1/zz_generated.deepcopy.go | 1 + pkg/ingress/query.go | 34 +++++++- pkg/ingress/query_test.go | 58 +++++++++++++ test/e2e/all_in_one_test.go | 87 +++++++++++++++++++ 6 files changed, 183 insertions(+), 5 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/apis/io/v1alpha1/types.go b/pkg/apis/io/v1alpha1/types.go index bfb577f92..357c5a194 100644 --- a/pkg/apis/io/v1alpha1/types.go +++ b/pkg/apis/io/v1alpha1/types.go @@ -81,6 +81,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/apis/io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go index eef0a9fe9..18be00649 100644 --- a/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go @@ -158,6 +158,7 @@ func (in *JaegerIngressSpec) DeepCopyInto(out *JaegerIngressSpec) { *out = new(bool) **out = **in } + in.JaegerCommonSpec.DeepCopyInto(&out.JaegerCommonSpec) return } diff --git a/pkg/ingress/query.go b/pkg/ingress/query.go index 866cbf803..2d25fddef 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" @@ -9,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 @@ -29,6 +31,21 @@ 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), + 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)) + } 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)) + } else { + spec.Backend = &backend + } + return &v1beta1.Ingress{ TypeMeta: metav1.TypeMeta{ Kind: "Ingress", @@ -46,12 +63,21 @@ func (i *QueryIngress) Get() *v1beta1.Ingress { Controller: &trueVar, }, }, + Annotations: commonSpec.Annotations, }, - Spec: v1beta1.IngressSpec{ - Backend: &v1beta1.IngressBackend{ - ServiceName: service.GetNameForQueryService(i.jaeger), - ServicePort: intstr.FromInt(service.GetPortForQueryService(i.jaeger)), + 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/pkg/ingress/query_test.go b/pkg/ingress/query_test.go index 9dcee912b..a1610388e 100644 --- a/pkg/ingress/query_test.go +++ b/pkg/ingress/query_test.go @@ -41,4 +41,62 @@ 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) +} + +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 bee5775fc..52ac34d32 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -3,13 +3,17 @@ package e2e import ( goctx "context" "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) { @@ -20,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 { @@ -57,3 +65,82 @@ 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) + } + + basePath := "/jaeger" + + 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": basePath, + }), + }, + }, + } + + 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 + } + + 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%s/search", address, basePath) + 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 + } + + 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 + } + + return len(body) > 0, nil + }) +}