Skip to content

Conversation

@alexandernorth
Copy link
Collaborator

This PR shows one way in which we can consolidate the reporting of status and conditions into a common function.

It exposes access to the EventRecorder so events can be emitted independently of condition updates, and also logs changes in conditions as well as errors.

Would be happy to receive feedback and take suggestions as to how we can improve this functionality.

Copy link
Collaborator

@bruelea bruelea left a comment

Choose a reason for hiding this comment

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

Nice, the possibility to call the Report method on the EventStatusRecorder makes it well readable, also the Conditions() method in the api is very clear.

Copy link
Collaborator

@faebr faebr left a comment

Choose a reason for hiding this comment

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

I very much like the usage of an interface and removing all this duplicate code. I have 2 improvement ideas, see comments.

adjust string separator

add copyright header

modified error return for prefixclaimcontroller

implement feedback - logging message location

only perform update call if the condition changed
@alexandernorth alexandernorth force-pushed the feature/conditions-improvements branch from 6e8e185 to ea07072 Compare April 16, 2025 08:16
@alexandernorth alexandernorth force-pushed the feature/conditions-improvements branch from 06ac2a7 to a454dde Compare April 16, 2025 15:55
@bruelea bruelea self-requested a review April 17, 2025 09:33
@alexandernorth alexandernorth requested a review from faebr April 17, 2025 14:12
@alexandernorth alexandernorth enabled auto-merge (squash) April 22, 2025 12:29
@alexandernorth alexandernorth merged commit 1fb995e into main Apr 22, 2025
8 checks passed
@alexandernorth alexandernorth deleted the feature/conditions-improvements branch April 22, 2025 13:19
vaishutin pushed a commit to vaishutin/netbox-operator that referenced this pull request Aug 17, 2025
* refactor event/status reporting

* only perform update call if the condition changed

* don't use private fields of EventStatusRecorder

* set ready false at beginning of reconcile loop

* add information about kind image loading with containerd

* fix match string for chainsaw tests

* change format string to wrap error
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.

4 participants