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

Ensure status.version is reconciled by watching Pods #3534

Merged
merged 1 commit into from
Jul 28, 2020

Conversation

sebgl
Copy link
Contributor

@sebgl sebgl commented Jul 27, 2020

We did not enqueue reconciliations on Pods changes in Kibana, APMServer
and Beats controllers. We did on Elasticsearch and EnterpriseSearch
controllers.
When controllers rely on information retrieved from Pods as
part of the reconciliation, not watching Pods can lead to missing some
events.
This is especially true for the status.version field in all resources.
Note this particular case could have been worked out differently, by
checking the ReplicaSets for example. I think the implementation would
be more complicated, and we already watch all Pods due to the
Elasticsearch and EnterpriseSearch controllers anyway.

This commit sets up a single helper function called from all
controllers.

Fixes #3533.

We did not enqueue reconciliations on Pods changes in Kibana, APMServer
and Beats controllers. We did on Elasticsearch and EnterpriseSearch
controllers.
When controllers rely on information retrieved from Pods as
part of the reconciliation, not watching Pods can lead to missing some
events.
This is especially true for the `status.version` field in all resources.
Note this particular case could have been worked out differently, by
checking the replicas sets for example. I think the implementation would
be more complicated, and we already watch all Pods due to the
Elasticsearch and EnterpriseSearch controllers anyway.

This commit sets up a single helper function called from all
controllers.
@sebgl sebgl added >bug Something isn't working v1.3.0 labels Jul 27, 2020
@sebgl sebgl changed the title Watch Pods in all controllers Ensure status.version is reconciled by watching Pods Jul 27, 2020
Copy link
Contributor

@barkbay barkbay left a comment

Choose a reason for hiding this comment

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

LGTM

@sebgl sebgl merged commit aad4d20 into elastic:master Jul 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug Something isn't working v1.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Version in APM status does not get populated (no reconciliation)
3 participants