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

Automatically handle .status.observedGeneration #563

Closed
2 tasks
csviri opened this issue Sep 27, 2021 · 1 comment · Fixed by #628
Closed
2 tasks

Automatically handle .status.observedGeneration #563

csviri opened this issue Sep 27, 2021 · 1 comment · Fixed by #628
Assignees
Labels
feature kind/feature Categorizes issue or PR as related to a new feature. scheduling-improvements-epic
Milestone

Comments

@csviri
Copy link
Collaborator

csviri commented Sep 27, 2021

It's a common practice in controllers to save observed generation in status sub-resource, thus to save the .metadata.generation that was last time reconciled to .status.observedGeneration.
The sdk could support this out of the box.

One way to do it, is to just check if the status POJO, on the resource has this field, and before we make the updateStatus call on API we just update it automatically.
(Note that quarkus needs to be supported, so be cautious about the reflection limitations)

An other more extensible way would be to provide an interface, which can be implemented by the custom resource, with two methods:
setObservedGeneration(generation) and getObservedGeneration. The user can implement the methods, to customize the field name or path. Although since it's quite common just to have the .status.observedGeneration in this path, this might be an over engineering.

The second part would be to use the field to make decision if a reconciliation loop should be triggered. Note that now we support generations in a way that controller is executed only if the generation is increased (can be turned off). However we store the last processed generation in memory, so if the operator is restarted we reconcile all the resources, event if the generation is not increased. So using this field would prevent unnecessary reconciliations and spare us some memory.

Note that this can be backwards compatible with the current implementation. But not sure if we should just push this into v2 in a opinionated way, where we just use the value from status, not from memory.

Implementation:

  • Implement within bounds of JOSDK
  • add quarkus support
@csviri csviri added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 27, 2021
@csviri csviri added this to the v2 milestone Sep 27, 2021
@csviri csviri removed this from the v2 milestone Sep 27, 2021
@csviri
Copy link
Collaborator Author

csviri commented Oct 12, 2021

Adding this to v2 for now. Let me know if you think otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature kind/feature Categorizes issue or PR as related to a new feature. scheduling-improvements-epic
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants