Skip to content

Commit

Permalink
Automatically set num shards and replicas from referenced OCP ES (#1737)
Browse files Browse the repository at this point in the history
* Automatically set num shards and replicas from references OCP ES

Signed-off-by: Pavol Loffay <[email protected]>

* Fix ci

Signed-off-by: Pavol Loffay <[email protected]>

* fix rebase

Signed-off-by: Pavol Loffay <[email protected]>

* Install ES controller only if ES Kind is installed

Signed-off-by: Pavol Loffay <[email protected]>

* Reuse as much as possible

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Add test

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

* Fix

Signed-off-by: Pavol Loffay <[email protected]>

Co-authored-by: Ruben Vargas <[email protected]>
  • Loading branch information
pavolloffay and rubenvp8510 authored Feb 25, 2022
1 parent 9a4630a commit 6daf3ae
Show file tree
Hide file tree
Showing 17 changed files with 599 additions and 48 deletions.
3 changes: 1 addition & 2 deletions apis/v1/jaeger_types.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
package v1

import (
esv1 "github.com/openshift/elasticsearch-operator/apis/logging/v1"
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

esv1 "github.com/openshift/elasticsearch-operator/apis/logging/v1"
)

// IngressSecurityType represents the possible values for the security type
Expand Down
56 changes: 55 additions & 1 deletion apis/v1/jaeger_webhook.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
package v1

import (
"context"
"fmt"

esv1 "github.com/openshift/elasticsearch-operator/apis/logging/v1"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
"sigs.k8s.io/controller-runtime/pkg/webhook"
)
Expand All @@ -12,10 +19,14 @@ const (
)

// log is for logging in this package.
var jaegerlog = logf.Log.WithName("jaeger-resource")
var (
jaegerlog = logf.Log.WithName("jaeger-resource")
cl client.Client
)

// SetupWebhookWithManager adds Jaeger webook to the manager.
func (j *Jaeger) SetupWebhookWithManager(mgr ctrl.Manager) error {
cl = mgr.GetClient()
return ctrl.NewWebhookManagedBy(mgr).
For(j).
Complete()
Expand All @@ -32,6 +43,19 @@ func (j *Jaeger) Default() {
if j.Spec.Storage.Elasticsearch.Name == "" {
j.Spec.Storage.Elasticsearch.Name = defaultElasticsearchName
}

if ShouldInjectOpenShiftElasticsearchConfiguration(j.Spec.Storage) && j.Spec.Storage.Elasticsearch.DoNotProvision {
// check if ES instance exists
es := &esv1.Elasticsearch{}
err := cl.Get(context.Background(), types.NamespacedName{
Namespace: j.Namespace,
Name: j.Spec.Storage.Elasticsearch.Name,
}, es)
if errors.IsNotFound(err) {
return
}
j.Spec.Storage.Elasticsearch.NodeCount = OpenShiftElasticsearchNodeCount(es.Spec)
}
}

// TODO(user): change verbs to "verbs=create;update;delete" if you want to enable deletion validation.
Expand All @@ -48,6 +72,18 @@ func (j *Jaeger) ValidateCreate() error {
// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type
func (j *Jaeger) ValidateUpdate(_ runtime.Object) error {
jaegerlog.Info("validate update", "name", j.Name)

if ShouldInjectOpenShiftElasticsearchConfiguration(j.Spec.Storage) && j.Spec.Storage.Elasticsearch.DoNotProvision {
// check if ES instance exists
es := &esv1.Elasticsearch{}
err := cl.Get(context.Background(), types.NamespacedName{
Namespace: j.Namespace,
Name: j.Spec.Storage.Elasticsearch.Name,
}, es)
if errors.IsNotFound(err) {
return fmt.Errorf("elasticsearch instance not found: %v", err)
}
}
return nil
}

Expand All @@ -56,3 +92,21 @@ func (j *Jaeger) ValidateDelete() error {
jaegerlog.Info("validate delete", "name", j.Name)
return nil
}

// OpenShiftElasticsearchNodeCount returns total node count of Elasticsearch nodes.
func OpenShiftElasticsearchNodeCount(spec esv1.ElasticsearchSpec) int32 {
nodes := int32(0)
for i := 0; i < len(spec.Nodes); i++ {
nodes += spec.Nodes[i].NodeCount
}
return nodes
}

// ShouldInjectOpenShiftElasticsearchConfiguration returns true if OpenShift Elasticsearch is used and its configuration should be used.
func ShouldInjectOpenShiftElasticsearchConfiguration(s JaegerStorageSpec) bool {
if s.Type != JaegerESStorage {
return false
}
_, ok := s.Options.Map()["es.server-urls"]
return !ok
}
248 changes: 248 additions & 0 deletions apis/v1/jaeger_webhook_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,248 @@
package v1

import (
"fmt"
"testing"

esv1 "github.com/openshift/elasticsearch-operator/apis/logging/v1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/client-go/kubernetes/scheme"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

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

func TestDefault(t *testing.T) {
tests := []struct {
name string
objs []runtime.Object
j *Jaeger
expected *Jaeger
}{
{
name: "set missing ES name",
j: &Jaeger{
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Elasticsearch: ElasticsearchSpec{
Name: "",
},
},
},
},
expected: &Jaeger{
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Elasticsearch: ElasticsearchSpec{
Name: "elasticsearch",
},
},
},
},
},
{
name: "set ES node count",
objs: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "project1",
},
},
&esv1.Elasticsearch{
ObjectMeta: metav1.ObjectMeta{
Name: "my-es",
Namespace: "project1",
},
Spec: esv1.ElasticsearchSpec{
Nodes: []esv1.ElasticsearchNode{
{
NodeCount: 3,
},
},
},
},
},
j: &Jaeger{
ObjectMeta: metav1.ObjectMeta{
Namespace: "project1",
},
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Type: "elasticsearch",
Elasticsearch: ElasticsearchSpec{
Name: "my-es",
DoNotProvision: true,
},
},
},
},
expected: &Jaeger{
ObjectMeta: metav1.ObjectMeta{
Namespace: "project1",
},
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Type: "elasticsearch",
Elasticsearch: ElasticsearchSpec{
Name: "my-es",
NodeCount: 3,
DoNotProvision: true,
},
},
},
},
},
{
name: "do not set ES node count",
j: &Jaeger{
ObjectMeta: metav1.ObjectMeta{
Namespace: "project1",
},
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Type: "elasticsearch",
Elasticsearch: ElasticsearchSpec{
Name: "my-es",
DoNotProvision: false,
NodeCount: 1,
},
},
},
},
expected: &Jaeger{
ObjectMeta: metav1.ObjectMeta{
Namespace: "project1",
},
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Type: "elasticsearch",
Elasticsearch: ElasticsearchSpec{
Name: "my-es",
NodeCount: 1,
DoNotProvision: false,
},
},
},
},
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
esv1.AddToScheme(scheme.Scheme)
AddToScheme(scheme.Scheme)
fakeCl := fake.NewClientBuilder().WithRuntimeObjects(test.objs...).Build()
cl = fakeCl

