Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Elastic APM Server: Set status.ObservedGeneration from metadata.Generation #5470

Merged
merged 47 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
e8a3ba6
apm observed generation changes.
naemono Feb 7, 2022
9de0275
Ensure apm status is updated when associations are broken.
naemono Mar 14, 2022
28e6b74
Capitalization issues
naemono Mar 14, 2022
04b538d
Adjust e2e tests for apmserver to support mutations and check observe…
naemono Mar 14, 2022
f47d4d5
Debugging e2e test failures
naemono Mar 14, 2022
272bd83
remove todo
naemono Mar 14, 2022
fcb6eac
nolint validation function
naemono Mar 14, 2022
1300560
remove unused erroring fake k8s client
naemono Mar 14, 2022
199c02b
remove fakeK8sClient from apm testing
naemono Mar 14, 2022
ca0b2ec
Fix spelling
naemono Mar 14, 2022
1cf201e
wip
naemono Mar 15, 2022
822498c
Avoid mutating the original apmserver's configuration during e2e tests
naemono Mar 21, 2022
d7285b1
Add step to Apm Server CheckStackTestSteps to ensure ES APM Server us…
naemono Mar 22, 2022
3588bb0
Add description to CheckIndexCreation step.
naemono Mar 22, 2022
6e830f2
Lint issues
naemono Mar 22, 2022
dcb5635
Cleanup of some unneeded logging messages and comments.
naemono Mar 22, 2022
e152518
remove duplicative recorder, dynamicwatches, and parameters
naemono Mar 22, 2022
e11c85c
wip
naemono Mar 24, 2022
4322f60
Remove confusing defer bits from apmserver reconciliation
naemono Mar 24, 2022
06551bb
Upcase Kubernetes
naemono Mar 24, 2022
fb6dba4
Merge branch 'main' into 3392-apm-observedGeneration
naemono Mar 24, 2022
56e1eda
move associations check inside doReconcile
naemono Mar 24, 2022
b14328a
Wrap erorrs, don't log in e2e test.
naemono Mar 31, 2022
d7c364d
Move CheckIndexCreation closer to where it's used.
naemono Mar 31, 2022
6bf5327
Use implemented deepcopy instead of copying the internal map in apm b…
naemono Mar 31, 2022
e6868d0
remove unneeded log import/var
naemono Apr 1, 2022
49ffe86
Move index variable closer to where it's used.
naemono Apr 4, 2022
811421e
Add without integration check piece to original builder.
naemono Apr 4, 2022
3e04abd
Allow counting of documents in index during upgrade to function prope…
naemono Apr 11, 2022
e6153a6
just return the assert
naemono Apr 11, 2022
83312dc
Adding apm tracing back to particular error.
naemono Apr 13, 2022
53c15d7
remove unused result from state
naemono Apr 13, 2022
c22eb6a
just leveraging eventually for the apm server upgrade etest.
naemono Apr 14, 2022
738561c
Merge branch 'main' into 3392-apm-observedGeneration
naemono Apr 14, 2022
8c0a532
just return func
naemono Apr 15, 2022
51c62c7
remove retry.UntilSuccess in CheckEventsInElasticsearch e2e test
naemono Apr 19, 2022
2e8791e
Move retry.UntilSuccess around the assert calls only.
naemono Apr 19, 2022
1a511d5
Add type check to satisfy linter.
naemono Apr 20, 2022
a0a727c
Add WithoutIntegrationCheck to smoke tests for apm server builder
naemono Apr 24, 2022
fc56794
Merge branch 'main' into 3392-apm-observedGeneration
naemono Apr 25, 2022
3a82b71
Latest released version, not latest snapshot
naemono Apr 25, 2022
4fbb15f
If no ES ref exists, just skip the step where you test for ES data in…
naemono Apr 26, 2022
516df86
Add WithoutIntegrationCheck to smoke tests for apm server builder (ag…
naemono Apr 26, 2022
895a2cf
Move retry logic to include checkEventsAPI
naemono Apr 28, 2022
a0e56e4
Expicitly return empty result.
naemono Apr 28, 2022
ea77435
minimize the total time, and retry time.
naemono May 4, 2022
7cc64ac
Adjust retry to just retry the check in ES.
naemono May 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions config/crds/v1/all-crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,15 @@ spec:
description: KibanaAssociationStatus is the status of any auto-linking
to Kibana.
type: string
observedGeneration:
description: ObservedGeneration represents the .metadata.generation
that the status is based upon. 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
APM Server controller has not yet processed the changes contained
in the APM Server specification.
format: int64
type: integer
secretTokenSecret:
description: SecretTokenSecretName is the name of the Secret that
contains the secret token
Expand Down
9 changes: 9 additions & 0 deletions config/crds/v1/bases/apm.k8s.elastic.co_apmservers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7686,6 +7686,15 @@ spec:
description: KibanaAssociationStatus is the status of any auto-linking
to Kibana.
type: string
observedGeneration:
description: ObservedGeneration represents the .metadata.generation
that the status is based upon. 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
APM Server controller has not yet processed the changes contained
in the APM Server specification.
format: int64
type: integer
secretTokenSecret:
description: SecretTokenSecretName is the name of the Secret that
contains the secret token
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1474,6 +1474,15 @@ spec:
description: KibanaAssociationStatus is the status of any auto-linking
to Kibana.
type: string
observedGeneration:
description: ObservedGeneration represents the .metadata.generation
that the status is based upon. 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
APM Server controller has not yet processed the changes contained
in the APM Server specification.
format: int64
type: integer
secretTokenSecret:
description: SecretTokenSecretName is the name of the Secret that
contains the secret token
Expand Down
16 changes: 16 additions & 0 deletions pkg/apis/apm/v1/apmserver_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,25 @@ type ApmServerSpec struct {
// ApmServerStatus defines the observed state of ApmServer
type ApmServerStatus struct {
commonv1.DeploymentStatus `json:",inline"`

// ExternalService is the name of the service the agents should connect to.
ExternalService string `json:"service,omitempty"`

// SecretTokenSecretName is the name of the Secret that contains the secret token
SecretTokenSecretName string `json:"secretTokenSecret,omitempty"`

// ElasticsearchAssociationStatus is the status of any auto-linking to Elasticsearch clusters.
ElasticsearchAssociationStatus commonv1.AssociationStatus `json:"elasticsearchAssociationStatus,omitempty"`

// KibanaAssociationStatus is the status of any auto-linking to Kibana.
KibanaAssociationStatus commonv1.AssociationStatus `json:"kibanaAssociationStatus,omitempty"`

// ObservedGeneration represents the .metadata.generation that the status is based upon.
// 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 APM Server
// controller has not yet processed the changes contained in the APM Server specification.
// +kubebuilder:validation:Optional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I ever noticed it, but there are 2 ways to specify a field as optional: +kubebuilder:validation:Optional and +optional. Only the former seems to be documented.

ObservedGeneration int64 `json:"observedGeneration,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down Expand Up @@ -186,6 +197,11 @@ func (as *ApmServer) SetAssociationStatusMap(typ commonv1.AssociationType, statu
}
}

// GetObservedGeneration will return the observedGeneration from the Elastic APM Server's status.
func (as *ApmServer) GetObservedGeneration() int64 {
return as.Status.ObservedGeneration
}

// ApmEsAssociation helps to manage the APMServer / Elasticsearch association
type ApmEsAssociation struct {
*ApmServer
Expand Down
64 changes: 33 additions & 31 deletions pkg/controller/apmserver/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ func Add(mgr manager.Manager, params operator.Parameters) error {

// newReconciler returns a new reconcile.Reconciler
func newReconciler(mgr manager.Manager, params operator.Parameters) *ReconcileApmServer {
client := mgr.GetClient()
return &ReconcileApmServer{
Client: client,
Client: mgr.GetClient(),
recorder: mgr.GetEventRecorderFor(controllerName),
dynamicWatches: watches.NewDynamicWatches(),
Parameters: params,
Expand Down Expand Up @@ -153,14 +152,18 @@ type ReconcileApmServer struct {
iteration uint64
}

// K8sClient returns the kubernetes client from the APM Server reconciler.
func (r *ReconcileApmServer) K8sClient() k8s.Client {
return r.Client
}

// DynamicWatches returns the set of dynamic watches from the APM Server reconciler.
func (r *ReconcileApmServer) DynamicWatches() watches.DynamicWatches {
return r.dynamicWatches
}

// Recorder returns the Kubernetes recorder that is responsible for recording and reporting
// events from the APM Server reconciler.
naemono marked this conversation as resolved.
Show resolved Hide resolved
func (r *ReconcileApmServer) Recorder() record.EventRecorder {
return r.recorder
}
Expand Down Expand Up @@ -200,30 +203,34 @@ func (r *ReconcileApmServer) Reconcile(ctx context.Context, request reconcile.Re
return reconcile.Result{}, r.onDelete(k8s.ExtractNamespacedName(&as))
}

results, state := r.doReconcile(ctx, &as)

return results.WithError(r.updateStatus(ctx, state)).Aggregate()
}

func (r *ReconcileApmServer) doReconcile(ctx context.Context, as *apmv1.ApmServer) (*reconciler.Results, State) {
state := NewState(as)
results := reconciler.NewResult(ctx)

areAssocsConfigured, err := association.AreConfiguredIfSet(as.GetAssociations(), r.recorder)
if err != nil {
return reconcile.Result{}, tracing.CaptureError(ctx, err)
naemono marked this conversation as resolved.
Show resolved Hide resolved
return results.WithError(tracing.CaptureError(ctx, err)), state
}
if !areAssocsConfigured {
return reconcile.Result{}, nil
return results, state
}

return r.doReconcile(ctx, request, &as)
}

func (r *ReconcileApmServer) doReconcile(ctx context.Context, request reconcile.Request, as *apmv1.ApmServer) (reconcile.Result, error) {
// Run validation in case the webhook is disabled
if err := r.validate(ctx, as); err != nil {
return reconcile.Result{}, err
return results.WithError(err), state
}

state := NewState(request, as)
svc, err := common.ReconcileService(ctx, r.Client, NewService(*as), as)
if err != nil {
return reconcile.Result{}, err
return results.WithError(err), state
}

_, results := certificates.Reconciler{
_, results = certificates.Reconciler{
K8sClient: r.K8sClient(),
DynamicWatches: r.DynamicWatches(),
Owner: as,
Expand All @@ -237,22 +244,22 @@ func (r *ReconcileApmServer) doReconcile(ctx context.Context, request reconcile.
GarbageCollectSecrets: true,
}.ReconcileCAAndHTTPCerts(ctx)
if results.HasError() {
res, err := results.Aggregate()
_, err := results.Aggregate()
k8s.EmitErrorEvent(r.recorder, err, as, events.EventReconciliationError, "Certificate reconciliation error: %v", err)
return res, err
return results, state
}

asVersion, err := version.Parse(as.Spec.Version)
if err != nil {
return reconcile.Result{}, err
return results.WithError(err), state
}
logger := log.WithValues("namespace", as.Namespace, "as_name", as.Name)
assocAllowed, err := association.AllowVersion(asVersion, as, logger, r.recorder)
if err != nil {
return reconcile.Result{}, tracing.CaptureError(ctx, err)
return results.WithError(tracing.CaptureError(ctx, err)), state
}
if !assocAllowed {
return reconcile.Result{}, nil // will eventually retry
return results, state // will eventually retry
}

r.warnIfDeprecated(asVersion, as)
Expand All @@ -261,23 +268,17 @@ func (r *ReconcileApmServer) doReconcile(ctx context.Context, request reconcile.
if err != nil {
if apierrors.IsConflict(err) {
log.V(1).Info("Conflict while updating status")
return reconcile.Result{Requeue: true}, nil
return results.WithResult(reconcile.Result{Requeue: true}), state
}
k8s.EmitErrorEvent(r.recorder, err, as, events.EventReconciliationError, "Deployment reconciliation error: %v", err)
return state.Result, tracing.CaptureError(ctx, err)
return results.WithError(tracing.CaptureError(ctx, err)), state
}

state.UpdateApmServerExternalService(*svc)

// update status
err = r.updateStatus(ctx, state)
if err != nil && apierrors.IsConflict(err) {
log.V(1).Info("Conflict while updating status", "namespace", as.Namespace, "as", as.Name)
return reconcile.Result{Requeue: true}, nil
}
res, err := results.WithError(err).Aggregate()
_, err = results.WithError(err).Aggregate()
k8s.EmitErrorEvent(r.recorder, err, as, events.EventReconciliationError, "Reconciliation error: %v", err)
return res, err
return results, state
}

func (r *ReconcileApmServer) warnIfDeprecated(version semver.Version, as *apmv1.ApmServer) {
Expand Down Expand Up @@ -345,12 +346,12 @@ func (r *ReconcileApmServer) updateStatus(ctx context.Context, state State) erro
span, _ := apm.StartSpan(ctx, "update_status", tracing.SpanTypeApp)
defer span.End()

current := state.originalApmServer
if reflect.DeepEqual(current.Status, state.ApmServer.Status) {
original := state.originalApmServer
if reflect.DeepEqual(original.Status, state.ApmServer.Status) {
return nil
}
if state.ApmServer.Status.IsDegraded(current.Status.DeploymentStatus) {
r.recorder.Event(current, corev1.EventTypeWarning, events.EventReasonUnhealthy, "Apm Server health degraded")
if state.ApmServer.Status.IsDegraded(original.Status.DeploymentStatus) {
r.recorder.Event(original, corev1.EventTypeWarning, events.EventReasonUnhealthy, "Apm Server health degraded")
}
log.V(1).Info("Updating status",
"iteration", atomic.LoadUint64(&r.iteration),
Expand All @@ -361,6 +362,7 @@ func (r *ReconcileApmServer) updateStatus(ctx context.Context, state State) erro
return common.UpdateStatus(r.Client, state.ApmServer)
}

// NewService returns the service used by the APM Server.
func NewService(as apmv1.ApmServer) *corev1.Service {
svc := corev1.Service{
ObjectMeta: as.Spec.HTTP.Service.ObjectMeta,
Expand Down
Loading