Skip to content

Commit

Permalink
Elasticsearch: Set status.ObservedGeneration from metadata.Generation. (
Browse files Browse the repository at this point in the history
#5331)

* Add es.status.ObservedGeneration to ES spec.
Set observedGeneration from metadata.generation when generating initial state during ES reconcile.
Make observedGeneration optional in spec.
Adjust docs for observedGeneration.
Add .DS_Store to git ignore.
Add tests for observedGeneration behavior.
Add e2e test for observedGeneration.

* Move observedGeneration to be more alphabetical in struct.
Adjust wording for state during ES reconciliation, as it's not a pointer.

* Correct Spelling

* Update documentation in test

* Update test comments

* Update e2e test to just update annotations, not disable tls.

* remove unused ctx

* allow fake k8s client to be disable/enabled on demand

* directive `//nolint:gosimple` is unused for linter gosimple (nolintlint)

* Fix phantom lint error??

* no need for func in testing, just call DisableFailing

* Remove MacOS files from gitignore.
Update ObservedGeneration documentation/comments.
Remove duplicative license header.
Use named field in test struct.

* rename test file

* remove new build tag format from new e2e file.

* Removing changes associated with a failing k8s client.

* Update crd generation description.
Remove specific es generation test.
Add generation test to all es mutation tests.

* Correct runtime spelling.
Update final generation e2e test step to be wrapped in test.Eventually

* Use errors.New when not needing formatting.

* remove erroneous nolint

* s/CompareClusterGenerations/CompareClusterGenerationsSteps/g

* Fix indentation in state.go

* Adjust ES generation tests because of recent change concerning unknown status being set initially.
  • Loading branch information
naemono committed Feb 22, 2022
1 parent e1c6240 commit 168ea70
Show file tree
Hide file tree
Showing 9 changed files with 355 additions and 2 deletions.
9 changes: 9 additions & 0 deletions config/crds/v1/all-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4490,6 +4490,15 @@ spec:
single Association of a given type (for ex. single ES reference),
this map contains a single entry.
type: object
observedGeneration:
description: ObservedGeneration is the most recent generation observed
for this Elasticsearch cluster. It corresponds to the metadata generation,
which is updated on mutation by the API Server. If the generation
observed in status diverges from the generation in metadata, the
Elasticsearch controller has not yet processed the changes contained
in the Elasticsearch specification.
format: int64
type: integer
phase:
description: ElasticsearchOrchestrationPhase is the phase Elasticsearch
is in from the controller point of view.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9189,6 +9189,15 @@ spec:
single Association of a given type (for ex. single ES reference),
this map contains a single entry.
type: object
observedGeneration:
description: ObservedGeneration is the most recent generation observed
for this Elasticsearch cluster. It corresponds to the metadata generation,
which is updated on mutation by the API Server. If the generation
observed in status diverges from the generation in metadata, the
Elasticsearch controller has not yet processed the changes contained
in the Elasticsearch specification.
format: int64
type: integer
phase:
description: ElasticsearchOrchestrationPhase is the phase Elasticsearch
is in from the controller point of view.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4520,6 +4520,15 @@ spec:
single Association of a given type (for ex. single ES reference),
this map contains a single entry.
type: object
observedGeneration:
description: ObservedGeneration is the most recent generation observed
for this Elasticsearch cluster. It corresponds to the metadata generation,
which is updated on mutation by the API Server. If the generation
observed in status diverges from the generation in metadata, the
Elasticsearch controller has not yet processed the changes contained
in the Elasticsearch specification.
format: int64
type: integer
phase:
description: ElasticsearchOrchestrationPhase is the phase Elasticsearch
is in from the controller point of view.
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/elasticsearch/v1/elasticsearch_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,13 +437,20 @@ const (
type ElasticsearchStatus struct {
// AvailableNodes is the number of available instances.
AvailableNodes int32 `json:"availableNodes,omitempty"`

// Version of the stack resource currently running. During version upgrades, multiple versions may run
// in parallel: this value specifies the lowest version currently running.
Version string `json:"version,omitempty"`
Health ElasticsearchHealth `json:"health,omitempty"`
Phase ElasticsearchOrchestrationPhase `json:"phase,omitempty"`

MonitoringAssociationsStatus commonv1.AssociationStatusMap `json:"monitoringAssociationStatus,omitempty"`

// ObservedGeneration is the most recent generation observed for this Elasticsearch cluster.
// It corresponds to the metadata generation, which is updated on mutation by the API Server.
// If the generation observed in status diverges from the generation in metadata, the Elasticsearch
// controller has not yet processed the changes contained in the Elasticsearch specification.
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

type ZenDiscoveryStatus struct {
Expand Down
211 changes: 211 additions & 0 deletions pkg/controller/elasticsearch/elasticsearch_controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,211 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License 2.0;
// you may not use this file except in compliance with the Elastic License 2.0.
package elasticsearch

import (
"context"
"testing"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/tools/record"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

esv1 "github.com/elastic/cloud-on-k8s/pkg/apis/elasticsearch/v1"
"github.com/elastic/cloud-on-k8s/pkg/controller/common"
"github.com/elastic/cloud-on-k8s/pkg/controller/common/comparison"
"github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/hints"
"github.com/elastic/cloud-on-k8s/pkg/utils/k8s"
)

// newTestReconciler returns a ReconcileElasticsearch struct, allowing the internal k8s client to
// contain certain runtime objects.
func newTestReconciler(objects ...runtime.Object) *ReconcileElasticsearch {
r := &ReconcileElasticsearch{
Client: k8s.NewFakeClient(objects...),
recorder: record.NewFakeRecorder(100),
}
return r
}

// esBuilder allows for a cleaner way to build a testable elasticsearch spec,
// minimizing repetition.
type esBuilder struct {
es *esv1.Elasticsearch
}

// newBuilder returns a new elasticsearch test builder
// with given name/namespace combination.
func newBuilder(name, namespace string) *esBuilder {
return &esBuilder{
es: &esv1.Elasticsearch{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: namespace,
},
},
}
}

// WithAnnotations adds the given annotations to the ES object.
func (e *esBuilder) WithAnnotations(annotations map[string]string) *esBuilder {
e.es.ObjectMeta.Annotations = annotations
return e
}

// WithGeneration adds the metadata.generation to the ES object.
func (e *esBuilder) WithGeneration(generation int64) *esBuilder {
e.es.ObjectMeta.Generation = generation
return e
}

// WithStatus adds the status subresource to the ES object.
func (e *esBuilder) WithStatus(status esv1.ElasticsearchStatus) *esBuilder {
e.es.Status = status
return e
}

// WithVersion adds the ES version to the ES objects specification.
func (e *esBuilder) WithVersion(version string) *esBuilder {
e.es.Spec.Version = version
return e
}

// Build builds the final ES object and returns a pointer.
func (e *esBuilder) Build() *esv1.Elasticsearch {
return e.es
}

// BuildAndCopy builds the final ES object and returns a copy.
func (e *esBuilder) BuildAndCopy() esv1.Elasticsearch {
return *e.es
}

func TestReconcileElasticsearch_Reconcile(t *testing.T) {
type k8sClientFields struct {
objects []runtime.Object
}
type args struct {
request reconcile.Request
}
tests := []struct {
name string
k8sClientFields k8sClientFields
args args
wantErr bool
expected esv1.Elasticsearch
}{
{
name: "unmanaged ES has no error, and no observedGeneration update",
k8sClientFields: k8sClientFields{
[]runtime.Object{
newBuilder("testES", "test").
WithGeneration(2).
WithAnnotations(map[string]string{common.ManagedAnnotation: "false"}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 1}).Build()},
},
args: args{
request: reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "testES",
Namespace: "test",
},
},
},
wantErr: false,
expected: newBuilder("testES", "test").
WithGeneration(2).
WithAnnotations(map[string]string{common.ManagedAnnotation: "false"}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 1}).BuildAndCopy(),
},
{
name: "ES with too long name, fails initial reconcile, but has observedGeneration updated",
k8sClientFields: k8sClientFields{
[]runtime.Object{
newBuilder("testESwithtoolongofanamereallylongname", "test").
WithGeneration(2).
WithAnnotations(map[string]string{hints.OrchestrationsHintsAnnotation: `{"no_transient_settings":false}`}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 1}).Build(),
},
},
args: args{
request: reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "testESwithtoolongofanamereallylongname",
Namespace: "test",
},
},
},
wantErr: false,
expected: newBuilder("testESwithtoolongofanamereallylongname", "test").
WithGeneration(2).
WithAnnotations(map[string]string{hints.OrchestrationsHintsAnnotation: `{"no_transient_settings":false}`}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 2, Phase: esv1.ElasticsearchResourceInvalid, Health: esv1.ElasticsearchUnknownHealth}).BuildAndCopy(),
},
{
name: "ES with too long name, and needing annotations update, fails initial reconcile, and does not have status.* updated because of a 409/resource conflict",
k8sClientFields: k8sClientFields{
[]runtime.Object{
newBuilder("testESwithtoolongofanamereallylongname", "test").
WithGeneration(2).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 1}).Build()},
},
args: args{
request: reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "testESwithtoolongofanamereallylongname",
Namespace: "test",
},
},
},
wantErr: false,
expected: newBuilder("testESwithtoolongofanamereallylongname", "test").
WithGeneration(2).
WithAnnotations(map[string]string{hints.OrchestrationsHintsAnnotation: `{"no_transient_settings":false}`}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 1}).BuildAndCopy(),
},
{
name: "Invalid ES version errors, and updates observedGeneration",
k8sClientFields: k8sClientFields{
[]runtime.Object{
newBuilder("testES", "test").
WithGeneration(2).
WithVersion("invalid").
WithAnnotations(map[string]string{hints.OrchestrationsHintsAnnotation: `{"no_transient_settings":false}`}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 1}).Build()},
},
args: args{
request: reconcile.Request{
NamespacedName: types.NamespacedName{
Name: "testES",
Namespace: "test",
},
},
},
wantErr: false,
expected: newBuilder("testES", "test").
WithGeneration(2).
WithVersion("invalid").
WithAnnotations(map[string]string{hints.OrchestrationsHintsAnnotation: `{"no_transient_settings":false}`}).
WithStatus(esv1.ElasticsearchStatus{ObservedGeneration: 2, Phase: esv1.ElasticsearchResourceInvalid, Health: esv1.ElasticsearchUnknownHealth}).BuildAndCopy(),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
r := newTestReconciler(tt.k8sClientFields.objects...)
if _, err := r.Reconcile(context.Background(), tt.args.request); (err != nil) != tt.wantErr {
t.Errorf("ReconcileElasticsearch.Reconcile() error = %v, wantErr %v", err, tt.wantErr)
return
}

