Elastic Agent: Set status.ObservedGeneration from metadata.Generation

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

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

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

pkg/apis/agent/v1alpha1/agent_types.go

@@ -137,6 +137,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
@@ -282,6 +288,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

pkg/controller/agent/controller.go

@@ -121,21 +121,24 @@ type ReconcileAgent struct {
 
 // Reconcile reads that state of the cluster for an Agent object and makes changes based on the state read
 // and what is in the Agent.Spec
-func (r *ReconcileAgent) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
+func (r *ReconcileAgent) Reconcile(ctx context.Context, request reconcile.Request) (result reconcile.Result, err error) {
 	ctx = common.NewReconciliationContext(ctx, &r.iteration, r.Tracer, controllerName, "agent_name", request)
 	defer common.LogReconciliationRunNoSideEffects(logconf.FromContext(ctx))()
 	defer tracing.EndContextTransaction(ctx)
 
-	var agent agentv1alpha1.Agent
-	if err := association.FetchWithAssociations(ctx, r.Client, request, &agent); err != nil {
+	agent := &agentv1alpha1.Agent{}
+	if err = association.FetchWithAssociations(ctx, r.Client, request, agent); err != nil {
 		if apierrors.IsNotFound(err) {
 			r.onDelete(request.NamespacedName)
 			return reconcile.Result{}, nil
 		}
-		return reconcile.Result{}, tracing.CaptureError(ctx, err)
+		if agent == nil {
+			return reconcile.Result{}, err
+		}
+		return updateStatus(ctx, *agent, r.Client, newStatus(*agent)).WithError(err).Aggregate()
 	}
 
-	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,22 +147,35 @@ 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)
+	result, err = r.doReconcile(ctx, *agent).Aggregate()
+	k8s.EmitErrorEvent(r.recorder, err, agent, events.EventReconciliationError, "Reconciliation error: %v", err)
 
-	return res, 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) (results *reconciler.Results) {
 	defer tracing.Span(&ctx)()
-	results := reconciler.NewResult(ctx)
+	var err error
+	results = reconciler.NewResult(ctx)
+	status := newStatus(agent)
+
+	// defer the updating of status to ensure that the status is updated regardless of the outcome of the reconciliation.
+	// note that this deferred function is modifying the return values, which are named return values, which allows this
+	// to function properly.
+	defer func() {
+		if updateStatusresults := updateStatus(ctx, agent, r.Client, status).WithError(err); updateStatusresults != nil {
+			results = updateStatusresults
+		}
+	}()
+
 	if !association.AreConfiguredIfSet(agent.GetAssociations(), r.recorder) {
 		return results
 	}
 
 	// Run basic validations as a fallback in case webhook is disabled.
-	if err := r.validate(ctx, agent); err != nil {
-		return results.WithError(err)
+	if err = r.validate(ctx, agent); err != nil {
+		results = results.WithError(err)
+		return results
 	}
 
 	driverResults := internalReconcile(Params{
@@ -169,9 +185,10 @@ func (r *ReconcileAgent) doReconcile(ctx context.Context, agent agentv1alpha1.Ag
 		Watches:        r.dynamicWatches,
 		Agent:          agent,
 		OperatorParams: r.Parameters,
-	})
+	}, &status)
 
-	return results.WithResults(driverResults)
+	results = results.WithResults(driverResults)
+	return results
 }
 
 func (r *ReconcileAgent) validate(ctx context.Context, agent agentv1alpha1.Agent) error {

pkg/controller/agent/controller_test.go

@@ -0,0 +1,144 @@
+// 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"
+	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/utils/k8s"
+)
+
+func newReconcileAgent(objs ...runtime.Object) *ReconcileAgent {
+	r := &ReconcileAgent{
+		Client:   k8s.NewFakeClient(objs...),
+		recorder: record.NewFakeRecorder(100),
+	}
+	return r
+}
+
+func TestReconcileAgent_Reconcile(t *testing.T) {
+	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,
+		},
+	}
+	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{}))
+		})
+	}
+}

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
 
@@ -46,18 +48,22 @@ type Params struct {
 	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,11 +72,18 @@ 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, status *agentv1alpha1.AgentStatus) *reconciler.Results {
 	defer tracing.Span(&params.Context)()
 	results := reconciler.NewResult(params.Context)
 
@@ -122,7 +135,7 @@ func internalReconcile(params Params) *reconciler.Results {
 	if err != nil {
 		return results.WithError(err)
 	}
-	return results.WithResults(reconcilePodVehicle(params, podTemplate))
+	return results.WithResults(reconcilePodVehicle(params, podTemplate, status))
 }
 
 func reconcileService(params Params) (*corev1.Service, error) {

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,