Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert storage type to typed string #1282

Merged
merged 3 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1582,4 +1582,4 @@ sigs.k8s.io/yaml v1.1.0/go.mod h1:UJmg0vDUVViEyp3mgSv9WPwZCDxu4rQW1olrI1uml+o=
sigs.k8s.io/yaml v1.2.0 h1:kr/MCeFWJWTwyaHoR9c8EjH9OumOmoF9YGiZd7lFm/Q=
sigs.k8s.io/yaml v1.2.0/go.mod h1:yfXDCHCao9+ENCvLSE62v9VSji2MKu5jeNfTrofGhJc=
sourcegraph.com/sqs/pbtypes v0.0.0-20180604144634-d3ebe8f20ae4/go.mod h1:ketZ/q3QxT9HOBeFhu6RdvsftgpsbFHBF5Cas6cDKZ0=
sourcegraph.com/sqs/pbtypes v1.0.0/go.mod h1:3AciMUv4qUuRHRHhOG4TZOB+72GdPVz5k+c648qsFS4=
sourcegraph.com/sqs/pbtypes v1.0.0/go.mod h1:3AciMUv4qUuRHRHhOG4TZOB+72GdPVz5k+c648qsFS4=
46 changes: 44 additions & 2 deletions pkg/apis/jaegertracing/v1/jaeger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ type IngressSecurityType string
// +k8s:openapi-gen=true
type JaegerPhase string

// JaegerStorageType represents the Jaeger storage type
// +k8s:openapi-gen=true
type JaegerStorageType string

