From 102d584f8871b78c2996d24da44f4e765d83d966 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Fri, 12 Jul 2019 21:14:31 -0500 Subject: [PATCH 1/9] Add controller version to annotation --- operators/Gopkg.lock | 46 +++++++++- .../apmserver/apmserver_controller.go | 5 ++ .../common/annotation/controller_version.go | 42 +++++++++ .../annotation/controller_version_test.go | 86 +++++++++++++++++++ .../elasticsearch/elasticsearch_controller.go | 6 ++ .../controller/kibana/kibana_controller.go | 7 ++ 6 files changed, 189 insertions(+), 3 deletions(-) create mode 100644 operators/pkg/controller/common/annotation/controller_version.go create mode 100644 operators/pkg/controller/common/annotation/controller_version_test.go diff --git a/operators/Gopkg.lock b/operators/Gopkg.lock index 19a3d5a31b..3de60fcd82 100644 --- a/operators/Gopkg.lock +++ b/operators/Gopkg.lock @@ -857,6 +857,7 @@ "pkg/util/uuid", "pkg/util/validation", "pkg/util/validation/field", + "pkg/util/version", "pkg/util/wait", "pkg/util/yaml", "pkg/version", @@ -874,41 +875,75 @@ name = "k8s.io/client-go" packages = [ "discovery", + "discovery/fake", "dynamic", "kubernetes", + "kubernetes/fake", "kubernetes/scheme", "kubernetes/typed/admissionregistration/v1alpha1", + "kubernetes/typed/admissionregistration/v1alpha1/fake", "kubernetes/typed/admissionregistration/v1beta1", + "kubernetes/typed/admissionregistration/v1beta1/fake", "kubernetes/typed/apps/v1", + "kubernetes/typed/apps/v1/fake", "kubernetes/typed/apps/v1beta1", + "kubernetes/typed/apps/v1beta1/fake", "kubernetes/typed/apps/v1beta2", + "kubernetes/typed/apps/v1beta2/fake", "kubernetes/typed/auditregistration/v1alpha1", + "kubernetes/typed/auditregistration/v1alpha1/fake", "kubernetes/typed/authentication/v1", + "kubernetes/typed/authentication/v1/fake", "kubernetes/typed/authentication/v1beta1", + "kubernetes/typed/authentication/v1beta1/fake", "kubernetes/typed/authorization/v1", + "kubernetes/typed/authorization/v1/fake", "kubernetes/typed/authorization/v1beta1", + "kubernetes/typed/authorization/v1beta1/fake", "kubernetes/typed/autoscaling/v1", + "kubernetes/typed/autoscaling/v1/fake", "kubernetes/typed/autoscaling/v2beta1", + "kubernetes/typed/autoscaling/v2beta1/fake", "kubernetes/typed/autoscaling/v2beta2", + "kubernetes/typed/autoscaling/v2beta2/fake", "kubernetes/typed/batch/v1", + "kubernetes/typed/batch/v1/fake", "kubernetes/typed/batch/v1beta1", + "kubernetes/typed/batch/v1beta1/fake", "kubernetes/typed/batch/v2alpha1", + "kubernetes/typed/batch/v2alpha1/fake", "kubernetes/typed/certificates/v1beta1", + "kubernetes/typed/certificates/v1beta1/fake", "kubernetes/typed/coordination/v1beta1", + "kubernetes/typed/coordination/v1beta1/fake", "kubernetes/typed/core/v1", + "kubernetes/typed/core/v1/fake", "kubernetes/typed/events/v1beta1", + "kubernetes/typed/events/v1beta1/fake", "kubernetes/typed/extensions/v1beta1", + "kubernetes/typed/extensions/v1beta1/fake", "kubernetes/typed/networking/v1", + "kubernetes/typed/networking/v1/fake", "kubernetes/typed/policy/v1beta1", + "kubernetes/typed/policy/v1beta1/fake", "kubernetes/typed/rbac/v1", + "kubernetes/typed/rbac/v1/fake", "kubernetes/typed/rbac/v1alpha1", + "kubernetes/typed/rbac/v1alpha1/fake", "kubernetes/typed/rbac/v1beta1", + "kubernetes/typed/rbac/v1beta1/fake", "kubernetes/typed/scheduling/v1alpha1", + "kubernetes/typed/scheduling/v1alpha1/fake", "kubernetes/typed/scheduling/v1beta1", + "kubernetes/typed/scheduling/v1beta1/fake", "kubernetes/typed/settings/v1alpha1", + "kubernetes/typed/settings/v1alpha1/fake", "kubernetes/typed/storage/v1", + "kubernetes/typed/storage/v1/fake", "kubernetes/typed/storage/v1alpha1", + "kubernetes/typed/storage/v1alpha1/fake", "kubernetes/typed/storage/v1beta1", + "kubernetes/typed/storage/v1beta1/fake", "pkg/apis/clientauthentication", "pkg/apis/clientauthentication/v1alpha1", "pkg/apis/clientauthentication/v1beta1", @@ -1094,6 +1129,7 @@ analyzer-name = "dep" analyzer-version = 1 input-imports = [ + "github.com/davecgh/go-spew/spew", "github.com/elastic/go-ucfg", "github.com/elastic/go-ucfg/diff", "github.com/elastic/go-ucfg/yaml", @@ -1102,19 +1138,19 @@ "github.com/go-test/deep", "github.com/hashicorp/go-reap", "github.com/magiconair/properties/assert", - "github.com/onsi/gomega", "github.com/pkg/errors", "github.com/spf13/cobra", "github.com/spf13/viper", "github.com/stretchr/testify/assert", "github.com/stretchr/testify/require", "golang.org/x/crypto/bcrypt", - "golang.org/x/net/context", "gopkg.in/yaml.v2", "k8s.io/api/admission/v1beta1", "k8s.io/api/admissionregistration/v1beta1", "k8s.io/api/apps/v1", "k8s.io/api/core/v1", + "k8s.io/api/policy/v1beta1", + "k8s.io/api/storage/v1", "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1", "k8s.io/apimachinery/pkg/api/errors", "k8s.io/apimachinery/pkg/api/meta", @@ -1129,8 +1165,13 @@ "k8s.io/apimachinery/pkg/util/intstr", "k8s.io/apimachinery/pkg/util/rand", "k8s.io/apimachinery/pkg/util/uuid", + "k8s.io/apimachinery/pkg/util/version", "k8s.io/apimachinery/pkg/util/yaml", + "k8s.io/apimachinery/pkg/version", + "k8s.io/apimachinery/pkg/watch", + "k8s.io/client-go/discovery", "k8s.io/client-go/kubernetes", + "k8s.io/client-go/kubernetes/fake", "k8s.io/client-go/kubernetes/scheme", "k8s.io/client-go/plugin/pkg/client/auth/gcp", "k8s.io/client-go/rest", @@ -1163,7 +1204,6 @@ "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types", "sigs.k8s.io/controller-tools/cmd/controller-gen", "sigs.k8s.io/testing_frameworks/integration", - "sigs.k8s.io/yaml", ] solver-name = "gps-cdcl" solver-version = 1 diff --git a/operators/pkg/controller/apmserver/apmserver_controller.go b/operators/pkg/controller/apmserver/apmserver_controller.go index c80565a7be..8f1a45510f 100644 --- a/operators/pkg/controller/apmserver/apmserver_controller.go +++ b/operators/pkg/controller/apmserver/apmserver_controller.go @@ -17,6 +17,7 @@ import ( "github.com/elastic/cloud-on-k8s/operators/pkg/controller/apmserver/labels" apmname "github.com/elastic/cloud-on-k8s/operators/pkg/controller/apmserver/name" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/annotation" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/association/keystore" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/certificates" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/defaults" @@ -194,6 +195,10 @@ func (r *ReconcileApmServer) Reconcile(request reconcile.Request) (reconcile.Res state := NewState(request, as) state.UpdateApmServerControllerVersion(r.OperatorInfo.BuildInfo.Version) + err = annotation.UpdateControllerVersion(r.Client, as, r.OperatorInfo.BuildInfo.Version) + if err != nil { + return reconcile.Result{}, err + } state, err = r.reconcileApmServerDeployment(state, as) if err != nil { diff --git a/operators/pkg/controller/common/annotation/controller_version.go b/operators/pkg/controller/common/annotation/controller_version.go new file mode 100644 index 0000000000..2a3fe5e704 --- /dev/null +++ b/operators/pkg/controller/common/annotation/controller_version.go @@ -0,0 +1,42 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package annotation + +import ( + "github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" +) + +// ControllerVersionAnnotation is the annotation name that indicates the last controller version to update a resource +const ControllerVersionAnnotation = "k8s.elastic.co/controller-version" + +// UpdateControllerVersion updates the controller version annotation to the current version if necessary +func UpdateControllerVersion(client k8s.Client, obj runtime.Object, version string) error { + metaObject, err := meta.Accessor(obj) + if err != nil { + log.Error(err, "error converting runtime object to metav1 object") + return err + } + annotations := metaObject.GetAnnotations() + if annotations == nil { + annotations = make(map[string]string) + } + + // do not send extraneous update if the value would not change + if annotations[ControllerVersionAnnotation] == version { + return nil + } + + annotations[ControllerVersionAnnotation] = version + accessor := meta.NewAccessor() + err = accessor.SetAnnotations(obj, annotations) + if err != nil { + log.Error(err, "error updating controller version annotation", "namespace", metaObject.GetNamespace(), "name", metaObject.GetName(), "kind", obj.GetObjectKind()) + return err + } + log.V(1).Info("updating controller version annotation", "namespace", metaObject.GetNamespace(), "name", metaObject.GetName(), "kind", obj.GetObjectKind()) + return client.Update(obj) +} diff --git a/operators/pkg/controller/common/annotation/controller_version_test.go b/operators/pkg/controller/common/annotation/controller_version_test.go new file mode 100644 index 0000000000..abf43b779a --- /dev/null +++ b/operators/pkg/controller/common/annotation/controller_version_test.go @@ -0,0 +1,86 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package annotation + +import ( + "testing" + + kibanav1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/kibana/v1alpha1" + "github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s" + + // "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + apmtype "github.com/elastic/cloud-on-k8s/operators/pkg/apis/apm/v1alpha1" + assoctype "github.com/elastic/cloud-on-k8s/operators/pkg/apis/associations/v1alpha1" + estype "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" + "k8s.io/client-go/kubernetes/scheme" +) + +// Test UpdateControllerVersion updates annotation if there is an older version +func TestAnnotationUpdated(t *testing.T) { + kibana := kibanav1alpha1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "kibana", + Annotations: map[string]string{ + ControllerVersionAnnotation: "oldversion", + }, + }, + } + obj := kibana.DeepCopy() + sc := setupScheme(t) + client := k8s.WrapClient(fake.NewFakeClientWithScheme(sc, obj)) + err := UpdateControllerVersion(client, obj, "newversion") + require.NoError(t, err) + require.Equal(t, obj.GetAnnotations()[ControllerVersionAnnotation], "newversion") +} + +// Test UpdateControllerVersion creates an annotation even if there are no current annotations +func TestAnnotationCreated(t *testing.T) { + kibana := kibanav1alpha1.Kibana{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "ns", + Name: "kibana", + }, + } + + obj := kibana.DeepCopy() + sc := setupScheme(t) + client := k8s.WrapClient(fake.NewFakeClientWithScheme(sc, obj)) + err := UpdateControllerVersion(client, obj, "newversion") + actualKibana := &kibanav1alpha1.Kibana{} + err = client.Get(types.NamespacedName{ + Namespace: obj.Namespace, + Name: obj.Name, + }, actualKibana) + require.NoError(t, err) + require.NotNil(t, actualKibana.GetAnnotations()) + assert.Equal(t, actualKibana.GetAnnotations()[ControllerVersionAnnotation], "newversion") +} + +// setupScheme creates a scheme to use for our fake clients so they know about our custom resources +// TODO move this into one of the upper level common packages and make public, refactor out this code that's in a lot of our tests +func setupScheme(t *testing.T) *runtime.Scheme { + sc := scheme.Scheme + if err := assoctype.SchemeBuilder.AddToScheme(sc); err != nil { + assert.Fail(t, "failed to add Association types") + } + if err := apmtype.SchemeBuilder.AddToScheme(sc); err != nil { + assert.Fail(t, "failed to add APM types") + } + if err := estype.SchemeBuilder.AddToScheme(sc); err != nil { + assert.Fail(t, "failed to add ES types") + } + if err := kibanav1alpha1.SchemeBuilder.AddToScheme(sc); err != nil { + assert.Fail(t, "failed to add Kibana types") + } + return sc +} diff --git a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go index 48a37fdbab..4451bb741f 100644 --- a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go +++ b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go @@ -10,6 +10,7 @@ import ( elasticsearchv1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/elasticsearch/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/annotation" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/certificates/http" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/finalizer" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/operator" @@ -195,6 +196,11 @@ func (r *ReconcileElasticsearch) Reconcile(request reconcile.Request) (reconcile state := esreconcile.NewState(es) state.UpdateElasticsearchControllerVersion(r.OperatorInfo.BuildInfo.Version) + err = annotation.UpdateControllerVersion(r.Client, &es, r.OperatorInfo.BuildInfo.Version) + if err != nil { + return reconcile.Result{}, err + } + results := r.internalReconcile(es, state) err = r.updateStatus(es, state) if err != nil && apierrors.IsConflict(err) { diff --git a/operators/pkg/controller/kibana/kibana_controller.go b/operators/pkg/controller/kibana/kibana_controller.go index 64d851d6cc..17f29b29c9 100644 --- a/operators/pkg/controller/kibana/kibana_controller.go +++ b/operators/pkg/controller/kibana/kibana_controller.go @@ -11,6 +11,7 @@ import ( kibanav1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/kibana/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common" + "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/annotation" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/association/keystore" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/events" "github.com/elastic/cloud-on-k8s/operators/pkg/controller/common/finalizer" @@ -172,12 +173,18 @@ func (r *ReconcileKibana) Reconcile(request reconcile.Request) (reconcile.Result } state := NewState(request, kb) state.UpdateKibanaControllerVersion(r.params.OperatorInfo.BuildInfo.Version) + err = annotation.UpdateControllerVersion(r.Client, kb, r.params.OperatorInfo.BuildInfo.Version) + if err != nil { + return reconcile.Result{}, err + } + driver, err := newDriver(r, r.scheme, *ver, r.dynamicWatches, r.recorder) if err != nil { return reconcile.Result{}, err } // version specific reconcile results := driver.Reconcile(&state, kb, r.params) + // update status err = r.updateStatus(state) if err != nil && errors.IsConflict(err) { From bbe0110326a9476376ac25e81106e30638f1395a Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Fri, 12 Jul 2019 21:24:02 -0500 Subject: [PATCH 2/9] Revert controller version status field --- operators/config/crds/apm_v1alpha1_apmserver.yaml | 4 ---- .../crds/elasticsearch_v1alpha1_elasticsearch.yaml | 4 ---- operators/config/crds/kibana_v1alpha1_kibana.yaml | 4 ---- operators/pkg/apis/apm/v1alpha1/apmserver_types.go | 2 -- .../apis/elasticsearch/v1alpha1/elasticsearch_types.go | 2 -- operators/pkg/apis/kibana/v1alpha1/kibana_types.go | 2 -- .../pkg/controller/apmserver/apmserver_controller.go | 3 +-- operators/pkg/controller/apmserver/state.go | 8 -------- .../elasticsearch/elasticsearch_controller.go | 3 +-- .../pkg/controller/elasticsearch/reconcile/state.go | 10 ---------- operators/pkg/controller/kibana/kibana_controller.go | 4 ++-- operators/pkg/controller/kibana/state.go | 10 ---------- 12 files changed, 4 insertions(+), 52 deletions(-) diff --git a/operators/config/crds/apm_v1alpha1_apmserver.yaml b/operators/config/crds/apm_v1alpha1_apmserver.yaml index deee499398..aa93cc5473 100644 --- a/operators/config/crds/apm_v1alpha1_apmserver.yaml +++ b/operators/config/crds/apm_v1alpha1_apmserver.yaml @@ -195,10 +195,6 @@ spec: type: object status: properties: - controllerVersion: - description: ControllerVersion is the version of the controller that - last updated the ApmServer instance - type: string health: type: string secretTokenSecret: diff --git a/operators/config/crds/elasticsearch_v1alpha1_elasticsearch.yaml b/operators/config/crds/elasticsearch_v1alpha1_elasticsearch.yaml index 388d5be7a8..377dab42f5 100644 --- a/operators/config/crds/elasticsearch_v1alpha1_elasticsearch.yaml +++ b/operators/config/crds/elasticsearch_v1alpha1_elasticsearch.yaml @@ -244,10 +244,6 @@ spec: properties: clusterUUID: type: string - controllerVersion: - description: ControllerVersion is the version of the controller that - last updated the Elasticsearch cluster - type: string health: type: string masterNode: diff --git a/operators/config/crds/kibana_v1alpha1_kibana.yaml b/operators/config/crds/kibana_v1alpha1_kibana.yaml index 77750a3464..963f05f61f 100644 --- a/operators/config/crds/kibana_v1alpha1_kibana.yaml +++ b/operators/config/crds/kibana_v1alpha1_kibana.yaml @@ -189,10 +189,6 @@ spec: properties: associationStatus: type: string - controllerVersion: - description: ControllerVersion is the version of the controller that - last updated the Kibana instance - type: string health: type: string type: object diff --git a/operators/pkg/apis/apm/v1alpha1/apmserver_types.go b/operators/pkg/apis/apm/v1alpha1/apmserver_types.go index 150ab0e4a0..e695ce42e9 100644 --- a/operators/pkg/apis/apm/v1alpha1/apmserver_types.go +++ b/operators/pkg/apis/apm/v1alpha1/apmserver_types.go @@ -98,8 +98,6 @@ type ApmServerStatus struct { SecretTokenSecretName string `json:"secretTokenSecret,omitempty"` // Association is the status of any auto-linking to Elasticsearch clusters. Association commonv1alpha1.AssociationStatus - // ControllerVersion is the version of the controller that last updated the ApmServer instance - ControllerVersion string `json:"controllerVersion,omitempty"` } // IsDegraded returns true if the current status is worse than the previous. diff --git a/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go b/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go index 033e1e1abf..b2fc3b88ed 100644 --- a/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go +++ b/operators/pkg/apis/elasticsearch/v1alpha1/elasticsearch_types.go @@ -219,8 +219,6 @@ type ElasticsearchStatus struct { MasterNode string `json:"masterNode,omitempty"` ExternalService string `json:"service,omitempty"` ZenDiscovery ZenDiscoveryStatus `json:"zenDiscovery,omitempty"` - // ControllerVersion is the version of the controller that last updated the Elasticsearch cluster - ControllerVersion string `json:"controllerVersion,omitempty"` } type ZenDiscoveryStatus struct { diff --git a/operators/pkg/apis/kibana/v1alpha1/kibana_types.go b/operators/pkg/apis/kibana/v1alpha1/kibana_types.go index 7f3c4de325..3f5927bd47 100644 --- a/operators/pkg/apis/kibana/v1alpha1/kibana_types.go +++ b/operators/pkg/apis/kibana/v1alpha1/kibana_types.go @@ -87,8 +87,6 @@ type KibanaStatus struct { commonv1alpha1.ReconcilerStatus Health KibanaHealth `json:"health,omitempty"` AssociationStatus commonv1alpha1.AssociationStatus `json:"associationStatus,omitempty"` - // ControllerVersion is the version of the controller that last updated the Kibana instance - ControllerVersion string `json:"controllerVersion,omitempty"` } // IsDegraded returns true if the current status is worse than the previous. diff --git a/operators/pkg/controller/apmserver/apmserver_controller.go b/operators/pkg/controller/apmserver/apmserver_controller.go index 8f1a45510f..c92caf4d71 100644 --- a/operators/pkg/controller/apmserver/apmserver_controller.go +++ b/operators/pkg/controller/apmserver/apmserver_controller.go @@ -193,13 +193,12 @@ func (r *ReconcileApmServer) Reconcile(request reconcile.Request) (reconcile.Res return reconcile.Result{}, nil } - state := NewState(request, as) - state.UpdateApmServerControllerVersion(r.OperatorInfo.BuildInfo.Version) err = annotation.UpdateControllerVersion(r.Client, as, r.OperatorInfo.BuildInfo.Version) if err != nil { return reconcile.Result{}, err } + state := NewState(request, as) state, err = r.reconcileApmServerDeployment(state, as) if err != nil { if errors.IsConflict(err) { diff --git a/operators/pkg/controller/apmserver/state.go b/operators/pkg/controller/apmserver/state.go index 64ba2f9fb5..f6b82273ea 100644 --- a/operators/pkg/controller/apmserver/state.go +++ b/operators/pkg/controller/apmserver/state.go @@ -43,11 +43,3 @@ func (s State) UpdateApmServerState(deployment v1.Deployment, apmServerSecret co func (s State) UpdateApmServerExternalService(svc corev1.Service) { s.ApmServer.Status.ExternalService = svc.Name } - -func (s *State) UpdateApmServerControllerVersion(version string) { - s.ApmServer.Status.ControllerVersion = version -} - -func (s *State) GetApmServerControllerVersion() string { - return s.ApmServer.Status.ControllerVersion -} diff --git a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go index 4451bb741f..d89a025a18 100644 --- a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go +++ b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go @@ -194,13 +194,12 @@ func (r *ReconcileElasticsearch) Reconcile(request reconcile.Request) (reconcile return common.PauseRequeue, nil } - state := esreconcile.NewState(es) - state.UpdateElasticsearchControllerVersion(r.OperatorInfo.BuildInfo.Version) err = annotation.UpdateControllerVersion(r.Client, &es, r.OperatorInfo.BuildInfo.Version) if err != nil { return reconcile.Result{}, err } + state := esreconcile.NewState(es) results := r.internalReconcile(es, state) err = r.updateStatus(es, state) if err != nil && apierrors.IsConflict(err) { diff --git a/operators/pkg/controller/elasticsearch/reconcile/state.go b/operators/pkg/controller/elasticsearch/reconcile/state.go index 85c5bfc738..b02db91547 100644 --- a/operators/pkg/controller/elasticsearch/reconcile/state.go +++ b/operators/pkg/controller/elasticsearch/reconcile/state.go @@ -157,13 +157,3 @@ func (s *State) UpdateElasticsearchInvalid(results []validation.Result) { s.AddEvent(corev1.EventTypeWarning, events.EventReasonValidation, r.Reason) } } - -// UpdateElasticsearchControllerVersion sets the elasticsearch controller version that last updated the ES cluster -func (s *State) UpdateElasticsearchControllerVersion(version string) { - s.status.ControllerVersion = version -} - -// GetElasticsearchControllerVersion returns the elasticsearch controller version that last updated the ES cluster -func (s *State) GetElasticsearchControllerVersion() string { - return s.status.ControllerVersion -} diff --git a/operators/pkg/controller/kibana/kibana_controller.go b/operators/pkg/controller/kibana/kibana_controller.go index 17f29b29c9..8c4e6e0b67 100644 --- a/operators/pkg/controller/kibana/kibana_controller.go +++ b/operators/pkg/controller/kibana/kibana_controller.go @@ -171,13 +171,13 @@ func (r *ReconcileKibana) Reconcile(request reconcile.Request) (reconcile.Result if err != nil { return reconcile.Result{}, err } - state := NewState(request, kb) - state.UpdateKibanaControllerVersion(r.params.OperatorInfo.BuildInfo.Version) + err = annotation.UpdateControllerVersion(r.Client, kb, r.params.OperatorInfo.BuildInfo.Version) if err != nil { return reconcile.Result{}, err } + state := NewState(request, kb) driver, err := newDriver(r, r.scheme, *ver, r.dynamicWatches, r.recorder) if err != nil { return reconcile.Result{}, err diff --git a/operators/pkg/controller/kibana/state.go b/operators/pkg/controller/kibana/state.go index f0d66fc722..072628d0a1 100644 --- a/operators/pkg/controller/kibana/state.go +++ b/operators/pkg/controller/kibana/state.go @@ -36,13 +36,3 @@ func (s State) UpdateKibanaState(deployment v1.Deployment) { } } } - -// UpdateKibanaControllerVersion updates the Kibana status with the controller version that last updated the Kibana instance -func (s *State) UpdateKibanaControllerVersion(version string) { - s.Kibana.Status.ControllerVersion = version -} - -// GetKibanaControllerVersion returns the controller version that last updated the Kibana instance -func (s *State) GetKibanaControllerVersion() string { - return s.Kibana.Status.ControllerVersion -} From f9df0a12e9c15a47400bc8cf46aefb60c44f7699 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Fri, 12 Jul 2019 21:29:45 -0500 Subject: [PATCH 3/9] Remove comment --- .../pkg/controller/common/annotation/controller_version_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/operators/pkg/controller/common/annotation/controller_version_test.go b/operators/pkg/controller/common/annotation/controller_version_test.go index abf43b779a..2fa6db2f9a 100644 --- a/operators/pkg/controller/common/annotation/controller_version_test.go +++ b/operators/pkg/controller/common/annotation/controller_version_test.go @@ -10,7 +10,6 @@ import ( kibanav1alpha1 "github.com/elastic/cloud-on-k8s/operators/pkg/apis/kibana/v1alpha1" "github.com/elastic/cloud-on-k8s/operators/pkg/utils/k8s" - // "github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" From d548ccb3eff239d7433b0e87ff62304a23334d35 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Mon, 15 Jul 2019 12:07:45 -0500 Subject: [PATCH 4/9] PR comments --- .../common/annotation/controller_version.go | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/operators/pkg/controller/common/annotation/controller_version.go b/operators/pkg/controller/common/annotation/controller_version.go index 2a3fe5e704..8261d72538 100644 --- a/operators/pkg/controller/common/annotation/controller_version.go +++ b/operators/pkg/controller/common/annotation/controller_version.go @@ -11,32 +11,41 @@ import ( ) // ControllerVersionAnnotation is the annotation name that indicates the last controller version to update a resource -const ControllerVersionAnnotation = "k8s.elastic.co/controller-version" +const ControllerVersionAnnotation = "common.k8s.elastic.co/controller-version" // UpdateControllerVersion updates the controller version annotation to the current version if necessary func UpdateControllerVersion(client k8s.Client, obj runtime.Object, version string) error { - metaObject, err := meta.Accessor(obj) + accessor := meta.NewAccessor() + namespace, err := accessor.Namespace(obj) + if err != nil { + log.Error(err, "error getting namespace", "kind", obj.GetObjectKind().GroupVersionKind().Kind) + return err + } + name, err := accessor.Name(obj) if err != nil { - log.Error(err, "error converting runtime object to metav1 object") + log.Error(err, "error getting name", "namespace", namespace, "kind", obj.GetObjectKind().GroupVersionKind().Kind) + return err + } + annotations, err := accessor.Annotations(obj) + if err != nil { + log.Error(err, "error getting annotations", "namespace", namespace, "name", name, "kind", obj.GetObjectKind().GroupVersionKind().Kind) return err } - annotations := metaObject.GetAnnotations() if annotations == nil { annotations = make(map[string]string) } - // do not send extraneous update if the value would not change + // do not send unnecessary update if the value would not change if annotations[ControllerVersionAnnotation] == version { return nil } annotations[ControllerVersionAnnotation] = version - accessor := meta.NewAccessor() err = accessor.SetAnnotations(obj, annotations) if err != nil { - log.Error(err, "error updating controller version annotation", "namespace", metaObject.GetNamespace(), "name", metaObject.GetName(), "kind", obj.GetObjectKind()) + log.Error(err, "error updating controller version annotation", "namespace", namespace, "name", name, "kind", obj.GetObjectKind()) return err } - log.V(1).Info("updating controller version annotation", "namespace", metaObject.GetNamespace(), "name", metaObject.GetName(), "kind", obj.GetObjectKind()) + log.V(1).Info("updating controller version annotation", "namespace", namespace, "name", name, "kind", obj.GetObjectKind()) return client.Update(obj) } From 82f72e29defe7fc128d4102e7b1ec14848528de4 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Tue, 16 Jul 2019 12:17:38 -0500 Subject: [PATCH 5/9] Update dep locks --- operators/Gopkg.lock | 9 --------- 1 file changed, 9 deletions(-) diff --git a/operators/Gopkg.lock b/operators/Gopkg.lock index 3de60fcd82..bca5be2632 100644 --- a/operators/Gopkg.lock +++ b/operators/Gopkg.lock @@ -204,14 +204,6 @@ pruneopts = "T" revision = "c63ab54fda8f77302f8d414e19933f2b6026a089" -[[projects]] - branch = "master" - digest = "1:93cde32e6a4f5911b86cdc7a0092f9a775fa228d6afce033b5cb575700cdca19" - name = "github.com/hashicorp/go-reap" - packages = ["."] - pruneopts = "T" - revision = "bf58d8a43e7b6bf026d99d6295c4de703b67b374" - [[projects]] digest = "1:8ec8d88c248041a6df5f6574b87bc00e7e0b493881dad2e7ef47b11dc69093b5" name = "github.com/hashicorp/golang-lru" @@ -1136,7 +1128,6 @@ "github.com/emicklei/go-restful", "github.com/ghodss/yaml", "github.com/go-test/deep", - "github.com/hashicorp/go-reap", "github.com/magiconair/properties/assert", "github.com/pkg/errors", "github.com/spf13/cobra", From a8b1746ed266491b58c9b946b18b5b29e9a8a9b6 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Tue, 16 Jul 2019 16:36:55 -0500 Subject: [PATCH 6/9] Fix lint --- .../pkg/controller/common/annotation/controller_version_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/operators/pkg/controller/common/annotation/controller_version_test.go b/operators/pkg/controller/common/annotation/controller_version_test.go index 2fa6db2f9a..d844e25a2a 100644 --- a/operators/pkg/controller/common/annotation/controller_version_test.go +++ b/operators/pkg/controller/common/annotation/controller_version_test.go @@ -55,6 +55,7 @@ func TestAnnotationCreated(t *testing.T) { sc := setupScheme(t) client := k8s.WrapClient(fake.NewFakeClientWithScheme(sc, obj)) err := UpdateControllerVersion(client, obj, "newversion") + require.NoError(t, err) actualKibana := &kibanav1alpha1.Kibana{} err = client.Get(types.NamespacedName{ Namespace: obj.Namespace, From 1cb82b6c1587bd1fe8670b901e11cc28e88b62e0 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Wed, 17 Jul 2019 22:24:51 -0500 Subject: [PATCH 7/9] Wrap e2e errors --- operators/test/e2e/test/apmserver/steps_deletion.go | 2 +- operators/test/e2e/test/elasticsearch/steps_deletion.go | 2 +- operators/test/e2e/test/kibana/steps_deletion.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operators/test/e2e/test/apmserver/steps_deletion.go b/operators/test/e2e/test/apmserver/steps_deletion.go index 293ce729d6..65c531126a 100644 --- a/operators/test/e2e/test/apmserver/steps_deletion.go +++ b/operators/test/e2e/test/apmserver/steps_deletion.go @@ -41,7 +41,7 @@ func (b Builder) DeletionTestSteps(k *test.K8sClient) test.StepList { continue } } - return errors.New("expected 404 not found API error here") + return errors.Wrap(err, "expected 404 not found API error here") } return nil diff --git a/operators/test/e2e/test/elasticsearch/steps_deletion.go b/operators/test/e2e/test/elasticsearch/steps_deletion.go index 1c871e3a83..99467d3669 100644 --- a/operators/test/e2e/test/elasticsearch/steps_deletion.go +++ b/operators/test/e2e/test/elasticsearch/steps_deletion.go @@ -42,7 +42,7 @@ func (b Builder) DeletionTestSteps(k *test.K8sClient) test.StepList { continue } } - return errors.New("expected 404 not found API error here") + return errors.Wrap(err, "expected 404 not found API error here") } return nil diff --git a/operators/test/e2e/test/kibana/steps_deletion.go b/operators/test/e2e/test/kibana/steps_deletion.go index 863eadd1dd..2f95b9aebd 100644 --- a/operators/test/e2e/test/kibana/steps_deletion.go +++ b/operators/test/e2e/test/kibana/steps_deletion.go @@ -42,7 +42,7 @@ func (b Builder) DeletionTestSteps(k *test.K8sClient) test.StepList { continue } } - return errors.New("expected 404 not found API error here") + return errors.Wrap(err, "expected 404 not found API error here") } return nil From 303e53cea26bf8f83ceba8d27ca356b5e50f7504 Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Wed, 17 Jul 2019 23:25:26 -0500 Subject: [PATCH 8/9] Fix typo --- .../pkg/controller/elasticsearch/elasticsearch_controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go index f7aa55dd48..d73528effb 100644 --- a/operators/pkg/controller/elasticsearch/elasticsearch_controller.go +++ b/operators/pkg/controller/elasticsearch/elasticsearch_controller.go @@ -170,7 +170,7 @@ func (r *ReconcileElasticsearch) Reconcile(request reconcile.Request) (reconcile iterationStartTime := time.Now() log.Info("Start reconcile iteration", "iteration", currentIteration, "namespace", request.Namespace, "es_name", request.Name) defer func() { - log.Info("End reconcile iteration", "iteration", currentIteration, "took", time.Since(iterationStartTime), "namespace", request.Namespace, "es_ame", request.Name) + log.Info("End reconcile iteration", "iteration", currentIteration, "took", time.Since(iterationStartTime), "namespace", request.Namespace, "es_name", request.Name) }() // Fetch the Elasticsearch instance From e1f0c7f9c4fd8cecf631b68e488b3330b015f3ce Mon Sep 17 00:00:00 2001 From: Jon Sabo Date: Thu, 18 Jul 2019 10:07:50 -0500 Subject: [PATCH 9/9] Add log --- operators/pkg/controller/common/annotation/controller_version.go | 1 + 1 file changed, 1 insertion(+) diff --git a/operators/pkg/controller/common/annotation/controller_version.go b/operators/pkg/controller/common/annotation/controller_version.go index 8261d72538..33f1c5dfb4 100644 --- a/operators/pkg/controller/common/annotation/controller_version.go +++ b/operators/pkg/controller/common/annotation/controller_version.go @@ -37,6 +37,7 @@ func UpdateControllerVersion(client k8s.Client, obj runtime.Object, version stri // do not send unnecessary update if the value would not change if annotations[ControllerVersionAnnotation] == version { + log.V(1).Info("Skipping controller version annotation update, version already matches", "namespace", namespace, "name", name, "kind", obj.GetObjectKind()) return nil }