From 89b4b04b2b0d6abbee2a94cd002e315b97ae1c30 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 6 Dec 2018 09:52:07 +0100 Subject: [PATCH 1/3] Enable dependencies and index cleaner by default Signed-off-by: Pavol Loffay --- deploy/examples/simple-prod.yaml | 2 -- pkg/apis/io/v1alpha1/jaeger_types.go | 4 ++-- pkg/cronjob/es_index_cleaner.go | 2 +- pkg/storage/cassandra_dependencies.go | 1 + pkg/strategy/all-in-one.go | 8 ++++++-- pkg/strategy/all-in-one_test.go | 14 ++++++++------ pkg/strategy/controller.go | 19 +++++++++++++++---- pkg/strategy/controller_test.go | 20 ++++++++++++-------- pkg/strategy/production.go | 4 ++-- test/e2e/es_index_cleaner_test.go | 1 - test/e2e/spark_dependencies_test.go | 1 - 11 files changed, 47 insertions(+), 29 deletions(-) diff --git a/deploy/examples/simple-prod.yaml b/deploy/examples/simple-prod.yaml index d9a552423..52dd1e184 100644 --- a/deploy/examples/simple-prod.yaml +++ b/deploy/examples/simple-prod.yaml @@ -10,5 +10,3 @@ spec: options: es: server-urls: http://elasticsearch:9200 - dependencies: - enabled: true diff --git a/pkg/apis/io/v1alpha1/jaeger_types.go b/pkg/apis/io/v1alpha1/jaeger_types.go index 44b7af8a3..67910cf23 100644 --- a/pkg/apis/io/v1alpha1/jaeger_types.go +++ b/pkg/apis/io/v1alpha1/jaeger_types.go @@ -139,7 +139,7 @@ type JaegerCassandraCreateSchemaSpec struct { // JaegerDependenciesSpec defined options for running spark-dependencies. type JaegerDependenciesSpec struct { - Enabled bool `json:"enabled"` + Enabled *bool `json:"enabled"` SparkMaster string `json:"sparkMaster"` Schedule string `json:"schedule"` Image string `json:"image"` @@ -153,7 +153,7 @@ type JaegerDependenciesSpec struct { // JaegerEsIndexCleanerSpec holds the options related to es-index-cleaner type JaegerEsIndexCleanerSpec struct { - Enabled bool `json:"enabled"` + Enabled *bool `json:"enabled"` NumberOfDays int `json:"numberOfDays"` Schedule string `json:"schedule"` Image string `json:"image"` diff --git a/pkg/cronjob/es_index_cleaner.go b/pkg/cronjob/es_index_cleaner.go index bfdeed311..7fd41f2e1 100644 --- a/pkg/cronjob/es_index_cleaner.go +++ b/pkg/cronjob/es_index_cleaner.go @@ -41,7 +41,7 @@ func CreateEsIndexCleaner(jaeger *v1alpha1.Jaeger) *batchv1beta1.CronJob { { Image: jaeger.Spec.Storage.EsIndexCleaner.Image, Name: name, - Env: []v1.EnvVar{{Name: "INDEX_PREFIX", Value: jaeger.Spec.Storage.Options.Map()["es.index-prefix"]}}, + Env: removeEmptyVars([]v1.EnvVar{{Name: "INDEX_PREFIX", Value: jaeger.Spec.Storage.Options.Map()["es.index-prefix"]}}), Args: []string{strconv.Itoa(jaeger.Spec.Storage.EsIndexCleaner.NumberOfDays), esUrls}, }, }, diff --git a/pkg/storage/cassandra_dependencies.go b/pkg/storage/cassandra_dependencies.go index 631cb342d..300e2cf0c 100644 --- a/pkg/storage/cassandra_dependencies.go +++ b/pkg/storage/cassandra_dependencies.go @@ -15,6 +15,7 @@ import ( func cassandraDeps(jaeger *v1alpha1.Jaeger) []batchv1.Job { trueVar := true + // TODO should be moved to normalize if jaeger.Spec.Storage.CassandraCreateSchema.Enabled == nil { jaeger.Spec.Storage.CassandraCreateSchema.Enabled = &trueVar } diff --git a/pkg/strategy/all-in-one.go b/pkg/strategy/all-in-one.go index 16284cec5..ef715d518 100644 --- a/pkg/strategy/all-in-one.go +++ b/pkg/strategy/all-in-one.go @@ -79,7 +79,7 @@ func (c *allInOneStrategy) Create() []runtime.Object { } } - if c.jaeger.Spec.Storage.SparkDependencies.Enabled { + if isBoolTrue(c.jaeger.Spec.Storage.SparkDependencies.Enabled) { if cronjob.SupportedStorage(c.jaeger.Spec.Storage.Type) { os = append(os, cronjob.CreateSparkDependencies(c.jaeger)) } else { @@ -87,7 +87,7 @@ func (c *allInOneStrategy) Create() []runtime.Object { } } - if c.jaeger.Spec.Storage.EsIndexCleaner.Enabled { + if isBoolTrue(c.jaeger.Spec.Storage.EsIndexCleaner.Enabled) { if c.jaeger.Spec.Storage.Type == "elasticsearch" { os = append(os, cronjob.CreateEsIndexCleaner(c.jaeger)) } else { @@ -106,3 +106,7 @@ func (c *allInOneStrategy) Update() []runtime.Object { func (c *allInOneStrategy) Dependencies() []batchv1.Job { return storage.Dependencies(c.jaeger) } + +func isBoolTrue(b *bool) bool { + return b != nil && *b +} diff --git a/pkg/strategy/all-in-one_test.go b/pkg/strategy/all-in-one_test.go index 7309189f7..6009f776b 100644 --- a/pkg/strategy/all-in-one_test.go +++ b/pkg/strategy/all-in-one_test.go @@ -141,21 +141,22 @@ func TestSparkDependenciesAllInOne(t *testing.T) { } func testSparkDependencies(t *testing.T, fce func(jaeger *v1alpha1.Jaeger) S) { + trueVar := true tests := []struct { jaeger *v1alpha1.Jaeger sparkCronJobEnabled bool }{ {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "elasticsearch", - SparkDependencies: v1alpha1.JaegerDependenciesSpec{Enabled: true}}, + SparkDependencies: v1alpha1.JaegerDependenciesSpec{Enabled: &trueVar}}, }}, sparkCronJobEnabled: true}, {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "cassandra", - SparkDependencies: v1alpha1.JaegerDependenciesSpec{Enabled: true}}, + SparkDependencies: v1alpha1.JaegerDependenciesSpec{Enabled: &trueVar}}, }}, sparkCronJobEnabled: true}, {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "kafka", - SparkDependencies: v1alpha1.JaegerDependenciesSpec{Enabled: true}}, + SparkDependencies: v1alpha1.JaegerDependenciesSpec{Enabled: &trueVar}}, }}, sparkCronJobEnabled: false}, {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "elasticsearch"}, @@ -180,21 +181,22 @@ func TestEsIndexCleanerAllInOne(t *testing.T) { } func testEsIndexCleaner(t *testing.T, fce func(jaeger *v1alpha1.Jaeger) S) { + trueVar := true tests := []struct { jaeger *v1alpha1.Jaeger sparkCronJobEnabled bool }{ {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "elasticsearch", - EsIndexCleaner: v1alpha1.JaegerEsIndexCleanerSpec{Enabled: true}}, + EsIndexCleaner: v1alpha1.JaegerEsIndexCleanerSpec{Enabled: &trueVar}}, }}, sparkCronJobEnabled: true}, {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "cassandra", - EsIndexCleaner: v1alpha1.JaegerEsIndexCleanerSpec{Enabled: true}}, + EsIndexCleaner: v1alpha1.JaegerEsIndexCleanerSpec{Enabled: &trueVar}}, }}, sparkCronJobEnabled: false}, {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "kafka", - EsIndexCleaner: v1alpha1.JaegerEsIndexCleanerSpec{Enabled: true}}, + EsIndexCleaner: v1alpha1.JaegerEsIndexCleanerSpec{Enabled: &trueVar}}, }}, sparkCronJobEnabled: false}, {jaeger: &v1alpha1.Jaeger{Spec: v1alpha1.JaegerSpec{ Storage: v1alpha1.JaegerStorageSpec{Type: "elasticsearch"}, diff --git a/pkg/strategy/controller.go b/pkg/strategy/controller.go index d4612f903..014de8a9f 100644 --- a/pkg/strategy/controller.go +++ b/pkg/strategy/controller.go @@ -11,6 +11,7 @@ import ( "k8s.io/apimachinery/pkg/runtime" "github.com/jaegertracing/jaeger-operator/pkg/apis/io/v1alpha1" + "github.com/jaegertracing/jaeger-operator/pkg/cronjob" "github.com/jaegertracing/jaeger-operator/pkg/storage" ) @@ -89,11 +90,16 @@ func normalize(jaeger *v1alpha1.Jaeger) { jaeger.Spec.Ingress.Security = v1alpha1.IngressSecurityNone } - normalizeSparkDependencies(&jaeger.Spec.Storage.SparkDependencies) - normalizeIndexCleaner(&jaeger.Spec.Storage.EsIndexCleaner) + normalizeSparkDependencies(&jaeger.Spec.Storage.SparkDependencies, jaeger.Spec.Storage.Type) + normalizeIndexCleaner(&jaeger.Spec.Storage.EsIndexCleaner, jaeger.Spec.Storage.Type) } -func normalizeSparkDependencies(spec *v1alpha1.JaegerDependenciesSpec) { +func normalizeSparkDependencies(spec *v1alpha1.JaegerDependenciesSpec, storage string) { + // auto enable only for supported storages + if cronjob.SupportedStorage(storage) && spec.Enabled == nil { + trueVar := true + spec.Enabled = &trueVar + } if spec.Image == "" { spec.Image = fmt.Sprintf("%s", viper.GetString("jaeger-spark-dependencies-image")) } @@ -102,7 +108,12 @@ func normalizeSparkDependencies(spec *v1alpha1.JaegerDependenciesSpec) { } } -func normalizeIndexCleaner(spec *v1alpha1.JaegerEsIndexCleanerSpec) { +func normalizeIndexCleaner(spec *v1alpha1.JaegerEsIndexCleanerSpec, storage string) { + // auto enable only for supported storages + if storage == "elasticsearch" && spec.Enabled == nil { + trueVar := true + spec.Enabled = &trueVar + } if spec.Image == "" { spec.Image = fmt.Sprintf("%s", viper.GetString("jaeger-es-index-cleaner-image")) } diff --git a/pkg/strategy/controller_test.go b/pkg/strategy/controller_test.go index fcf7f88a8..eb65a2298 100644 --- a/pkg/strategy/controller_test.go +++ b/pkg/strategy/controller_test.go @@ -170,17 +170,19 @@ func TestAcceptExplicitValueFromSecurityWhenOnOpenShift(t *testing.T) { func TestNormalizeIndexCleaner(t *testing.T) { viper.Set("jaeger-es-index-cleaner-image", "foo") defer viper.Reset() + trueVar := true + falseVar := false tests := []struct { underTest v1alpha1.JaegerEsIndexCleanerSpec expected v1alpha1.JaegerEsIndexCleanerSpec }{ {underTest: v1alpha1.JaegerEsIndexCleanerSpec{}, - expected: v1alpha1.JaegerEsIndexCleanerSpec{Image: "foo", Schedule: "55 23 * * *", NumberOfDays: 7}}, - {underTest: v1alpha1.JaegerEsIndexCleanerSpec{Image: "bla", Schedule: "lol", NumberOfDays: 55}, - expected: v1alpha1.JaegerEsIndexCleanerSpec{Image: "bla", Schedule: "lol", NumberOfDays: 55}}, + expected: v1alpha1.JaegerEsIndexCleanerSpec{Image: "foo", Schedule: "55 23 * * *", NumberOfDays: 7, Enabled: &trueVar}}, + {underTest: v1alpha1.JaegerEsIndexCleanerSpec{Image: "bla", Schedule: "lol", NumberOfDays: 55, Enabled: &falseVar}, + expected: v1alpha1.JaegerEsIndexCleanerSpec{Image: "bla", Schedule: "lol", NumberOfDays: 55, Enabled: &falseVar}}, } for _, test := range tests { - normalizeIndexCleaner(&test.underTest) + normalizeIndexCleaner(&test.underTest, "elasticsearch") assert.Equal(t, test.expected, test.underTest) } } @@ -188,17 +190,19 @@ func TestNormalizeIndexCleaner(t *testing.T) { func TestNormalizeSparkDependencies(t *testing.T) { viper.Set("jaeger-spark-dependencies-image", "foo") defer viper.Reset() + trueVar := true + falseVar := false tests := []struct { underTest v1alpha1.JaegerDependenciesSpec expected v1alpha1.JaegerDependenciesSpec }{ {underTest: v1alpha1.JaegerDependenciesSpec{}, - expected: v1alpha1.JaegerDependenciesSpec{Schedule: "55 23 * * *", Image: "foo"}}, - {underTest: v1alpha1.JaegerDependenciesSpec{Schedule: "foo", Image: "bla"}, - expected: v1alpha1.JaegerDependenciesSpec{Schedule: "foo", Image: "bla"}}, + expected: v1alpha1.JaegerDependenciesSpec{Schedule: "55 23 * * *", Image: "foo", Enabled: &trueVar}}, + {underTest: v1alpha1.JaegerDependenciesSpec{Schedule: "foo", Image: "bla", Enabled: &falseVar}, + expected: v1alpha1.JaegerDependenciesSpec{Schedule: "foo", Image: "bla", Enabled: &falseVar}}, } for _, test := range tests { - normalizeSparkDependencies(&test.underTest) + normalizeSparkDependencies(&test.underTest, "elasticsearch") assert.Equal(t, test.expected, test.underTest) } } diff --git a/pkg/strategy/production.go b/pkg/strategy/production.go index ddac5f1c6..9444f1f52 100644 --- a/pkg/strategy/production.go +++ b/pkg/strategy/production.go @@ -85,7 +85,7 @@ func (c *productionStrategy) Create() []runtime.Object { } } - if c.jaeger.Spec.Storage.SparkDependencies.Enabled { + if isBoolTrue(c.jaeger.Spec.Storage.SparkDependencies.Enabled) { if cronjob.SupportedStorage(c.jaeger.Spec.Storage.Type) { os = append(os, cronjob.CreateSparkDependencies(c.jaeger)) } else { @@ -93,7 +93,7 @@ func (c *productionStrategy) Create() []runtime.Object { } } - if c.jaeger.Spec.Storage.EsIndexCleaner.Enabled { + if isBoolTrue(c.jaeger.Spec.Storage.EsIndexCleaner.Enabled) { if c.jaeger.Spec.Storage.Type == "elasticsearch" { os = append(os, cronjob.CreateEsIndexCleaner(c.jaeger)) } else { diff --git a/test/e2e/es_index_cleaner_test.go b/test/e2e/es_index_cleaner_test.go index 619b9504b..a603f8639 100644 --- a/test/e2e/es_index_cleaner_test.go +++ b/test/e2e/es_index_cleaner_test.go @@ -44,7 +44,6 @@ func esIndexCleanerTest(t *testing.T, f *framework.Framework, testCtx *framework "es.server-urls": "http://elasticsearch.default.svc:9200", }), EsIndexCleaner:v1alpha1.JaegerEsIndexCleanerSpec{ - Enabled:true, Schedule: "*/1 * * * *", }, }, diff --git a/test/e2e/spark_dependencies_test.go b/test/e2e/spark_dependencies_test.go index 433948691..42ac51a58 100644 --- a/test/e2e/spark_dependencies_test.go +++ b/test/e2e/spark_dependencies_test.go @@ -47,7 +47,6 @@ func sparkTest(t *testing.T, f *framework.Framework, testCtx *framework.TestCtx, } storage.SparkDependencies = v1alpha1.JaegerDependenciesSpec{ - Enabled: true, // run immediately Schedule: "*/1 * * * *", } From ebc6512d42fa80494a3361a5d5f7486916678a77 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Thu, 6 Dec 2018 09:59:52 +0100 Subject: [PATCH 2/3] Fix generate Signed-off-by: Pavol Loffay --- pkg/apis/io/v1alpha1/zz_generated.deepcopy.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go index 59165a95d..4a6d4e44e 100644 --- a/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/io/v1alpha1/zz_generated.deepcopy.go @@ -190,6 +190,11 @@ func (in *JaegerCommonSpec) DeepCopy() *JaegerCommonSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JaegerDependenciesSpec) DeepCopyInto(out *JaegerDependenciesSpec) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } return } @@ -206,6 +211,11 @@ func (in *JaegerDependenciesSpec) DeepCopy() *JaegerDependenciesSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *JaegerEsIndexCleanerSpec) DeepCopyInto(out *JaegerEsIndexCleanerSpec) { *out = *in + if in.Enabled != nil { + in, out := &in.Enabled, &out.Enabled + *out = new(bool) + **out = **in + } return } @@ -355,8 +365,8 @@ func (in *JaegerStorageSpec) DeepCopyInto(out *JaegerStorageSpec) { *out = *in in.Options.DeepCopyInto(&out.Options) in.CassandraCreateSchema.DeepCopyInto(&out.CassandraCreateSchema) - out.SparkDependencies = in.SparkDependencies - out.EsIndexCleaner = in.EsIndexCleaner + in.SparkDependencies.DeepCopyInto(&out.SparkDependencies) + in.EsIndexCleaner.DeepCopyInto(&out.EsIndexCleaner) return } From 1444a77d4f79b7a40cdde759ac241dc84d635544 Mon Sep 17 00:00:00 2001 From: Pavol Loffay Date: Fri, 7 Dec 2018 12:40:57 +0100 Subject: [PATCH 3/3] Use to lower Signed-off-by: Pavol Loffay --- pkg/cronjob/spark_dependencies.go | 3 ++- pkg/strategy/all-in-one.go | 3 ++- pkg/strategy/production.go | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/cronjob/spark_dependencies.go b/pkg/cronjob/spark_dependencies.go index 192189699..5efe4d28c 100644 --- a/pkg/cronjob/spark_dependencies.go +++ b/pkg/cronjob/spark_dependencies.go @@ -3,6 +3,7 @@ package cronjob import ( "fmt" "strconv" + "strings" batchv1 "k8s.io/api/batch/v1" batchv1beta1 "k8s.io/api/batch/v1beta1" @@ -16,7 +17,7 @@ import ( var supportedStorageTypes = map[string]bool{"elasticsearch": true, "cassandra": true} func SupportedStorage(storage string) bool { - return supportedStorageTypes[storage] + return supportedStorageTypes[strings.ToLower(storage)] } func CreateSparkDependencies(jaeger *v1alpha1.Jaeger) *batchv1beta1.CronJob { diff --git a/pkg/strategy/all-in-one.go b/pkg/strategy/all-in-one.go index ef715d518..17aca9c5e 100644 --- a/pkg/strategy/all-in-one.go +++ b/pkg/strategy/all-in-one.go @@ -2,6 +2,7 @@ package strategy import ( "context" + "strings" "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -88,7 +89,7 @@ func (c *allInOneStrategy) Create() []runtime.Object { } if isBoolTrue(c.jaeger.Spec.Storage.EsIndexCleaner.Enabled) { - if c.jaeger.Spec.Storage.Type == "elasticsearch" { + if strings.ToLower(c.jaeger.Spec.Storage.Type) == "elasticsearch" { os = append(os, cronjob.CreateEsIndexCleaner(c.jaeger)) } else { logrus.WithField("type", c.jaeger.Spec.Storage.Type).Warn("Skipping Elasticsearch index cleaner job due to unsupported storage.") diff --git a/pkg/strategy/production.go b/pkg/strategy/production.go index 9444f1f52..6362845a8 100644 --- a/pkg/strategy/production.go +++ b/pkg/strategy/production.go @@ -2,6 +2,7 @@ package strategy import ( "context" + "strings" "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -94,7 +95,7 @@ func (c *productionStrategy) Create() []runtime.Object { } if isBoolTrue(c.jaeger.Spec.Storage.EsIndexCleaner.Enabled) { - if c.jaeger.Spec.Storage.Type == "elasticsearch" { + if strings.ToLower(c.jaeger.Spec.Storage.Type) == "elasticsearch" { os = append(os, cronjob.CreateEsIndexCleaner(c.jaeger)) } else { logrus.WithField("type", c.jaeger.Spec.Storage.Type).Warn("Skipping Elasticsearch index cleaner job due to unsupported storage.")