const (
// FlagPlatformKubernetes represents the value for the 'platform' flag for Kubernetes
// +k8s:openapi-gen=true
Expand Down Expand Up @@ -79,8 +83,47 @@ const (
// JaegerPhaseRunning indicates that the Jaeger instance is ready and running
// +k8s:openapi-gen=true
JaegerPhaseRunning JaegerPhase = "Running"

// JaegerMemoryStorage indicates that the Jaeger storage type is memory. This is the default storage type.
// +k8s:openapi-gen=true
JaegerMemoryStorage JaegerStorageType = "memory"

// JaegerCassandraStorage indicates that the Jaeger storage type is cassandra
// +k8s:openapi-gen=true
JaegerCassandraStorage JaegerStorageType = "cassandra"

// JaegerESStorage indicates that the Jaeger storage type is elasticsearch
// +k8s:openapi-gen=true
JaegerESStorage JaegerStorageType = "elasticsearch"

// JaegerKafkaStorage indicates that the Jaeger storage type is kafka
// +k8s:openapi-gen=true
JaegerKafkaStorage JaegerStorageType = "kafka"

// JaegerBadgerStorage indicates that the Jaeger storage type is badger
// +k8s:openapi-gen=true
JaegerBadgerStorage JaegerStorageType = "badger"
)

// ValidStorageTypes returns the list of valid storage types
func ValidStorageTypes() []JaegerStorageType {
return []JaegerStorageType{
JaegerMemoryStorage,
JaegerCassandraStorage,
JaegerESStorage,
JaegerKafkaStorage,
JaegerBadgerStorage,
}
}

// OptionsPrefix returns the options prefix associated with the storage type
func (storageType JaegerStorageType) OptionsPrefix() string {
if storageType == JaegerESStorage {
return "es"
}
return string(storageType)
}

// JaegerSpec defines the desired state of Jaeger
// +k8s:openapi-gen=true
type JaegerSpec struct {
Expand Down Expand Up @@ -395,9 +438,8 @@ type JaegerAgentSpec struct {
// JaegerStorageSpec defines the common storage options to be used for the query and collector
// +k8s:openapi-gen=true
type JaegerStorageSpec struct {
// Type can be `memory` (default), `cassandra`, `elasticsearch`, `kafka` or `badger`
// +optional
Type string `json:"type,omitempty"`
Type JaegerStorageType `json:"type,omitempty"`

// +optional
SecretName string `json:"secretName,omitempty"`
Expand Down
26 changes: 26 additions & 0 deletions pkg/apis/jaegertracing/v1/jaeger_types_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package v1

import (
"testing"

"github.com/stretchr/testify/assert"
)

func TestDefaultPrefix(t *testing.T) {
assert.Equal(t, "anystorage", JaegerStorageType("anystorage").OptionsPrefix())
}

func TestElasticsearchPrefix(t *testing.T) {
assert.Equal(t, "es", JaegerESStorage.OptionsPrefix())
}

func TestValidTypes(t *testing.T) {
assert.ElementsMatch(t, ValidStorageTypes(),
[]JaegerStorageType{
JaegerMemoryStorage,
JaegerCassandraStorage,
JaegerESStorage,
JaegerKafkaStorage,
JaegerBadgerStorage,
})
}
5 changes: 2 additions & 3 deletions pkg/apis/jaegertracing/v1/zz_generated.openapi.go

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

6 changes: 3 additions & 3 deletions pkg/cronjob/es_index_cleaner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ func TestEsIndexCleanerResources(t *testing.T) {
expected corev1.ResourceRequirements
}{
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "elasticsearch"}}},
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage}}},
expected: corev1.ResourceRequirements{},
},
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{Type: "elasticsearch"},
Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage},
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: parentResources,
},
Expand All @@ -170,7 +170,7 @@ func TestEsIndexCleanerResources(t *testing.T) {
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{
Type: "elasticsearch",
Type: v1.JaegerESStorage,
EsIndexCleaner: v1.JaegerEsIndexCleanerSpec{
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: childResources,
Expand Down
12 changes: 6 additions & 6 deletions pkg/cronjob/es_rollover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,12 +189,12 @@ func TestEsRolloverResources(t *testing.T) {
expected corev1.ResourceRequirements
}{
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "elasticsearch"}}},
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage}}},
expected: corev1.ResourceRequirements{},
},
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{Type: "elasticsearch"},
Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage},
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: parentResources,
},
Expand All @@ -204,7 +204,7 @@ func TestEsRolloverResources(t *testing.T) {
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{
Type: "elasticsearch",
Type: v1.JaegerESStorage,
EsRollover: v1.JaegerEsRolloverSpec{
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: childResources,
Expand Down Expand Up @@ -292,12 +292,12 @@ func TestEsRolloverLookbackResources(t *testing.T) {
expected corev1.ResourceRequirements
}{
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "elasticsearch"}}},
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage}}},
expected: corev1.ResourceRequirements{},
},
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{Type: "elasticsearch"},
Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage},
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: parentResources,
},
Expand All @@ -307,7 +307,7 @@ func TestEsRolloverLookbackResources(t *testing.T) {
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{
Type: "elasticsearch",
Type: v1.JaegerESStorage,
EsRollover: v1.JaegerEsRolloverSpec{
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: childResources,
Expand Down
12 changes: 6 additions & 6 deletions pkg/cronjob/spark_dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/util"
)

var supportedStorageTypes = map[string]bool{"elasticsearch": true, "cassandra": true}
var supportedStorageTypes = map[v1.JaegerStorageType]bool{v1.JaegerESStorage: true, v1.JaegerCassandraStorage: true}

// SupportedStorage returns whether the given storage is supported
func SupportedStorage(storage string) bool {
return supportedStorageTypes[strings.ToLower(storage)]
func SupportedStorage(storage v1.JaegerStorageType) bool {
return supportedStorageTypes[storage]
}

// CreateSparkDependencies creates a new cronjob for the Spark Dependencies task
func CreateSparkDependencies(jaeger *v1.Jaeger) *batchv1beta1.CronJob {
logTLSNotSupported(jaeger)
envVars := []corev1.EnvVar{
{Name: "STORAGE", Value: jaeger.Spec.Storage.Type},
{Name: "STORAGE", Value: string(jaeger.Spec.Storage.Type)},
{Name: "SPARK_MASTER", Value: jaeger.Spec.Storage.Dependencies.SparkMaster},
{Name: "JAVA_OPTS", Value: jaeger.Spec.Storage.Dependencies.JavaOpts},
}
Expand Down Expand Up @@ -118,7 +118,7 @@ func CreateSparkDependencies(jaeger *v1.Jaeger) *batchv1beta1.CronJob {
func getStorageEnvs(s v1.JaegerStorageSpec) []corev1.EnvVar {
sFlagsMap := s.Options.Map()
switch s.Type {
case "cassandra":
case v1.JaegerCassandraStorage:
keyspace := sFlagsMap["cassandra.keyspace"]
if keyspace == "" {
keyspace = "jaeger_v1_test"
Expand All @@ -132,7 +132,7 @@ func getStorageEnvs(s v1.JaegerStorageSpec) []corev1.EnvVar {
{Name: "CASSANDRA_LOCAL_DC", Value: sFlagsMap["cassandra.local-dc"]},
{Name: "CASSANDRA_CLIENT_AUTH_ENABLED", Value: strconv.FormatBool(s.Dependencies.CassandraClientAuthEnabled)},
}
case "elasticsearch":
case v1.JaegerESStorage:
vars := []corev1.EnvVar{
{Name: "ES_NODES", Value: sFlagsMap["es.server-urls"]},
{Name: "ES_INDEX_PREFIX", Value: sFlagsMap["es.index-prefix"]},
Expand Down
18 changes: 9 additions & 9 deletions pkg/cronjob/spark_dependencies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestStorageEnvs(t *testing.T) {
expected []corev1.EnvVar
}{
{storage: v1.JaegerStorageSpec{Type: "foo"}},
{storage: v1.JaegerStorageSpec{Type: "cassandra",
{storage: v1.JaegerStorageSpec{Type: v1.JaegerCassandraStorage,
Options: v1.NewOptions(map[string]interface{}{"cassandra.servers": "lol:hol", "cassandra.keyspace": "haha",
"cassandra.username": "jdoe", "cassandra.password": "none"})},
expected: []corev1.EnvVar{
Expand All @@ -32,7 +32,7 @@ func TestStorageEnvs(t *testing.T) {
{Name: "CASSANDRA_LOCAL_DC", Value: ""},
{Name: "CASSANDRA_CLIENT_AUTH_ENABLED", Value: "false"},
}},
{storage: v1.JaegerStorageSpec{Type: "cassandra",
{storage: v1.JaegerStorageSpec{Type: v1.JaegerCassandraStorage,
Options: v1.NewOptions(map[string]interface{}{"cassandra.servers": "lol:hol", "cassandra.keyspace": "haha",
"cassandra.username": "jdoe", "cassandra.password": "none", "cassandra.tls": "ofcourse!", "cassandra.local-dc": "no-remote"})},
expected: []corev1.EnvVar{
Expand All @@ -44,7 +44,7 @@ func TestStorageEnvs(t *testing.T) {
{Name: "CASSANDRA_LOCAL_DC", Value: "no-remote"},
{Name: "CASSANDRA_CLIENT_AUTH_ENABLED", Value: "false"},
}},
{storage: v1.JaegerStorageSpec{Type: "elasticsearch",
{storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage,
Options: v1.NewOptions(map[string]interface{}{"es.server-urls": "lol:hol", "es.index-prefix": "haha",
"es.username": "jdoe", "es.password": "none"})},
expected: []corev1.EnvVar{
Expand All @@ -53,7 +53,7 @@ func TestStorageEnvs(t *testing.T) {
{Name: "ES_USERNAME", Value: "jdoe"},
{Name: "ES_PASSWORD", Value: "none"},
}},
{storage: v1.JaegerStorageSpec{Type: "elasticsearch",
{storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage,
Options: v1.NewOptions(map[string]interface{}{"es.server-urls": "lol:hol", "es.index-prefix": "haha",
"es.username": "jdoe", "es.password": "none"}),
Dependencies: v1.JaegerDependenciesSpec{ElasticsearchClientNodeOnly: &trueVar, ElasticsearchNodesWanOnly: &falseVar}},
Expand All @@ -73,7 +73,7 @@ func TestStorageEnvs(t *testing.T) {
}

func TestCreate(t *testing.T) {
assert.NotNil(t, CreateSparkDependencies(&v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "elasticsearch"}}}))
assert.NotNil(t, CreateSparkDependencies(&v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage}}}))
}

func TestSparkDependenciesSecrets(t *testing.T) {
Expand All @@ -90,7 +90,7 @@ func TestSparkDependenciesSecrets(t *testing.T) {
}

func TestSparkDependencies(t *testing.T) {
j := &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "elasticsearch"}}}
j := &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage}}}
historyLimits := int32(3)
j.Spec.Storage.Dependencies.SuccessfulJobsHistoryLimit = &historyLimits
cjob := CreateSparkDependencies(j)
Expand Down Expand Up @@ -168,12 +168,12 @@ func TestSparkDependenciesResources(t *testing.T) {
expected corev1.ResourceRequirements
}{
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: "elasticsearch"}}},
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage}}},
expected: corev1.ResourceRequirements{},
},
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{Type: "elasticsearch"},
Storage: v1.JaegerStorageSpec{Type: v1.JaegerESStorage},
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: parentResources,
},
Expand All @@ -183,7 +183,7 @@ func TestSparkDependenciesResources(t *testing.T) {
{
jaeger: &v1.Jaeger{Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{
Type: "elasticsearch",
Type: v1.JaegerESStorage,
Dependencies: v1.JaegerDependenciesSpec{
JaegerCommonSpec: v1.JaegerCommonSpec{
Resources: dependencyResources,
Expand Down
5 changes: 2 additions & 3 deletions pkg/deployment/all_in_one.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/config/tls"
configmap "github.com/jaegertracing/jaeger-operator/pkg/config/ui"
"github.com/jaegertracing/jaeger-operator/pkg/service"
"github.com/jaegertracing/jaeger-operator/pkg/storage"
"github.com/jaegertracing/jaeger-operator/pkg/util"
)

Expand Down Expand Up @@ -57,7 +56,7 @@ func (a *AllInOne) Get() *appsv1.Deployment {
commonSpec := util.Merge([]v1.JaegerCommonSpec{a.jaeger.Spec.AllInOne.JaegerCommonSpec, a.jaeger.Spec.JaegerCommonSpec, baseCommonSpec})

options := allArgs(a.jaeger.Spec.AllInOne.Options,
a.jaeger.Spec.Storage.Options.Filter(storage.OptionsPrefix(a.jaeger.Spec.Storage.Type)))
a.jaeger.Spec.Storage.Options.Filter(a.jaeger.Spec.Storage.Type.OptionsPrefix()))

configmap.Update(a.jaeger, commonSpec, &options)
sampling.Update(a.jaeger, commonSpec, &options)
Expand Down Expand Up @@ -135,7 +134,7 @@ func (a *AllInOne) Get() *appsv1.Deployment {
Env: []corev1.EnvVar{
{
Name: "SPAN_STORAGE_TYPE",
Value: a.jaeger.Spec.Storage.Type,
Value: string(a.jaeger.Spec.Storage.Type),
},
{
Name: "COLLECTOR_ZIPKIN_HTTP_PORT",
Expand Down
7 changes: 3 additions & 4 deletions pkg/deployment/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"github.com/jaegertracing/jaeger-operator/pkg/config/sampling"
"github.com/jaegertracing/jaeger-operator/pkg/config/tls"
"github.com/jaegertracing/jaeger-operator/pkg/service"
"github.com/jaegertracing/jaeger-operator/pkg/storage"
"github.com/jaegertracing/jaeger-operator/pkg/util"
)

Expand Down Expand Up @@ -72,10 +71,10 @@ func (c *Collector) Get() *appsv1.Deployment {
// If strategy is DeploymentStrategyStreaming, then change storage type
// to Kafka, and the storage options will be used in the Ingester instead
if c.jaeger.Spec.Strategy == v1.DeploymentStrategyStreaming {
storageType = "kafka"
storageType = v1.JaegerKafkaStorage
}
options := allArgs(c.jaeger.Spec.Collector.Options,
c.jaeger.Spec.Storage.Options.Filter(storage.OptionsPrefix(storageType)))
c.jaeger.Spec.Storage.Options.Filter(storageType.OptionsPrefix()))

sampling.Update(c.jaeger, commonSpec, &options)
tls.Update(c.jaeger, commonSpec, &options)
Expand Down Expand Up @@ -130,7 +129,7 @@ func (c *Collector) Get() *appsv1.Deployment {
Env: []corev1.EnvVar{
{
Name: "SPAN_STORAGE_TYPE",
Value: storageType,
Value: string(storageType),
},
{
Name: "COLLECTOR_ZIPKIN_HTTP_PORT",
Expand Down
4 changes: 2 additions & 2 deletions pkg/deployment/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func TestCollectorWithDirectStorageType(t *testing.T) {
},
Spec: v1.JaegerSpec{
Storage: v1.JaegerStorageSpec{
Type: "elasticsearch",
Type: v1.JaegerESStorage,
Options: v1.NewOptions(map[string]interface{}{
"es.server-urls": "http://somewhere",
}),
Expand All @@ -333,7 +333,7 @@ func TestCollectorWithDirectStorageType(t *testing.T) {
envvars := []corev1.EnvVar{
{
Name: "SPAN_STORAGE_TYPE",
Value: "elasticsearch",
Value: string(v1.JaegerESStorage),
},
{
Name: "COLLECTOR_ZIPKIN_HTTP_PORT",
Expand Down
Loading