var actualES esv1.Elasticsearch
if err := r.Client.Get(context.Background(), tt.args.request.NamespacedName, &actualES); err != nil {
t.Error(err)
return
}
comparison.AssertEqual(t, &actualES, &tt.expected)
})
}
}
5 changes: 3 additions & 2 deletions pkg/controller/elasticsearch/reconcile/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ import (

var log = ulog.Log.WithName("elasticsearch-controller")

// State holds the accumulated state during the reconcile loop including the response and a pointer to an
// Elasticsearch resource for status updates.
// State holds the accumulated state during the reconcile loop including the response and a copy of the
// Elasticsearch resource from the start of reconciliation, for status updates.
type State struct {
*events.Recorder
cluster esv1.Elasticsearch
Expand All @@ -38,6 +38,7 @@ func NewState(c esv1.Elasticsearch) (*State, error) {
return nil, err
}
status := *c.Status.DeepCopy()
status.ObservedGeneration = c.Generation
// reset the health to 'unknown' so that if reconciliation fails before the observer has had a chance to get it,
// we stop reporting a health that may be out of date
status.Health = esv1.ElasticsearchUnknownHealth
Expand Down
18 changes: 18 additions & 0 deletions pkg/controller/elasticsearch/reconcile/state_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,24 @@ func TestState_Apply(t *testing.T) {
Phase: esv1.ElasticsearchApplyingChangesPhase,
},
},
{
name: "Status.observedGeneration is set from metadata.generation",
cluster: esv1.Elasticsearch{
ObjectMeta: metav1.ObjectMeta{
Generation: int64(1),
},
},
effects: func(s *State) {
s.UpdateElasticsearchApplyingChanges([]corev1.Pod{})
},
wantEvents: []events.Event{},
wantStatus: &esv1.ElasticsearchStatus{
ObservedGeneration: int64(1),
AvailableNodes: 0,
Health: esv1.ElasticsearchRedHealth,
Phase: esv1.ElasticsearchApplyingChangesPhase,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading

0 comments on commit 168ea70

Please sign in to comment.