[ACM-10812]: retry status update on conflict #1427
Conversation
Signed-off-by: Thibault Mange <[email protected]>
Signed-off-by: Thibault Mange <[email protected]>
Signed-off-by: Thibault Mange <[email protected]>
operators/endpointmetrics/controllers/observabilityendpoint/observabilityaddon_controller.go
Show resolved
Hide resolved
|
|
||
| // shouldAppendCondition checks if the new condition should be appended to the status conditions | ||
| // based on the last condition in the slice. | ||
| func shouldAppendCondition(conditions []oav1beta1.StatusCondition, newCondition *oav1beta1.StatusCondition) bool { |
There was a problem hiding this comment.
So it stops from changing the condition, if the condition is the same?
There was a problem hiding this comment.
The idea is to avoid adding duplicated status. If it is available, I don't need to report available again.
But I assume here that statuses are sorted by date, which is not necessarily the case IIRC. Probably have to change this! Having a look.
|
|
||
| obsAddon.Status.Conditions = append(obsAddon.Status.Conditions, *newCondition) | ||
|
|
||
| if len(obsAddon.Status.Conditions) > MaxStatusConditionsCount { |
There was a problem hiding this comment.
Is this just for convenience, or why do we want only 10?
There was a problem hiding this comment.
It's an arbitrary number to limit the list size... You think we need more than 10? Previously we were overriding the whole list with the last element, so 10 is better than before already.
There was a problem hiding this comment.
I mostly just wondered if there was a specific reason for such a limit. Couldn't we just keep adding? Not sure if there is some best practice on this area.
Anything, this should be fine for now.
There was a problem hiding this comment.
I want to avoid an ever increasing list... And potential associated bugs... Maybe it's over engineered
Signed-off-by: Thibault Mange <[email protected]>
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jacobbaungard, thibaultmg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@thibaultmg: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
* retry status update on conflict Signed-off-by: Thibault Mange <[email protected]> * add maxConditions handling Signed-off-by: Thibault Mange <[email protected]> * return err Signed-off-by: Thibault Mange <[email protected]> * sort status condition Signed-off-by: Thibault Mange <[email protected]> --------- Signed-off-by: Thibault Mange <[email protected]>
* retry status update on conflict Signed-off-by: Thibault Mange <[email protected]> * add maxConditions handling Signed-off-by: Thibault Mange <[email protected]> * return err Signed-off-by: Thibault Mange <[email protected]> * sort status condition Signed-off-by: Thibault Mange <[email protected]> --------- Signed-off-by: Thibault Mange <[email protected]>
* [ACM-10812]: fix addon status not reported in hub (#1420) * init version Signed-off-by: Thibault Mange <[email protected]> * fix Signed-off-by: Thibault Mange <[email protected]> * env test Signed-off-by: Thibault Mange <[email protected]> * change withReload naming Signed-off-by: Thibault Mange <[email protected]> --------- Signed-off-by: Thibault Mange <[email protected]> * [ACM-10812]: retry status update on conflict (#1427) * retry status update on conflict Signed-off-by: Thibault Mange <[email protected]> * add maxConditions handling Signed-off-by: Thibault Mange <[email protected]> * return err Signed-off-by: Thibault Mange <[email protected]> * sort status condition Signed-off-by: Thibault Mange <[email protected]> --------- Signed-off-by: Thibault Mange <[email protected]> * fix Signed-off-by: Thibault Mange <[email protected]> --------- Signed-off-by: Thibault Mange <[email protected]>
In some cases, while the metrics collector is well deployed in the spoke, the addon status remains in progressing state on the hub. After having modified the part that reflects the local observability addon state to the hub in #1420, this PR modifies the code that updates the local observability addon on the spoke:
It also removes some weird stuff like the
reportStatus boolfunction parameter in the reportStatus function 😅