From f120758b3023e31e7e5941191592c0f02f215dc7 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Tue, 6 Nov 2018 15:15:42 +0000 Subject: [PATCH 1/7] Change CRD to use lower camel case Signed-off-by: Gary Brown --- README.adoc | 14 +++++++------- deploy/examples/all-in-one-with-options.yaml | 4 ++-- deploy/examples/disable-ingress.yaml | 2 +- pkg/apis/io/v1alpha1/types.go | 4 ++-- pkg/controller/controller.go | 8 ++++---- pkg/controller/controller_test.go | 2 +- test/e2e/all_in_one_test.go | 2 +- test/e2e/cassandra.go | 2 +- test/e2e/daemonset.go | 2 +- test/e2e/sidecar.go | 2 +- 10 files changed, 21 insertions(+), 21 deletions(-) diff --git a/README.adoc b/README.adoc index ff165c105..b347baa22 100644 --- a/README.adoc +++ b/README.adoc @@ -111,8 +111,8 @@ kind: Jaeger metadata: name: my-jaeger spec: - strategy: all-in-one # <1> - all-in-one: + strategy: allInOne # <1> + allInOne: image: jaegertracing/all-in-one:1.7 # <2> options: # <3> log-level: debug # <4> @@ -123,7 +123,7 @@ spec: agent: strategy: DaemonSet # <7> ---- -<1> The default strategy is `all-in-one`. The only other possible value is `production`. +<1> The default strategy is `allInOne`. The only other possible value is `production`. <2> The image to use, in a regular Docker syntax <3> The options to be passed verbatim to the underlying binary.Refer to the Jaeger documentation and/or to the `--help` option from the related binary for all the available options <4> The option is a simple `key: value` map. In this case, we want the option `--log-level=debug` to be passed to the binary. @@ -255,10 +255,10 @@ kind: Jaeger metadata: name: cassandra-without-create-schema spec: - strategy: all-in-one + strategy: allInOne storage: type: cassandra - cassandra-create-schema: + cassandraCreateSchema: enabled: false # <1> ---- <1> Defaults to `true` @@ -272,14 +272,14 @@ kind: Jaeger metadata: name: cassandra-with-create-schema spec: - strategy: all-in-one # <1> + strategy: allInOne # <1> storage: type: cassandra options: # <2> cassandra: servers: cassandra keyspace: jaeger_v1_datacenter3 - cassandra-create-schema: # <3> + cassandraCreateSchema: # <3> datacenter: "datacenter3" mode: "test" ---- diff --git a/deploy/examples/all-in-one-with-options.yaml b/deploy/examples/all-in-one-with-options.yaml index 445eedd5a..c0897a80a 100644 --- a/deploy/examples/all-in-one-with-options.yaml +++ b/deploy/examples/all-in-one-with-options.yaml @@ -3,8 +3,8 @@ kind: "Jaeger" metadata: name: "my-jaeger" spec: - strategy: all-in-one - all-in-one: + strategy: allInOne + allInOne: image: jaegertracing/all-in-one:1.7 options: log-level: debug diff --git a/deploy/examples/disable-ingress.yaml b/deploy/examples/disable-ingress.yaml index e374a5480..493fa4e8f 100644 --- a/deploy/examples/disable-ingress.yaml +++ b/deploy/examples/disable-ingress.yaml @@ -4,4 +4,4 @@ metadata: name: disable-ingress spec: ingress: - enabled: false \ No newline at end of file + enabled: false diff --git a/pkg/apis/io/v1alpha1/types.go b/pkg/apis/io/v1alpha1/types.go index 88ab0e2c3..8d948e229 100644 --- a/pkg/apis/io/v1alpha1/types.go +++ b/pkg/apis/io/v1alpha1/types.go @@ -26,7 +26,7 @@ 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:"all-in-one"` + AllInOne JaegerAllInOneSpec `json:"allInOne"` Query JaegerQuerySpec `json:"query"` Collector JaegerCollectorSpec `json:"collector"` Agent JaegerAgentSpec `json:"agent"` @@ -79,7 +79,7 @@ type JaegerAgentSpec struct { type JaegerStorageSpec struct { Type string `json:"type"` // can be `memory` (default), `cassandra`, `elasticsearch`, `kafka` or `managed` Options Options `json:"options"` - CassandraCreateSchema JaegerCassandraCreateSchemaSpec `json:"cassandra-create-schema"` + CassandraCreateSchema JaegerCassandraCreateSchemaSpec `json:"cassandraCreateSchema"` } // JaegerCassandraCreateSchemaSpec holds the options related to the create-schema batch job diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index d7e6ad37f..06dcbc9fa 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -23,7 +23,7 @@ func NewController(ctx context.Context, jaeger *v1alpha1.Jaeger) Controller { normalize(jaeger) logrus.Debugf("Jaeger strategy: %s", jaeger.Spec.Strategy) - if jaeger.Spec.Strategy == "all-in-one" { + if jaeger.Spec.Strategy == "allInOne" { return newAllInOneController(ctx, jaeger) } @@ -57,18 +57,18 @@ func normalize(jaeger *v1alpha1.Jaeger) { // normalize the deployment strategy if strings.ToLower(jaeger.Spec.Strategy) != "production" { - jaeger.Spec.Strategy = "all-in-one" + jaeger.Spec.Strategy = "allInOne" } // check for incompatible options // if the storage is `memory`, then the only possible strategy is `all-in-one` - if strings.ToLower(jaeger.Spec.Storage.Type) == "memory" && strings.ToLower(jaeger.Spec.Strategy) != "all-in-one" { + if strings.ToLower(jaeger.Spec.Storage.Type) == "memory" && strings.ToLower(jaeger.Spec.Strategy) != "allInOne" { logrus.Warnf( "No suitable storage was provided for the Jaeger instance '%v'. Falling back to all-in-one. Storage type: '%v'", jaeger.Name, jaeger.Spec.Storage.Type, ) - jaeger.Spec.Strategy = "all-in-one" + jaeger.Spec.Strategy = "allInOne" } } diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 0a8f9ab72..8b421ae2f 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -94,7 +94,7 @@ func TestIncompatibleStorageForProduction(t *testing.T) { }, } normalize(jaeger) - assert.Equal(t, "all-in-one", jaeger.Spec.Strategy) + assert.Equal(t, "allInOne", jaeger.Spec.Strategy) } func getDeployments(objs []sdk.Object) []*appsv1.Deployment { diff --git a/test/e2e/all_in_one_test.go b/test/e2e/all_in_one_test.go index 6f0c5c268..bee5775fc 100644 --- a/test/e2e/all_in_one_test.go +++ b/test/e2e/all_in_one_test.go @@ -39,7 +39,7 @@ func allInOneTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) Namespace: namespace, }, Spec: v1alpha1.JaegerSpec{ - Strategy: "all-in-one", + Strategy: "allInOne", AllInOne: v1alpha1.JaegerAllInOneSpec{ Options: v1alpha1.NewOptions(map[string]interface{}{ "log-level": "debug", diff --git a/test/e2e/cassandra.go b/test/e2e/cassandra.go index d740d87a7..8d624fc87 100644 --- a/test/e2e/cassandra.go +++ b/test/e2e/cassandra.go @@ -39,7 +39,7 @@ func cassandraTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) Namespace: namespace, }, Spec: v1alpha1.JaegerSpec{ - Strategy: "all-in-one", + Strategy: "allInOne", AllInOne: v1alpha1.JaegerAllInOneSpec{}, Storage: v1alpha1.JaegerStorageSpec{ Type: "cassandra", diff --git a/test/e2e/daemonset.go b/test/e2e/daemonset.go index 55a54acc6..4a5a153aa 100644 --- a/test/e2e/daemonset.go +++ b/test/e2e/daemonset.go @@ -47,7 +47,7 @@ func daemonsetTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) Namespace: namespace, }, Spec: v1alpha1.JaegerSpec{ - Strategy: "all-in-one", + Strategy: "allInOne", AllInOne: v1alpha1.JaegerAllInOneSpec{}, Agent: v1alpha1.JaegerAgentSpec{ Strategy: "DaemonSet", diff --git a/test/e2e/sidecar.go b/test/e2e/sidecar.go index 337891c78..7d01e24c0 100644 --- a/test/e2e/sidecar.go +++ b/test/e2e/sidecar.go @@ -47,7 +47,7 @@ func sidecarTest(t *testing.T, f *framework.Framework, ctx *framework.TestCtx) e Namespace: namespace, }, Spec: v1alpha1.JaegerSpec{ - Strategy: "all-in-one", + Strategy: "allInOne", AllInOne: v1alpha1.JaegerAllInOneSpec{}, Agent: v1alpha1.JaegerAgentSpec{ Options: v1alpha1.NewOptions(map[string]interface{}{ From 118130ec5042db9b0265408e9ed1f80cdaefb2da Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Tue, 6 Nov 2018 15:45:17 +0000 Subject: [PATCH 2/7] Add warning for strategy all-in-one Signed-off-by: Gary Brown --- pkg/controller/controller.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 06dcbc9fa..75a959bdd 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -22,6 +22,11 @@ type Controller interface { func NewController(ctx context.Context, jaeger *v1alpha1.Jaeger) Controller { normalize(jaeger) + if jaeger.Spec.Strategy == "all-in-one" { + logrus.Warnf("Strategy 'all-in-one' is no longer supported, please use 'allInOne'") + jaeger.Spec.Strategy = "allInOne" + } + logrus.Debugf("Jaeger strategy: %s", jaeger.Spec.Strategy) if jaeger.Spec.Strategy == "allInOne" { return newAllInOneController(ctx, jaeger) From 01512d963870e83b892d676eac602a0bdb984116 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Wed, 7 Nov 2018 09:31:54 +0000 Subject: [PATCH 3/7] Add test for deprecated all-in-one strategy Signed-off-by: Gary Brown --- pkg/controller/controller_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 8b421ae2f..a89545322 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -97,6 +97,16 @@ func TestIncompatibleStorageForProduction(t *testing.T) { assert.Equal(t, "allInOne", jaeger.Spec.Strategy) } +func TestDreprecatedAllInOneStrategy(t *testing.T) { + jaeger := &v1alpha1.Jaeger{ + Spec: v1alpha1.JaegerSpec{ + Strategy: "all-in-one", + }, + } + normalize(jaeger) + assert.Equal(t, "allInOne", jaeger.Spec.Strategy) +} + func getDeployments(objs []sdk.Object) []*appsv1.Deployment { var deps []*appsv1.Deployment From cd069f99b3b10a80a0e1158664ce20fcb2157075 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Wed, 7 Nov 2018 09:39:01 +0000 Subject: [PATCH 4/7] Fix for coverage Signed-off-by: Gary Brown --- pkg/controller/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index a89545322..c6f03d5ba 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -103,7 +103,7 @@ func TestDreprecatedAllInOneStrategy(t *testing.T) { Strategy: "all-in-one", }, } - normalize(jaeger) + NewController(context.TODO(), jaeger) assert.Equal(t, "allInOne", jaeger.Spec.Strategy) } From 6e9727ebcf96f45da5ab40e37be253771d2630f5 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Wed, 7 Nov 2018 10:22:58 +0000 Subject: [PATCH 5/7] Fix spelling Signed-off-by: Gary Brown --- pkg/controller/controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index c6f03d5ba..a561d2964 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -97,7 +97,7 @@ func TestIncompatibleStorageForProduction(t *testing.T) { assert.Equal(t, "allInOne", jaeger.Spec.Strategy) } -func TestDreprecatedAllInOneStrategy(t *testing.T) { +func TestDeprecatedAllInOneStrategy(t *testing.T) { jaeger := &v1alpha1.Jaeger{ Spec: v1alpha1.JaegerSpec{ Strategy: "all-in-one", From 763ed64f111e555d5ce7cc901849d6f8767d1256 Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Wed, 7 Nov 2018 10:30:16 +0000 Subject: [PATCH 6/7] Fix coverage again Signed-off-by: Gary Brown --- pkg/controller/controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 75a959bdd..08410b419 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -20,13 +20,13 @@ type Controller interface { // NewController build a new controller object for the given spec func NewController(ctx context.Context, jaeger *v1alpha1.Jaeger) Controller { - normalize(jaeger) - if jaeger.Spec.Strategy == "all-in-one" { logrus.Warnf("Strategy 'all-in-one' is no longer supported, please use 'allInOne'") jaeger.Spec.Strategy = "allInOne" } + normalize(jaeger) + logrus.Debugf("Jaeger strategy: %s", jaeger.Spec.Strategy) if jaeger.Spec.Strategy == "allInOne" { return newAllInOneController(ctx, jaeger) From a58d9e2ee7f5a9c8cfbd6b1a64891df284b46a0b Mon Sep 17 00:00:00 2001 From: Gary Brown Date: Wed, 7 Nov 2018 10:54:07 +0000 Subject: [PATCH 7/7] Make case insensitive and add extra test for memory and not allinone Signed-off-by: Gary Brown --- pkg/controller/controller.go | 6 +++--- pkg/controller/controller_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 08410b419..47c8246d3 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -20,7 +20,7 @@ type Controller interface { // NewController build a new controller object for the given spec func NewController(ctx context.Context, jaeger *v1alpha1.Jaeger) Controller { - if jaeger.Spec.Strategy == "all-in-one" { + if strings.ToLower(jaeger.Spec.Strategy) == "all-in-one" { logrus.Warnf("Strategy 'all-in-one' is no longer supported, please use 'allInOne'") jaeger.Spec.Strategy = "allInOne" } @@ -28,7 +28,7 @@ func NewController(ctx context.Context, jaeger *v1alpha1.Jaeger) Controller { normalize(jaeger) logrus.Debugf("Jaeger strategy: %s", jaeger.Spec.Strategy) - if jaeger.Spec.Strategy == "allInOne" { + if strings.ToLower(jaeger.Spec.Strategy) == "allinone" { return newAllInOneController(ctx, jaeger) } @@ -67,7 +67,7 @@ func normalize(jaeger *v1alpha1.Jaeger) { // check for incompatible options // if the storage is `memory`, then the only possible strategy is `all-in-one` - if strings.ToLower(jaeger.Spec.Storage.Type) == "memory" && strings.ToLower(jaeger.Spec.Strategy) != "allInOne" { + if strings.ToLower(jaeger.Spec.Storage.Type) == "memory" && strings.ToLower(jaeger.Spec.Strategy) != "allinone" { logrus.Warnf( "No suitable storage was provided for the Jaeger instance '%v'. Falling back to all-in-one. Storage type: '%v'", jaeger.Name, diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index a561d2964..af1f61df0 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -107,6 +107,19 @@ func TestDeprecatedAllInOneStrategy(t *testing.T) { assert.Equal(t, "allInOne", jaeger.Spec.Strategy) } +func TestStorageMemoryOnlyUsedWithAllInOneStrategy(t *testing.T) { + jaeger := &v1alpha1.Jaeger{ + Spec: v1alpha1.JaegerSpec{ + Strategy: "production", + Storage: v1alpha1.JaegerStorageSpec{ + Type: "memory", + }, + }, + } + NewController(context.TODO(), jaeger) + assert.Equal(t, "allInOne", jaeger.Spec.Strategy) +} + func getDeployments(objs []sdk.Object) []*appsv1.Deployment { var deps []*appsv1.Deployment