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.

Sign up for GitHub

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

New DeploymentStrategy type for JaegerSpec.Strategy #704

Merged
merged 7 commits into from
Oct 22, 2019
Merged

New DeploymentStrategy type for JaegerSpec.Strategy #704

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4dcf62a
Add DeploymentStrategy type and apply to JaegerSpec.Strategy
volmedo Oct 16, 2019
5e43380
Add constants for DeploymentStrategy
volmedo Oct 17, 2019
5843126
Adapt code to the new type
volmedo Oct 17, 2019
cafa6ce
Complete comments
volmedo Oct 17, 2019
0a29840
Remove strategy type constants in pkg/strategy and use the newly defi…
volmedo Oct 21, 2019
a8850d6
Change package aliases to match convention
volmedo Oct 21, 2019
ae31e2b
Add unmarshaling code and tests
volmedo Oct 21, 2019
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 pkg/strategy/all_in_one.go
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions pkg/strategy/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ 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) {
Expand All @@ -33,7 +33,7 @@ func TestNewControllerForProduction(t *testing.T) {
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 Down
2 changes: 1 addition & 1 deletion pkg/strategy/production.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

func newProductionStrategy(jaeger *v1.Jaeger, es *storage.ElasticsearchDeployment) S {
c := S{typ: Production}
c := S{typ: v1.DeploymentStrategyProduction}
collector := deployment.NewCollector(jaeger)
query := deployment.NewQuery(jaeger)
agent := deployment.NewAgent(jaeger)
Expand Down
20 changes: 3 additions & 17 deletions pkg/strategy/strategy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import (
"k8s.io/api/extensions/v1beta1"
rbac "k8s.io/api/rbac/v1"

jv1 "github.com/jaegertracing/jaeger-operator/pkg/apis/jaegertracing/v1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically use v1 for Jaeger, and corev1 for k8s.io/api/core/v1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes a lot of sense!

I saw that v1 was already in use and invented a different alias for Jaeger. Didn't think about the possibility of renaming the existing one :). Will do it now.

esv1 "github.com/jaegertracing/jaeger-operator/pkg/storage/elasticsearch/v1"
)

// S knows what type of deployments to build based on a given spec
type S struct {
typ Type
typ jv1.DeploymentStrategy
accounts []v1.ServiceAccount
configMaps []v1.ConfigMap
cronJobs []batchv1beta1.CronJob
Expand All @@ -29,28 +30,13 @@ type S struct {
secrets []v1.Secret
}

// Type represents a specific deployment strategy, like 'all-in-one'
type Type string

const (

// AllInOne represents the 'all-in-one' deployment strategy
AllInOne Type = "allInOne"

// Production represents the 'production' deployment strategy
Production Type = "production"

// Streaming represents the 'streaming' deployment strategy
Streaming Type = "streaming"
)

// New constructs a new strategy from scratch
func New() *S {
return &S{}
}

// Type returns the strategy type for the given strategy
func (s S) Type() Type {
func (s S) Type() jv1.DeploymentStrategy {
return s.typ
}

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

func newStreamingStrategy(jaeger *v1.Jaeger) S {
c := S{typ: Streaming}
c := S{typ: v1.DeploymentStrategyStreaming}
collector := deployment.NewCollector(jaeger)
query := deployment.NewQuery(jaeger)
agent := deployment.NewAgent(jaeger)
Expand Down