Skip to content

Commit

Permalink
Change CRD to use lower camel case (#87)
Browse files Browse the repository at this point in the history
* Change CRD to use lower camel case

Signed-off-by: Gary Brown <[email protected]>

* Add warning for strategy all-in-one

Signed-off-by: Gary Brown <[email protected]>

* Add test for deprecated all-in-one strategy

Signed-off-by: Gary Brown <[email protected]>

* Fix for coverage

Signed-off-by: Gary Brown <[email protected]>

* Fix spelling

Signed-off-by: Gary Brown <[email protected]>

* Fix coverage again

Signed-off-by: Gary Brown <[email protected]>

* Make case insensitive and add extra test for memory and not allinone

Signed-off-by: Gary Brown <[email protected]>
  • Loading branch information
objectiser authored and jpkrohling committed Nov 7, 2018
1 parent 8891ff5 commit 61150a9
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 21 deletions.
14 changes: 7 additions & 7 deletions README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -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.
Expand Down Expand Up @@ -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`
Expand All @@ -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"
----
Expand Down
4 changes: 2 additions & 2 deletions deploy/examples/all-in-one-with-options.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion deploy/examples/disable-ingress.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@ metadata:
name: disable-ingress
spec:
ingress:
enabled: false
enabled: false
4 changes: 2 additions & 2 deletions pkg/apis/io/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,15 @@ type Controller interface {

// NewController build a new controller object for the given spec
func NewController(ctx context.Context, jaeger *v1alpha1.Jaeger) Controller {
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"
}

normalize(jaeger)

logrus.Debugf("Jaeger strategy: %s", jaeger.Spec.Strategy)
if jaeger.Spec.Strategy == "all-in-one" {
if strings.ToLower(jaeger.Spec.Strategy) == "allinone" {
return newAllInOneController(ctx, jaeger)
}

Expand Down Expand Up @@ -57,18 +62,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"
}
}

Expand Down
25 changes: 24 additions & 1 deletion pkg/controller/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,30 @@ func TestIncompatibleStorageForProduction(t *testing.T) {
},
}
normalize(jaeger)
assert.Equal(t, "all-in-one", jaeger.Spec.Strategy)
assert.Equal(t, "allInOne", jaeger.Spec.Strategy)
}

func TestDeprecatedAllInOneStrategy(t *testing.T) {
jaeger := &v1alpha1.Jaeger{
Spec: v1alpha1.JaegerSpec{
Strategy: "all-in-one",
},
}
NewController(context.TODO(), jaeger)
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 {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/all_in_one_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cassandra.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/daemonset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/sidecar.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}{
Expand Down

0 comments on commit 61150a9

Please sign in to comment.