test.j.Default()
assert.Equal(t, test.expected, test.j)
})
}
}

func TestValidate(t *testing.T) {
tests := []struct {
name string
objsToCreate []runtime.Object
current *Jaeger
err string
}{
{
name: "ES instance exists",
objsToCreate: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "project1",
},
},
&esv1.Elasticsearch{
ObjectMeta: metav1.ObjectMeta{
Name: "my-es",
Namespace: "project1",
},
Spec: esv1.ElasticsearchSpec{
Nodes: []esv1.ElasticsearchNode{
{
NodeCount: 3,
},
},
},
},
},
current: &Jaeger{
ObjectMeta: metav1.ObjectMeta{
Namespace: "project1",
},
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Type: "elasticsearch",
Elasticsearch: ElasticsearchSpec{
Name: "my-es",
DoNotProvision: true,
},
},
},
},
},
{
name: "ES instance does not exist",
objsToCreate: []runtime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: "project1",
},
},
},
current: &Jaeger{
ObjectMeta: metav1.ObjectMeta{
Namespace: "project1",
},
Spec: JaegerSpec{
Storage: JaegerStorageSpec{
Type: "elasticsearch",
Elasticsearch: ElasticsearchSpec{
Name: "my-es",
DoNotProvision: true,
},
},
},
},
err: `elasticsearch instance not found: elasticsearchs.logging.openshift.io "my-es" not found`,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
esv1.AddToScheme(scheme.Scheme)
AddToScheme(scheme.Scheme)
fakeCl := fake.NewClientBuilder().WithRuntimeObjects(test.objsToCreate...).Build()
cl = fakeCl

err := test.current.ValidateCreate()
if test.err != "" {
assert.Equal(t, test.err, err.Error())
} else {
assert.Nil(t, err)
}
})
}
}

func TestShouldDeployElasticsearch(t *testing.T) {
tests := []struct {
j JaegerStorageSpec
expected bool
}{
{j: JaegerStorageSpec{}},
{j: JaegerStorageSpec{Type: JaegerCassandraStorage}},
{j: JaegerStorageSpec{Type: JaegerESStorage, Options: NewOptions(map[string]interface{}{"es.server-urls": "foo"})}},
{j: JaegerStorageSpec{Type: JaegerESStorage}, expected: true},
}
for i, test := range tests {
t.Run(fmt.Sprintf("%d", i), func(t *testing.T) {
assert.Equal(t, test.expected, ShouldInjectOpenShiftElasticsearchConfiguration(test.j))
})
}
}
12 changes: 12 additions & 0 deletions bundle/manifests/jaeger-operator.clusterserviceversion.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,18 @@ spec:
- patch
- update
- watch
- apiGroups:
- logging.openshift.io
resources:
- elasticsearch
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- logging.openshift.io
resources:
Expand Down
12 changes: 12 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,18 @@ rules:
- patch
- update
- watch
- apiGroups:
- logging.openshift.io
resources:
- elasticsearch
verbs:
- create
- delete
- get
- list
- patch
- update
- watch
- apiGroups:
- logging.openshift.io
resources:
Expand Down
Loading

0 comments on commit 6daf3ae

Please sign in to comment.