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

Set .status.observedGeneration #3392

Closed
benlangfeld opened this issue Jul 3, 2020 · 5 comments
Closed

Set .status.observedGeneration #3392

benlangfeld opened this issue Jul 3, 2020 · 5 comments
Assignees
Labels
>enhancement Enhancement of existing functionality

Comments

@benlangfeld
Copy link

Proposal

In order for deployment tools like krane to deterministically monitor the rollout of custom resources, those resources must set .status.observedGeneration.

This is a feature request for exactly that in this operator, such that our automation may effectively monitor rollout.

@botelastic botelastic bot added the triage label Jul 3, 2020
@sebgl sebgl added the >enhancement Enhancement of existing functionality label Jul 3, 2020
@botelastic botelastic bot removed the triage label Jul 3, 2020
@sebgl
Copy link
Contributor

sebgl commented Jul 3, 2020

It sounds like a good idea.

One important thing to discuss though:

The purpose of status.observedGeneration, as I understand it, is for people to compare with metadata.generation, so you know whether the Status you are looking at is up-to-date with the object you're looking at.
It does not mean that the underlying resources (eg. Pods) match the specification of the resource.
For example in a StatefulSet, seeing metadata.generation == status.observedGeneration means the controller has processed the StatefulSet resource with that generation, but it does not mean all underlying Pods are running.

I think it's an important distinction. Let's take an example with this Elasticsearch resource, where let's say we increased count from 1 to 3:

apiVersion: elasticsearch.k8s.elastic.co/v1
  kind: Elasticsearch
  metadata:
    name: mycluster
    generation: 12
  spec:
    version: 7.8.0
    nodeSets:
      count: 3
      name: default
  status:
    availableNodes: 1
    health: green
    phase: ApplyingChanges
    observedGeneration: 12

We see here that ECK has processed the resource in generation 12 (observedGeneration: 12). But it does not mean the desired 3-nodes cluster is up and running (see how availableNodes is still 1). So it does not mean the "rollout" of the new Elasticsearch specification (from 1 to 3) is complete yet.
In order for a check on status.observedGeneration to be useful, it must be combined with another check in the status section (eg. status.phase). It requires us to have strong consistency in all fields of the status. I'm not sure that's the case currently, but we should aim for it.

@benlangfeld
Copy link
Author

Yes, of course. If you check out the krane documentation I linked to you’ll see how that’s done. It’s easy and we’re ready to do it in our use case, but blocked on this requirement of the operator; that’s why I opened this feature request, not because I believe it’s the only requirement :)

@benlangfeld
Copy link
Author

@sebgl Are you able to describe what is necessary in order to implement this such that we might take a shot at it? The lack of an ability to robustly monitor ElasticSearch rollout is problematic for our application deployments which must perform indexing operations as soon as ElasticSearch is ready.

@naemono naemono self-assigned this Jan 24, 2022
@naemono
Copy link
Contributor

naemono commented Jan 25, 2022

I'm going to proceed on implementing this, moving forward with the below assumption:

The Status.* sections will only be applying to the latest status.observedGeneration, namely

status:
  observedGeneration: 15
  availableNodes: 3
  health: green
  phase: Ready
  version: 7.15.1

The status being observed using these fields only apply to generation 15, and if generation 16 has been applied to the spec, the operator hasn't acted upon this generation as of yet.

Please let me know if this assumption does not hold true, or needs further clarification.

@pebrc
Copy link
Collaborator

pebrc commented Jun 7, 2022

Closing as this is implemented now. Starting with ECK operator version 2.1 and Elasticsearch and Kibana.

@pebrc pebrc closed this as completed Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement Enhancement of existing functionality
Projects
None yet
Development

No branches or pull requests

4 participants