Skip to content

Commit

Permalink
Enable dependencies and index cleaner by default (jaegertracing#162)
Browse files Browse the repository at this point in the history 
* Enable dependencies and index cleaner by default

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Fix generate

Signed-off-by: Pavol Loffay <ploffay@redhat.com>

* Use to lower

Signed-off-by: Pavol Loffay <ploffay@redhat.com>
  • Loading branch information
@pavolloffay
pavolloffay authored Dec 10, 2018
1 parent a3b8455 commit 1e134be
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 34 deletions.
2 changes: 0 additions & 2 deletions deploy/examples/simple-prod.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,3 @@ spec:
options:
es:
server-urls: http://elasticsearch:9200
dependencies:
enabled: true
4 changes: 2 additions & 2 deletions pkg/apis/io/v1alpha1/jaeger_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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"`
Expand Down
14 changes: 12 additions & 2 deletions pkg/apis/io/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pkg/cronjob/es_index_cleaner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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},
},
},
Expand Down
3 changes: 2 additions & 1 deletion pkg/cronjob/spark_dependencies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cronjob
import (
"fmt"
"strconv"
"strings"

batchv1 "k8s.io/api/batch/v1"
batchv1beta1 "k8s.io/api/batch/v1beta1"
Expand All @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions pkg/storage/cassandra_dependencies.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
11 changes: 8 additions & 3 deletions pkg/strategy/all-in-one.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package strategy

import (
"context"
"strings"

"github.com/sirupsen/logrus"
"github.com/spf13/viper"
Expand Down Expand Up @@ -79,16 +80,16 @@ 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 {
logrus.WithField("type", c.jaeger.Spec.Storage.Type).Warn("Skipping spark dependencies job due to unsupported storage.")
}
}

if c.jaeger.Spec.Storage.EsIndexCleaner.Enabled {
if c.jaeger.Spec.Storage.Type == "elasticsearch" {
if isBoolTrue(c.jaeger.Spec.Storage.EsIndexCleaner.Enabled) {
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.")
Expand All @@ -106,3 +107,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
}
14 changes: 8 additions & 6 deletions pkg/strategy/all-in-one_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
Expand All @@ -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"},
Expand Down
19 changes: 15 additions & 4 deletions pkg/strategy/controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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"))
}
Expand All @@ -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"))
}
Expand Down
20 changes: 12 additions & 8 deletions pkg/strategy/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,35 +170,39 @@ 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)
}
}

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)
}
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/strategy/production.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package strategy

import (
"context"
"strings"

"github.com/sirupsen/logrus"
"github.com/spf13/viper"
Expand Down Expand Up @@ -85,16 +86,16 @@ 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 {
logrus.WithField("type", c.jaeger.Spec.Storage.Type).Warn("Skipping spark dependencies job due to unsupported storage.")
}
}

if c.jaeger.Spec.Storage.EsIndexCleaner.Enabled {
if c.jaeger.Spec.Storage.Type == "elasticsearch" {
if isBoolTrue(c.jaeger.Spec.Storage.EsIndexCleaner.Enabled) {
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.")
Expand Down
1 change: 0 additions & 1 deletion test/e2e/es_index_cleaner_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 * * * *",
},
},
Expand Down
1 change: 0 additions & 1 deletion test/e2e/spark_dependencies_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 * * * *",
}
Expand Down

0 comments on commit 1e134be

Please sign in to comment.