diff --git a/config/crds/v1/all-crds.yaml b/config/crds/v1/all-crds.yaml index 5f04b54a15..5d8a431b27 100644 --- a/config/crds/v1/all-crds.yaml +++ b/config/crds/v1/all-crds.yaml @@ -785,6 +785,15 @@ spec: kibanaAssociationStatus: description: AssociationStatus is the status of an association resource. type: string + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this Elastic Agent. 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 + Elastic Agent controller has not yet processed the changes contained + in the Elastic Agent specification. + format: int64 + type: integer version: description: 'Version of the stack resource currently running. During version upgrades, multiple versions may run in parallel: this value diff --git a/config/crds/v1/bases/agent.k8s.elastic.co_agents.yaml b/config/crds/v1/bases/agent.k8s.elastic.co_agents.yaml index da7eb26943..e44a349da5 100644 --- a/config/crds/v1/bases/agent.k8s.elastic.co_agents.yaml +++ b/config/crds/v1/bases/agent.k8s.elastic.co_agents.yaml @@ -15630,6 +15630,15 @@ spec: kibanaAssociationStatus: description: AssociationStatus is the status of an association resource. type: string + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this Elastic Agent. 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 + Elastic Agent controller has not yet processed the changes contained + in the Elastic Agent specification. + format: int64 + type: integer version: description: 'Version of the stack resource currently running. During version upgrades, multiple versions may run in parallel: this value diff --git a/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml b/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml index 604a34b7c6..f0af4eae21 100644 --- a/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml +++ b/deploy/eck-operator/charts/eck-operator-crds/templates/all-crds.yaml @@ -791,6 +791,15 @@ spec: kibanaAssociationStatus: description: AssociationStatus is the status of an association resource. type: string + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this Elastic Agent. 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 + Elastic Agent controller has not yet processed the changes contained + in the Elastic Agent specification. + format: int64 + type: integer version: description: 'Version of the stack resource currently running. During version upgrades, multiple versions may run in parallel: this value diff --git a/pkg/apis/agent/v1alpha1/agent_types.go b/pkg/apis/agent/v1alpha1/agent_types.go index ec84fd780a..bff3f645c9 100644 --- a/pkg/apis/agent/v1alpha1/agent_types.go +++ b/pkg/apis/agent/v1alpha1/agent_types.go @@ -141,6 +141,12 @@ type AgentStatus struct { // +kubebuilder:validation:Optional FleetServerAssociationStatus commonv1.AssociationStatus `json:"fleetServerAssociationStatus,omitempty"` + + // ObservedGeneration is the most recent generation observed for this Elastic Agent. + // 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 Elastic + // Agent controller has not yet processed the changes contained in the Elastic Agent specification. + ObservedGeneration int64 `json:"observedGeneration,omitempty"` } type AgentHealth string @@ -288,6 +294,11 @@ func (a *Agent) SecureSettings() []commonv1.SecretSource { return a.Spec.SecureSettings } +// GetObservedGeneration will return the observedGeneration from the Elastic Agent's status. +func (a *Agent) GetObservedGeneration() int64 { + return a.Status.ObservedGeneration +} + type AgentESAssociation struct { *Agent // ref is the namespaced name of the Elasticsearch used in Association diff --git a/pkg/controller/agent/controller.go b/pkg/controller/agent/controller.go index 5409805991..1086765f4b 100644 --- a/pkg/controller/agent/controller.go +++ b/pkg/controller/agent/controller.go @@ -126,8 +126,8 @@ func (r *ReconcileAgent) Reconcile(ctx context.Context, request reconcile.Reques defer common.LogReconciliationRunNoSideEffects(logconf.FromContext(ctx))() defer tracing.EndContextTransaction(ctx) - var agent agentv1alpha1.Agent - if err := r.Client.Get(ctx, request.NamespacedName, &agent); err != nil { + agent := &agentv1alpha1.Agent{} + if err := r.Client.Get(ctx, request.NamespacedName, agent); err != nil { if apierrors.IsNotFound(err) { r.onDelete(request.NamespacedName) return reconcile.Result{}, nil @@ -135,7 +135,7 @@ func (r *ReconcileAgent) Reconcile(ctx context.Context, request reconcile.Reques return reconcile.Result{}, tracing.CaptureError(ctx, err) } - if common.IsUnmanaged(&agent) { + if common.IsUnmanaged(agent) { logconf.FromContext(ctx).Info("Object is currently not managed by this controller. Skipping reconciliation") return reconcile.Result{}, nil } @@ -144,38 +144,49 @@ func (r *ReconcileAgent) Reconcile(ctx context.Context, request reconcile.Reques return reconcile.Result{}, nil } - res, err := r.doReconcile(ctx, agent).Aggregate() - k8s.EmitErrorEvent(r.recorder, err, &agent, events.EventReconciliationError, "Reconciliation error: %v", err) + results, status := r.doReconcile(ctx, *agent) - return res, err + if err := updateStatus(*agent, r.Client, status); err != nil { + if apierrors.IsConflict(err) { + return results.WithResult(reconcile.Result{Requeue: true}).Aggregate() + } + results = results.WithError(err) + } + + result, err := results.Aggregate() + k8s.EmitErrorEvent(r.recorder, err, agent, events.EventReconciliationError, "Reconciliation error: %v", err) + + return result, err } -func (r *ReconcileAgent) doReconcile(ctx context.Context, agent agentv1alpha1.Agent) *reconciler.Results { +func (r *ReconcileAgent) doReconcile(ctx context.Context, agent agentv1alpha1.Agent) (*reconciler.Results, agentv1alpha1.AgentStatus) { defer tracing.Span(&ctx)() results := reconciler.NewResult(ctx) + status := newStatus(agent) + areAssocsConfigured, err := association.AreConfiguredIfSet(agent.GetAssociations(), r.recorder) if err != nil { - return results.WithError(err) + return results.WithError(err), status } if !areAssocsConfigured { - return results + return results, status } // Run basic validations as a fallback in case webhook is disabled. if err := r.validate(ctx, agent); err != nil { - return results.WithError(err) + results = results.WithError(err) + return results, status } - driverResults := internalReconcile(Params{ + return internalReconcile(Params{ Context: ctx, Client: r.Client, EventRecorder: r.recorder, Watches: r.dynamicWatches, Agent: agent, + Status: status, OperatorParams: r.Parameters, }) - - return results.WithResults(driverResults) } func (r *ReconcileAgent) validate(ctx context.Context, agent agentv1alpha1.Agent) error { diff --git a/pkg/controller/agent/controller_test.go b/pkg/controller/agent/controller_test.go new file mode 100644 index 0000000000..759fe0a565 --- /dev/null +++ b/pkg/controller/agent/controller_test.go @@ -0,0 +1,238 @@ +// 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 agent + +import ( + "context" + "reflect" + "testing" + + "github.com/google/go-cmp/cmp" + appsv1 "k8s.io/api/apps/v1" + corev1 "k8s.io/api/core/v1" + 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" + + agentv1alpha1 "github.com/elastic/cloud-on-k8s/pkg/apis/agent/v1alpha1" + "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/common/hash" + "github.com/elastic/cloud-on-k8s/pkg/controller/common/watches" + "github.com/elastic/cloud-on-k8s/pkg/utils/k8s" + "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" +) + +func newReconcileAgent(objs ...runtime.Object) *ReconcileAgent { + r := &ReconcileAgent{ + Client: k8s.NewFakeClient(objs...), + recorder: record.NewFakeRecorder(100), + dynamicWatches: watches.NewDynamicWatches(), + } + return r +} + +func TestReconcileAgent_Reconcile(t *testing.T) { + defaultLabels := NewLabels(agentv1alpha1.Agent{ObjectMeta: metav1.ObjectMeta{Name: "testAgent"}}) + tests := []struct { + name string + objs []runtime.Object + request reconcile.Request + want reconcile.Result + expected agentv1alpha1.Agent + wantErr bool + }{ + { + name: "valid unmanaged agent does not increment observedGeneration", + objs: []runtime.Object{ + &agentv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgent", + Namespace: "test", + Generation: 1, + Annotations: map[string]string{ + common.ManagedAnnotation: "false", + }, + }, + Spec: agentv1alpha1.AgentSpec{ + Version: "8.0.1", + Deployment: &agentv1alpha1.DeploymentSpec{}, + }, + Status: agentv1alpha1.AgentStatus{ + ObservedGeneration: 1, + }, + }, + }, + request: reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "testAgent", + }, + }, + want: reconcile.Result{}, + expected: agentv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgent", + Namespace: "test", + Generation: 1, + Annotations: map[string]string{ + common.ManagedAnnotation: "false", + }, + }, + Spec: agentv1alpha1.AgentSpec{ + Version: "8.0.1", + Deployment: &agentv1alpha1.DeploymentSpec{}, + }, + Status: agentv1alpha1.AgentStatus{ + ObservedGeneration: 1, + }, + }, + wantErr: false, + }, + { + name: "too long name fails validation, and updates observedGeneration", + objs: []runtime.Object{ + &agentv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgentwithtoolongofanamereallylongname", + Namespace: "test", + Generation: 2, + }, + Status: agentv1alpha1.AgentStatus{ + ObservedGeneration: 1, + }, + }, + }, + request: reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "testAgentwithtoolongofanamereallylongname", + }, + }, + want: reconcile.Result{}, + expected: agentv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgentwithtoolongofanamereallylongname", + Namespace: "test", + Generation: 2, + }, + Status: agentv1alpha1.AgentStatus{ + ObservedGeneration: 2, + }, + }, + wantErr: true, + }, + { + name: "agent with ready deployment+pod updates status.health properly", + objs: []runtime.Object{ + &agentv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgent", + Namespace: "test", + Generation: 2, + }, + Spec: agentv1alpha1.AgentSpec{ + Version: "8.0.1", + Deployment: &agentv1alpha1.DeploymentSpec{ + Replicas: pointer.Int32(1), + }, + }, + Status: agentv1alpha1.AgentStatus{ + ObservedGeneration: 1, + }, + }, + &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgent-agent", + Namespace: "test", + Labels: addLabel(defaultLabels, hash.TemplateHashLabelName, "2519944696"), + }, + Status: appsv1.DeploymentStatus{ + AvailableReplicas: 1, + Replicas: 1, + ReadyReplicas: 1, + Conditions: []appsv1.DeploymentCondition{ + { + Type: appsv1.DeploymentAvailable, + Status: corev1.ConditionTrue, + }, + }, + }, + }, + &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgent", + Namespace: "test", + Generation: 2, + Labels: map[string]string{NameLabelName: "testAgent", VersionLabelName: "8.0.1"}, + }, + Status: corev1.PodStatus{ + Phase: corev1.PodRunning, + }, + }, + }, + request: reconcile.Request{ + NamespacedName: types.NamespacedName{ + Namespace: "test", + Name: "testAgent", + }, + }, + want: reconcile.Result{}, + expected: agentv1alpha1.Agent{ + ObjectMeta: metav1.ObjectMeta{ + Name: "testAgent", + Namespace: "test", + Generation: 2, + }, + Spec: agentv1alpha1.AgentSpec{ + Version: "8.0.1", + Deployment: &agentv1alpha1.DeploymentSpec{ + Replicas: pointer.Int32(1), + }, + }, + Status: agentv1alpha1.AgentStatus{ + Version: "8.0.1", + ExpectedNodes: 1, + AvailableNodes: 1, + ObservedGeneration: 2, + Health: agentv1alpha1.AgentGreenHealth, + }, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := newReconcileAgent(tt.objs...) + got, err := r.Reconcile(context.Background(), tt.request) + if (err != nil) != tt.wantErr { + t.Errorf("ReconcileAgent.Reconcile() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("ReconcileAgent.Reconcile() = %v, want %v", got, tt.want) + } + + var agent agentv1alpha1.Agent + if err := r.Client.Get(context.Background(), tt.request.NamespacedName, &agent); err != nil { + t.Error(err) + return + } + // AllowUnexported required because of *AssocConf on the agent. + comparison.AssertEqual(t, &agent, &tt.expected, cmp.AllowUnexported(agentv1alpha1.Agent{})) + }) + } +} + +func addLabel(labels map[string]string, key, value string) map[string]string { + newLabels := make(map[string]string, len(labels)) + for k, v := range labels { + newLabels[k] = v + } + newLabels[key] = value + return newLabels +} diff --git a/pkg/controller/agent/driver.go b/pkg/controller/agent/driver.go index fe6611caae..4bdf754926 100644 --- a/pkg/controller/agent/driver.go +++ b/pkg/controller/agent/driver.go @@ -31,9 +31,11 @@ import ( ) const ( + // FleetServerPort is the standard Elastic Fleet Server port. FleetServerPort int32 = 8220 ) +// Params are a set of parameters used during internal reconciliation of Elastic Agents. type Params struct { Context context.Context @@ -41,23 +43,28 @@ type Params struct { EventRecorder record.EventRecorder Watches watches.DynamicWatches - Agent agentv1alpha1.Agent + Agent agentv1alpha1.Agent + Status agentv1alpha1.AgentStatus OperatorParams operator.Parameters } +// K8sClient returns the Kubernetes client. func (p Params) K8sClient() k8s.Client { return p.Client } +// Recorder returns the Kubernetes event recorder. func (p Params) Recorder() record.EventRecorder { return p.EventRecorder } +// DynamicWatches returns the set of stateful dynamic watches used during reconciliation. func (p Params) DynamicWatches() watches.DynamicWatches { return p.Watches } +// GetPodTemplate returns the configured pod template for the associated Elastic Agent. func (p *Params) GetPodTemplate() corev1.PodTemplateSpec { if p.Agent.Spec.DaemonSet != nil { return p.Agent.Spec.DaemonSet.PodTemplate @@ -66,29 +73,36 @@ func (p *Params) GetPodTemplate() corev1.PodTemplateSpec { return p.Agent.Spec.Deployment.PodTemplate } +// Logger returns the configured logger for use during reconciliation. func (p *Params) Logger() logr.Logger { return log.FromContext(p.Context) } -func internalReconcile(params Params) *reconciler.Results { +func newStatus(agent agentv1alpha1.Agent) agentv1alpha1.AgentStatus { + status := agent.Status + status.ObservedGeneration = agent.Generation + return status +} + +func internalReconcile(params Params) (*reconciler.Results, agentv1alpha1.AgentStatus) { defer tracing.Span(¶ms.Context)() results := reconciler.NewResult(params.Context) agentVersion, err := version.Parse(params.Agent.Spec.Version) if err != nil { - return results.WithError(err) + return results.WithError(err), params.Status } assocAllowed, err := association.AllowVersion(agentVersion, ¶ms.Agent, params.Logger(), params.EventRecorder) if err != nil { - return results.WithError(err) + return results.WithError(err), params.Status } if !assocAllowed { - return results // will eventually retry + return results, params.Status // will eventually retry } svc, err := reconcileService(params) if err != nil { - return results.WithError(err) + return results.WithError(err), params.Status } configHash := fnv.New32a() @@ -109,24 +123,24 @@ func internalReconcile(params Params) *reconciler.Results { ExtraHTTPSANs: []commonv1.SubjectAlternativeName{{DNS: fmt.Sprintf("*.%s.%s.svc", HTTPServiceName(params.Agent.Name), params.Agent.Namespace)}}, }.ReconcileCAAndHTTPCerts(params.Context) if caResults.HasError() { - return results.WithResults(caResults) + return results.WithResults(caResults), params.Status } _, _ = configHash.Write(fleetCerts.Data[certificates.CertFileName]) } if res := reconcileConfig(params, configHash); res.HasError() { - return results.WithResults(res) + return results.WithResults(res), params.Status } // we need to deref the secret here (if any) to include it in the configHash otherwise Agent will not be rolled on content changes if err := commonassociation.WriteAssocsToConfigHash(params.Client, params.Agent.GetAssociations(), configHash); err != nil { - return results.WithError(err) + return results.WithError(err), params.Status } podTemplate, err := buildPodTemplate(params, fleetCerts, configHash) if err != nil { - return results.WithError(err) + return results.WithError(err), params.Status } - return results.WithResults(reconcilePodVehicle(params, podTemplate)) + return reconcilePodVehicle(params, podTemplate) } func reconcileService(params Params) (*corev1.Service, error) { diff --git a/pkg/controller/agent/labels.go b/pkg/controller/agent/labels.go index b34e7c38d9..96ed635255 100644 --- a/pkg/controller/agent/labels.go +++ b/pkg/controller/agent/labels.go @@ -10,7 +10,7 @@ import ( ) const ( - // Type represents the Agent type. + // TypeLabelValue represents the Agent type. TypeLabelValue = "agent" // NameLabelName used to represent an Agent in k8s resources @@ -20,6 +20,7 @@ const ( NamespaceLabelName = "agent.k8s.elastic.co/namespace" ) +// NewLabels returns the set of common labels for an Elastic Agent. func NewLabels(agent agentv1alpha1.Agent) map[string]string { return map[string]string{ common.TypeLabelName: TypeLabelValue, diff --git a/pkg/controller/agent/name.go b/pkg/controller/agent/name.go index e893c86b2d..f180c83c4c 100644 --- a/pkg/controller/agent/name.go +++ b/pkg/controller/agent/name.go @@ -11,18 +11,23 @@ const httpServiceSuffix = "http" // Namer is a Namer that is configured with the defaults for resources related to an Agent resource. var Namer = common_name.NewNamer("agent") +// ConfigSecretName returns the name of a secret used to storage Elastic Agent configuration data. func ConfigSecretName(name string) string { return Namer.Suffix(name, "config") } +// Name returns the name of an Elastic Agent. func Name(name string) string { return Namer.Suffix(name) } +// HTTPServiceName returns the name of the HTTP service for a given Elastic Agent name. func HTTPServiceName(name string) string { return Namer.Suffix(name, httpServiceSuffix) } +// EnvVarsSecretName returns the name of the secret used to storage environmental variables +// for a given Elastic Agent name. func EnvVarsSecretName(name string) string { return Namer.Suffix(name, "envvars") } diff --git a/pkg/controller/agent/reconcile.go b/pkg/controller/agent/reconcile.go index 7d99771fcc..307c650619 100644 --- a/pkg/controller/agent/reconcile.go +++ b/pkg/controller/agent/reconcile.go @@ -6,6 +6,7 @@ package agent import ( "context" + "reflect" v1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -15,7 +16,8 @@ import ( "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" - "sigs.k8s.io/controller-runtime/pkg/reconcile" + + "github.com/pkg/errors" agentv1alpha1 "github.com/elastic/cloud-on-k8s/pkg/apis/agent/v1alpha1" "github.com/elastic/cloud-on-k8s/pkg/controller/common" @@ -27,7 +29,7 @@ import ( "github.com/elastic/cloud-on-k8s/pkg/utils/pointer" ) -func reconcilePodVehicle(params Params, podTemplate corev1.PodTemplateSpec) *reconciler.Results { +func reconcilePodVehicle(params Params, podTemplate corev1.PodTemplateSpec) (*reconciler.Results, agentv1alpha1.AgentStatus) { defer tracing.Span(¶ms.Context)() results := reconciler.NewResult(params.Context) @@ -62,7 +64,7 @@ func reconcilePodVehicle(params Params, podTemplate corev1.PodTemplateSpec) *rec }) if err != nil { - return results.WithError(err) + return results.WithError(err), params.Status } // clean up the other one @@ -75,13 +77,12 @@ func reconcilePodVehicle(params Params, podTemplate corev1.PodTemplateSpec) *rec results.WithError(err) } - err = updateStatus(params, ready, desired) - if err != nil && apierrors.IsConflict(err) { - params.Logger().V(1).Info("Conflict while updating status") - return results.WithResult(reconcile.Result{Requeue: true}) + var status agentv1alpha1.AgentStatus + if status, err = calculateStatus(¶ms, ready, desired); err != nil { + err = errors.Wrap(err, "while calculating status") } - return results.WithError(err) + return results.WithError(err), status } func reconcileDeployment(rp ReconciliationParams) (int32, int32, error) { @@ -128,27 +129,40 @@ func reconcileDaemonSet(rp ReconciliationParams) (int32, int32, error) { return reconciled.Status.NumberReady, reconciled.Status.DesiredNumberScheduled, nil } +// ReconciliationParams are the parameters used during an Elastic Agent's reconciliation. type ReconciliationParams struct { client k8s.Client agent agentv1alpha1.Agent podTemplate corev1.PodTemplateSpec } -func updateStatus(params Params, ready, desired int32) error { +// calculateStatus will calculate a new status from the state of the pods within the k8s cluster +// and will return any error encountered. +func calculateStatus(params *Params, ready, desired int32) (agentv1alpha1.AgentStatus, error) { agent := params.Agent + status := params.Status pods, err := k8s.PodsMatchingLabels(params.Client, agent.Namespace, map[string]string{NameLabelName: agent.Name}) if err != nil { - return err + return status, err } - agent.Status.AvailableNodes = ready - agent.Status.ExpectedNodes = desired + + status.Version = common.LowestVersionFromPods(status.Version, pods, VersionLabelName) + status.AvailableNodes = ready + status.ExpectedNodes = desired health, err := CalculateHealth(agent.GetAssociations(), ready, desired) if err != nil { - return err + return status, err } - agent.Status.Health = health - agent.Status.Version = common.LowestVersionFromPods(agent.Status.Version, pods, VersionLabelName) + status.Health = health + return status, nil +} - return params.Client.Status().Update(context.Background(), &agent) +// updateStatus will update the Elastic Agent's status within the k8s cluster, using the given Elastic Agent and status. +func updateStatus(agent agentv1alpha1.Agent, client client.Client, status agentv1alpha1.AgentStatus) error { + if reflect.DeepEqual(agent.Status, status) { + return nil + } + agent.Status = status + return common.UpdateStatus(client, &agent) } diff --git a/pkg/controller/association/reconciler.go b/pkg/controller/association/reconciler.go index 5f97a6b8be..b8b0df7fba 100644 --- a/pkg/controller/association/reconciler.go +++ b/pkg/controller/association/reconciler.go @@ -11,6 +11,7 @@ import ( "time" "github.com/go-logr/logr" + "github.com/pkg/errors" "go.elastic.co/apm" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -209,12 +210,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( } // we want to attempt a status update even in the presence of errors - if err := r.updateStatus(ctx, associated, newStatusMap); err != nil { - if apierrors.IsConflict(err) { - log.V(1).Info("Conflict while updating status") - return results.WithResult(reconcile.Result{Requeue: true}).Aggregate() - } - return defaultRequeue, tracing.CaptureError(ctx, err) + if err := r.updateStatus(ctx, associated, newStatusMap); err != nil && apierrors.IsConflict(err) { + log.V(1).Info( + "Conflict while updating status", + "namespace", associatedKey.Namespace, + "name", associatedKey.Name) + return results.WithResult(reconcile.Result{Requeue: true}).Aggregate() + } else if err != nil { + return defaultRequeue, tracing.CaptureError(ctx, errors.Wrapf(err, "while updating status")) } return results. WithResult(RequeueRbacCheck(r.accessReviewer)). diff --git a/pkg/controller/common/comparison/comparison.go b/pkg/controller/common/comparison/comparison.go index 06433bd99c..c4bce4bde1 100644 --- a/pkg/controller/common/comparison/comparison.go +++ b/pkg/controller/common/comparison/comparison.go @@ -23,17 +23,17 @@ func Equal(a, b runtime.Object) bool { // Diff returns the difference between two objects ignoring the TypeMeta and ResourceVersion. Often used for tests ensuring that we receive structs that match what we expect without // runtime-specific information -func Diff(a, b runtime.Object) string { +func Diff(a, b runtime.Object, opts ...cmp.Option) string { typemeta := cmpopts.IgnoreTypes(metav1.TypeMeta{}) rv := cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ResourceVersion") timestamps := cmpopts.IgnoreTypes(metav1.Time{}) - return cmp.Diff(a, b, typemeta, rv, timestamps) + return cmp.Diff(a, b, append(opts, typemeta, rv, timestamps)...) } // AssertEqual errors if two objects ignoring the TypeMeta and ResourceVersion. Equivalent to calling t.Error() -func AssertEqual(t *testing.T, a, b runtime.Object) { +func AssertEqual(t *testing.T, a, b runtime.Object, opts ...cmp.Option) { t.Helper() - if diff := Diff(a, b); diff != "" { + if diff := Diff(a, b, opts...); diff != "" { t.Errorf("Expected objects to be the same. Differences:\n%v", diff) } } diff --git a/test/e2e/agent/upgrade_test.go b/test/e2e/agent/upgrade_test.go index 80879285d0..c60bdfa8c2 100644 --- a/test/e2e/agent/upgrade_test.go +++ b/test/e2e/agent/upgrade_test.go @@ -20,9 +20,6 @@ func TestAgentVersionUpgradeToLatest8x(t *testing.T) { srcVersion := test.Ctx().ElasticStackVersion dstVersion := test.LatestSnapshotVersion8x - // TODO remove skip when https://github.com/elastic/kibana/issues/126611 is fixed - t.SkipNow() - test.SkipInvalidUpgrade(t, srcVersion, dstVersion) name := "test-agent-upgrade" @@ -69,7 +66,7 @@ func TestAgentVersionUpgradeToLatest8x(t *testing.T) { esBuilder.WithVersion(dstVersion).WithMutatedFrom(&esBuilder), kbBuilder.WithVersion(dstVersion).WithMutatedFrom(&kbBuilder), fleetServerBuilder.WithVersion(dstVersion), - agentBuilder.WithVersion(dstVersion), + agentBuilder.WithVersion(dstVersion).WithMutatedFrom(&agentBuilder), }, ) } diff --git a/test/e2e/test/agent/builder.go b/test/e2e/test/agent/builder.go index a86096fdeb..351ecd6633 100644 --- a/test/e2e/test/agent/builder.go +++ b/test/e2e/test/agent/builder.go @@ -43,6 +43,8 @@ type Builder struct { ValidationsOutputs []string AdditionalObjects []k8sclient.Object + MutatedFrom *Builder + // PodTemplate points to the PodTemplate in spec.DaemonSet or spec.Deployment PodTemplate *corev1.PodTemplateSpec @@ -107,6 +109,11 @@ func (b Builder) WithVersion(version string) Builder { return b } +func (b Builder) WithMutatedFrom(builder *Builder) Builder { + b.MutatedFrom = builder + return b +} + func (b Builder) WithDaemonSet() Builder { b.Agent.Spec.DaemonSet = &agentv1alpha1.DaemonSetSpec{} diff --git a/test/e2e/test/agent/steps.go b/test/e2e/test/agent/steps.go index eaff7b784d..78e33d0a2c 100644 --- a/test/e2e/test/agent/steps.go +++ b/test/e2e/test/agent/steps.go @@ -23,6 +23,7 @@ import ( "github.com/elastic/cloud-on-k8s/test/e2e/cmd/run" "github.com/elastic/cloud-on-k8s/test/e2e/test" "github.com/elastic/cloud-on-k8s/test/e2e/test/elasticsearch" + "github.com/elastic/cloud-on-k8s/test/e2e/test/generation" ) func (b Builder) InitTestSteps(k *test.K8sClient) test.StepList { @@ -121,6 +122,7 @@ func (b Builder) CheckK8sTestSteps(k *test.K8sClient) test.StepList { agent.Status.ElasticsearchAssociationsStatus = nil agent.Status.KibanaAssociationStatus = "" agent.Status.FleetServerAssociationStatus = "" + agent.Status.ObservedGeneration = 0 expected := agentv1alpha1.AgentStatus{ Version: b.Agent.Spec.Version, @@ -253,9 +255,16 @@ func (b Builder) DeletionTestSteps(k *test.K8sClient) test.StepList { } func (b Builder) MutationTestSteps(k *test.K8sClient) test.StepList { - return b.UpgradeTestSteps(k). + var agentGenerationBeforeMutation, agentObservedGenerationBeforeMutation int64 + + isMutated := b.MutatedFrom != nil + + return test.StepList{ + generation.RetrieveGenerationsStep(&b.Agent, k, &agentGenerationBeforeMutation, &agentObservedGenerationBeforeMutation), + }.WithSteps(b.UpgradeTestSteps(k)). WithSteps(b.CheckK8sTestSteps(k)). - WithSteps(b.CheckStackTestSteps(k)) + WithSteps(b.CheckStackTestSteps(k)). + WithStep(generation.CompareObjectGenerationsStep(&b.Agent, k, isMutated, agentGenerationBeforeMutation, agentObservedGenerationBeforeMutation)) } func (b Builder) MutationReversalTestContext() test.ReversalTestContext {