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

Chore(Internal): Switch all resources to use status struct with common fields #1766

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Baarsgaard
Copy link
Contributor

@Baarsgaard Baarsgaard commented Nov 18, 2024

Similar to #1765 this is working towards grinding down the differences between the various resources and reconciliation functions.

The goal here is to establish a common set of fields that should always be present on a Status object.
Additional fields can then be added as necessary as seen in the Dashboard status

@Baarsgaard Baarsgaard force-pushed the align_statuses branch 2 times, most recently from 61060be to 26475f6 Compare December 16, 2024 13:19
@Baarsgaard Baarsgaard changed the title Refactor: Align status objects across CRs Chore(Internal): Switch all resources to use status struct with common fields Dec 16, 2024
@Baarsgaard Baarsgaard marked this pull request as ready for review December 16, 2024 13:32
@weisdd
Copy link
Collaborator

weisdd commented Dec 19, 2024

@Baarsgaard My concern here is that it's not a 1:1 switch, because some resources do not implement either LastResync (alert groups, contact points, notification policies) or Conditions (datasources). So, I guess we should add the missing logic either in this or a separate PR. WDYT?

@Baarsgaard
Copy link
Contributor Author

Baarsgaard commented Dec 19, 2024

The plan was to discuss what fields should be in the Common status object.
Then once we agree, add them all in this PR since unused fields don't really do any harm.
After that I would refactor each reconciler one at a time.
It aligns well with how @theSuess wanted to do smaller releases previously.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants