test: Fix bug #909
test: Fix bug #909
Conversation
| @@ -180,24 +183,23 @@ func (r *Reconciler) Reconcile(ctx context.Context, req controllerruntime.Reques | |||
| Message: "All of the works are synchronized to the latest", | |||
| }) | |||
| if workUpdated { | |||
| // revert the applied condition and failedPlacement if we made any changes to the work | |||
| resourceBinding.Status.FailedPlacements = nil | |||
| // revert the applied condition if we made any changes to the work | |||
There was a problem hiding this comment.
Why only revert the applied? Where are applied and available set in this case? It seems they are both not there.
There was a problem hiding this comment.
we set the applied condition as false below. In that case, available condition nil means unknown.
In the CRP status, if the previous condition is not true, we will reset the remaining and leave them as "unknown".
There was a problem hiding this comment.
why set it to false then? it seems that leaving it not set has the same effect?
| // Reset the conditions and failed placements. | ||
| for i := condition.OverriddenCondition; i < condition.TotalCondition; i++ { | ||
| resourceBinding.RemoveCondition(string(i.ResourceBindingConditionType())) | ||
| } | ||
| resourceBinding.Status.FailedPlacements = nil |
There was a problem hiding this comment.
I think the original design says the binding will always have all the conditions like CRP (unlike work). I am not sure if this will affect other component (i.e. CRP, rollout)
There was a problem hiding this comment.
if i remember it correctly, the original design is to keep the existing conditions there and only update the new ones. It won't affect other component and they all use "condition.IsConditionStatusTrue(availableCondition, binding.GetGeneration())". Nil will return false to the caller.
In the CRP code, we also reset all the existing conditions. https://github.com/Azure/fleet/blob/main/pkg/controllers/clusterresourceplacement/placement_status.go#L170
|
Thanks for the help, @britaniar is resuming her PR. |
Description of your changes
Fixes #
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
Special notes for your reviewer