Skip to content

Commit

Permalink
New DeploymentStrategy type for JaegerSpec.Strategy (#704)
Browse files Browse the repository at this point in the history
Signed-off-by: volmedo <[email protected]>
  • Loading branch information
volmedo authored and jpkrohling committed Oct 22, 2019
1 parent b986f39 commit 281ce62
Show file tree
Hide file tree
Showing 26 changed files with 177 additions and 90 deletions.
49 changes: 49 additions & 0 deletions pkg/apis/jaegertracing/v1/deployment_strategy.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package v1

import (
"errors"
"strings"
)

// DeploymentStrategy represents the possible values for deployment strategies
// +k8s:openapi-gen=true
type DeploymentStrategy string

const (
// DeploymentStrategyDeprecatedAllInOne represents the (deprecated) 'all-in-one' deployment strategy
// +k8s:openapi-gen=true
DeploymentStrategyDeprecatedAllInOne DeploymentStrategy = "all-in-one"

// DeploymentStrategyAllInOne represents the 'allInOne' deployment strategy (default)
// +k8s:openapi-gen=true
DeploymentStrategyAllInOne DeploymentStrategy = "allinone"

// DeploymentStrategyStreaming represents the 'streaming' deployment strategy
// +k8s:openapi-gen=true
DeploymentStrategyStreaming DeploymentStrategy = "streaming"

// DeploymentStrategyProduction represents the 'production' deployment strategy
// +k8s:openapi-gen=true
DeploymentStrategyProduction DeploymentStrategy = "production"
)

// UnmarshalText implements encoding.TextUnmarshaler to ensure that JSON values in the
// strategy field of JSON jaeger specs are interpreted in a case-insensitive manner
func (ds *DeploymentStrategy) UnmarshalText(text []byte) error {
if ds == nil {
return errors.New("DeploymentStrategy: UnmarshalText on nil pointer")
}

switch strings.ToLower(string(text)) {
default:
*ds = DeploymentStrategyAllInOne
case string(DeploymentStrategyDeprecatedAllInOne):
*ds = DeploymentStrategyDeprecatedAllInOne
case string(DeploymentStrategyStreaming):
*ds = DeploymentStrategyStreaming
case string(DeploymentStrategyProduction):
*ds = DeploymentStrategyProduction
}

return nil
}
55 changes: 55 additions & 0 deletions pkg/apis/jaegertracing/v1/deployment_strategy_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package v1

import (
"encoding/json"
"testing"

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

func TestUnmarshalJSON(t *testing.T) {
tcs := map[string]struct {
json string
expected DeploymentStrategy
}{
"allInOne": {json: `"allInOne"`, expected: DeploymentStrategyAllInOne},
"streaming": {json: `"streaming"`, expected: DeploymentStrategyStreaming},
"production": {json: `"production"`, expected: DeploymentStrategyProduction},
"all-in-one": {json: `"all-in-one"`, expected: DeploymentStrategyDeprecatedAllInOne},
"ALLinONE": {json: `"ALLinONE"`, expected: DeploymentStrategyAllInOne},
"StReAmInG": {json: `"StReAmInG"`, expected: DeploymentStrategyStreaming},
"Production": {json: `"Production"`, expected: DeploymentStrategyProduction},
"All-IN-One": {json: `"All-IN-One"`, expected: DeploymentStrategyDeprecatedAllInOne},
"random value": {json: `"random value"`, expected: DeploymentStrategyAllInOne},
"empty string": {json: `""`, expected: DeploymentStrategyAllInOne},
}

for name, tc := range tcs {
t.Run(name, func(t *testing.T) {
ds := DeploymentStrategy("")
err := json.Unmarshal([]byte(tc.json), &ds)
assert.NoError(t, err)
assert.Equal(t, tc.expected, ds)
})
}
}

func TestMarshalJSON(t *testing.T) {
tcs := map[string]struct {
strategy DeploymentStrategy
expected string
}{
"allinone": {strategy: DeploymentStrategyAllInOne, expected: `"allinone"`},
"streaming": {strategy: DeploymentStrategyStreaming, expected: `"streaming"`},
"production": {strategy: DeploymentStrategyProduction, expected: `"production"`},
"all-in-one": {strategy: DeploymentStrategyDeprecatedAllInOne, expected: `"all-in-one"`},
}

for name, tc := range tcs {
t.Run(name, func(t *testing.T) {
data, err := json.Marshal(tc.strategy)
assert.NoError(t, err)
assert.Equal(t, tc.expected, string(data))
})
}
}
2 changes: 1 addition & 1 deletion pkg/apis/jaegertracing/v1/jaeger_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const (
// +k8s:openapi-gen=true
type JaegerSpec struct {
// +optional
Strategy string `json:"strategy,omitempty"`
Strategy DeploymentStrategy `json:"strategy,omitempty"`

// +optional
AllInOne JaegerAllInOneSpec `json:"allInOne,omitempty"`
Expand Down
5 changes: 2 additions & 3 deletions pkg/deployment/collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"sort"
"strconv"
"strings"

"github.com/spf13/viper"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -76,9 +75,9 @@ func (c *Collector) Get() *appsv1.Deployment {
}

storageType := c.jaeger.Spec.Storage.Type
// If strategy is "streaming", then change storage type
// If strategy is DeploymentStrategyStreaming, then change storage type
// to Kafka, and the storage options will be used in the Ingester instead
if strings.EqualFold(c.jaeger.Spec.Strategy, "streaming") {
if c.jaeger.Spec.Strategy == v1.DeploymentStrategyStreaming {
storageType = "kafka"
}
options := allArgs(c.jaeger.Spec.Collector.Options,
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 @@ -377,7 +377,7 @@ func TestCollectorWithKafkaStorageType(t *testing.T) {
Name: "TestCollectorWithIngesterStorageType",
},
Spec: v1.JaegerSpec{
Strategy: "streaming",
Strategy: v1.DeploymentStrategyStreaming,
Collector: v1.JaegerCollectorSpec{
Options: v1.NewOptions(map[string]interface{}{
"kafka.producer.topic": "mytopic",
Expand Down Expand Up @@ -417,7 +417,7 @@ func TestCollectorWithIngesterNoOptionsStorageType(t *testing.T) {
Name: "TestCollectorWithIngesterNoOptionsStorageType",
},
Spec: v1.JaegerSpec{
Strategy: "streaming",
Strategy: v1.DeploymentStrategyStreaming,
Storage: v1.JaegerStorageSpec{
Type: "elasticsearch",
Options: v1.NewOptions(map[string]interface{}{
Expand Down
3 changes: 1 addition & 2 deletions pkg/deployment/ingester.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"sort"
"strconv"
"strings"

"github.com/spf13/viper"
appsv1 "k8s.io/api/apps/v1"
Expand Down Expand Up @@ -41,7 +40,7 @@ func NewIngester(jaeger *v1.Jaeger) *Ingester {

// Get returns a ingester pod
func (i *Ingester) Get() *appsv1.Deployment {
if !strings.EqualFold(i.jaeger.Spec.Strategy, "streaming") {
if i.jaeger.Spec.Strategy != v1.DeploymentStrategyStreaming {
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/deployment/ingester_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ func TestIngesterWithStorageType(t *testing.T) {
Name: "TestIngesterStorageType",
},
Spec: v1.JaegerSpec{
Strategy: "streaming",
Strategy: v1.DeploymentStrategyStreaming,
Ingester: v1.JaegerIngesterSpec{
Options: v1.NewOptions(map[string]interface{}{
"kafka.consumer.topic": "mytopic",
Expand Down Expand Up @@ -394,7 +394,7 @@ func newIngesterJaeger(name string) *v1.Jaeger {
Name: name,
},
Spec: v1.JaegerSpec{
Strategy: "streaming",
Strategy: v1.DeploymentStrategyStreaming,
Ingester: v1.JaegerIngesterSpec{
Options: v1.NewOptions(map[string]interface{}{
"any": "option",
Expand Down
7 changes: 3 additions & 4 deletions pkg/ingress/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ package ingress

import (
"fmt"
"strings"

extv1beta1 "k8s.io/api/extensions/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"

"github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
v1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
"github.com/jaegertracing/jaeger-operator/pkg/service"
"github.com/jaegertracing/jaeger-operator/pkg/util"
)
Expand Down Expand Up @@ -49,9 +48,9 @@ func (i *QueryIngress) Get() *extv1beta1.Ingress {
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.EqualFold(i.jaeger.Spec.Strategy, "allinone") {
if _, ok := i.jaeger.Spec.AllInOne.Options.Map()["query.base-path"]; ok && i.jaeger.Spec.Strategy == v1.DeploymentStrategyAllInOne {
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.EqualFold(i.jaeger.Spec.Strategy, "production") {
} else if _, ok := i.jaeger.Spec.Query.Options.Map()["query.base-path"]; ok && i.jaeger.Spec.Strategy == v1.DeploymentStrategyProduction {
spec.Rules = append(spec.Rules, getRule(i.jaeger.Spec.Query.Options, backend))
} else {
spec.Backend = &backend
Expand Down
4 changes: 2 additions & 2 deletions pkg/ingress/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func TestQueryIngressAllInOneBasePath(t *testing.T) {
name := "TestQueryIngressAllInOneBasePath"
jaeger := v1.NewJaeger(types.NamespacedName{Name: name})
jaeger.Spec.Ingress.Enabled = &enabled
jaeger.Spec.Strategy = "allInOne"
jaeger.Spec.Strategy = v1.DeploymentStrategyAllInOne
jaeger.Spec.AllInOne.Options = v1.NewOptions(map[string]interface{}{"query.base-path": "/jaeger"})
ingress := NewQueryIngress(jaeger)

Expand All @@ -68,7 +68,7 @@ func TestQueryIngressQueryBasePath(t *testing.T) {
name := "TestQueryIngressQueryBasePath"
jaeger := v1.NewJaeger(types.NamespacedName{Name: name})
jaeger.Spec.Ingress.Enabled = &enabled
jaeger.Spec.Strategy = "production"
jaeger.Spec.Strategy = v1.DeploymentStrategyProduction
jaeger.Spec.Query.Options = v1.NewOptions(map[string]interface{}{"query.base-path": "/jaeger"})
ingress := NewQueryIngress(jaeger)

Expand Down
2 changes: 1 addition & 1 deletion pkg/strategy/all_in_one.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
)

func newAllInOneStrategy(jaeger *v1.Jaeger) S {
c := S{typ: AllInOne}
c := S{typ: v1.DeploymentStrategyAllInOne}
jaeger.Logger().Debug("Creating all-in-one deployment")

dep := deployment.NewAllInOne(jaeger)
Expand Down
18 changes: 9 additions & 9 deletions pkg/strategy/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,19 @@ var (

// For returns the appropriate Strategy for the given Jaeger instance
func For(ctx context.Context, jaeger *v1.Jaeger, secrets []corev1.Secret) S {
if strings.EqualFold(jaeger.Spec.Strategy, "all-in-one") {
if jaeger.Spec.Strategy == v1.DeploymentStrategyDeprecatedAllInOne {
jaeger.Logger().Warn("Strategy 'all-in-one' is no longer supported, please use 'allInOne'")
jaeger.Spec.Strategy = "allInOne"
jaeger.Spec.Strategy = v1.DeploymentStrategyAllInOne
}

normalize(jaeger)

jaeger.Logger().WithField("strategy", jaeger.Spec.Strategy).Debug("Strategy chosen")
if strings.EqualFold(jaeger.Spec.Strategy, "allinone") {
if jaeger.Spec.Strategy == v1.DeploymentStrategyAllInOne {
return newAllInOneStrategy(jaeger)
}

if strings.EqualFold(jaeger.Spec.Strategy, "streaming") {
if jaeger.Spec.Strategy == v1.DeploymentStrategyStreaming {
return newStreamingStrategy(jaeger)
}

Expand Down Expand Up @@ -73,15 +73,15 @@ func normalize(jaeger *v1.Jaeger) {
}

// normalize the deployment strategy
if !strings.EqualFold(jaeger.Spec.Strategy, "production") && !strings.EqualFold(jaeger.Spec.Strategy, "streaming") {
jaeger.Spec.Strategy = "allInOne"
if jaeger.Spec.Strategy != v1.DeploymentStrategyProduction && jaeger.Spec.Strategy != v1.DeploymentStrategyStreaming {
jaeger.Spec.Strategy = v1.DeploymentStrategyAllInOne
}

// check for incompatible options
// if the storage is `memory`, then the only possible strategy is `all-in-one`
if !distributedStorage(jaeger.Spec.Storage.Type) && !strings.EqualFold(jaeger.Spec.Strategy, "allinone") {
jaeger.Logger().WithField("storage", jaeger.Spec.Storage.Type).Warn("No suitable storage provided. Falling back to all-in-one")
jaeger.Spec.Strategy = "allInOne"
if !distributedStorage(jaeger.Spec.Storage.Type) && jaeger.Spec.Strategy != v1.DeploymentStrategyAllInOne {
jaeger.Logger().WithField("storage", jaeger.Spec.Storage.Type).Warn("No suitable storage provided. Falling back to allInOne")
jaeger.Spec.Strategy = v1.DeploymentStrategyAllInOne
}

// we always set the value to None, except when we are on OpenShift *and* the user has not explicitly set to 'none'
Expand Down
32 changes: 16 additions & 16 deletions pkg/strategy/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,24 +16,24 @@ func TestNewControllerForAllInOneAsDefault(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestNewControllerForAllInOneAsDefault"})

ctrl := For(context.TODO(), jaeger, []corev1.Secret{})
assert.Equal(t, ctrl.Type(), AllInOne)
assert.Equal(t, ctrl.Type(), v1.DeploymentStrategyAllInOne)
}

func TestNewControllerForAllInOneAsExplicitValue(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestNewControllerForAllInOneAsExplicitValue"})
jaeger.Spec.Strategy = "ALL-IN-ONE" // same as 'all-in-one'
jaeger.Spec.Strategy = v1.DeploymentStrategyDeprecatedAllInOne // same as 'all-in-one'

ctrl := For(context.TODO(), jaeger, []corev1.Secret{})
assert.Equal(t, ctrl.Type(), AllInOne)
assert.Equal(t, ctrl.Type(), v1.DeploymentStrategyAllInOne)
}

func TestNewControllerForProduction(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestNewControllerForProduction"})
jaeger.Spec.Strategy = "production"
jaeger.Spec.Strategy = v1.DeploymentStrategyProduction
jaeger.Spec.Storage.Type = "elasticsearch"

ctrl := For(context.TODO(), jaeger, []corev1.Secret{})
assert.Equal(t, ctrl.Type(), Production)
assert.Equal(t, ctrl.Type(), v1.DeploymentStrategyProduction)
}

func TestUnknownStorage(t *testing.T) {
Expand All @@ -45,7 +45,7 @@ func TestUnknownStorage(t *testing.T) {

func TestElasticsearchAsStorageOptions(t *testing.T) {
jaeger := v1.NewJaeger(types.NamespacedName{Name: "TestElasticsearchAsStorageOptions"})
jaeger.Spec.Strategy = "production"
jaeger.Spec.Strategy = v1.DeploymentStrategyProduction
jaeger.Spec.Storage.Type = "elasticsearch"
jaeger.Spec.Storage.Options = v1.NewOptions(map[string]interface{}{
"es.server-urls": "http://elasticsearch-example-es-cluster:9200",
Expand Down Expand Up @@ -75,63 +75,63 @@ func TestDefaultName(t *testing.T) {
func TestIncompatibleMemoryStorageForProduction(t *testing.T) {
jaeger := &v1.Jaeger{
Spec: v1.JaegerSpec{
Strategy: "production",
Strategy: v1.DeploymentStrategyProduction,
Storage: v1.JaegerStorageSpec{
Type: "memory",
},
},
}
normalize(jaeger)
assert.Equal(t, "allInOne", jaeger.Spec.Strategy)
assert.Equal(t, v1.DeploymentStrategyAllInOne, jaeger.Spec.Strategy)
}

func TestIncompatibleBadgerStorageForProduction(t *testing.T) {
jaeger := &v1.Jaeger{
Spec: v1.JaegerSpec{
Strategy: "production",
Strategy: v1.DeploymentStrategyProduction,
Storage: v1.JaegerStorageSpec{
Type: "badger",
},
},
}
normalize(jaeger)
assert.Equal(t, "allInOne", jaeger.Spec.Strategy)
assert.Equal(t, v1.DeploymentStrategyAllInOne, jaeger.Spec.Strategy)
}

func TestIncompatibleStorageForStreaming(t *testing.T) {
jaeger := &v1.Jaeger{
Spec: v1.JaegerSpec{
Strategy: "streaming",
Strategy: v1.DeploymentStrategyStreaming,
Storage: v1.JaegerStorageSpec{
Type: "memory",
},
},
}
normalize(jaeger)
assert.Equal(t, "allInOne", jaeger.Spec.Strategy)
assert.Equal(t, v1.DeploymentStrategyAllInOne, jaeger.Spec.Strategy)
}

func TestDeprecatedAllInOneStrategy(t *testing.T) {
jaeger := &v1.Jaeger{
Spec: v1.JaegerSpec{
Strategy: "all-in-one",
Strategy: v1.DeploymentStrategyDeprecatedAllInOne,
},
}
For(context.TODO(), jaeger, []corev1.Secret{})
assert.Equal(t, "allInOne", jaeger.Spec.Strategy)
assert.Equal(t, v1.DeploymentStrategyAllInOne, jaeger.Spec.Strategy)
}

func TestStorageMemoryOnlyUsedWithAllInOneStrategy(t *testing.T) {
jaeger := &v1.Jaeger{
Spec: v1.JaegerSpec{
Strategy: "production",
Strategy: v1.DeploymentStrategyProduction,
Storage: v1.JaegerStorageSpec{
Type: "memory",
},
},
}
For(context.TODO(), jaeger, []corev1.Secret{})
assert.Equal(t, "allInOne", jaeger.Spec.Strategy)
assert.Equal(t, v1.DeploymentStrategyAllInOne, jaeger.Spec.Strategy)
}

func TestSetSecurityToNoneByDefault(t *testing.T) {
Expand Down
Loading

0 comments on commit 281ce62

Please sign in to comment.