🌱Export conditions.HasSameState method

[m-messiah]

What this PR does / why we need it:

There are cases when one wants to compare existing condition and the condition to be applied before applying them, to avoid extra PATCH request to the server, but the only way to do it today is to wrap it in gomega.Matcher, like this:

condition := clusterv1.Condition{...}
if existingCondition := conditions.Get(obj, condition.Type); existingCondition != nil {
	if ok, _ := conditions.HaveSameStateOf(&condition).Match(existingCondition); ok {
		return nil
	}
}
conditions.Set(obj, &condition)
return client.Update(ctx, obj)

In this PR I propose to export HasSameState, to simplify the code above as (I understand not a big benefit, but just not clear why should we use gomega for simple comparison

condition := clusterv1.Condition{...}
if existingCondition := conditions.Get(obj, condition.Type); existingCondition != nil {
	if conditions.HaveSameState(existingCondition, condition) {
		return nil
	}
}
conditions.Set(obj, &condition)
return client.Update(ctx, obj)

/area util

The only question is about conditions.v1beta2 - how close they are and does it make sense to improve the current code? It looks like those would not use the comparison helpers and meta.SetStatusCondition already returns the answer if the condition is changed.

[neolit123]

general +1, but if this is done i would also mark HaveSameStateOf as deprecated and remove it on the next minor release. there are no rules around that, but i'm not a fan of importing goomega/* code in non _test.go files or in non-test dedicated util packages.

[m-messiah]

general +1, but if this is done i would also mark HaveSameStateOf as deprecated and remove it on the next minor release. there are no rules around that, but i'm not a fan of importing goomega/* code in non _test.go files or in non-test dedicated util packages.

me too, that's why it is created, but it looks like HaveSameStateOf is used solely in tests of the library as a matcher and does the same job as MatchCondition matcher (but for pointers)

[vincepri]

/lgtm
/assign @fabriziopandini @sbueringer

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: ee5fe662a86f3d98a24df1844090e4e0480c1bbc

[fabriziopandini]

There are cases when one wants to compare existing condition and the condition to be applied before applying them, to avoid extra PATCH request to the server

If using patch helper, it does already this for you.

the only way to do it today is to wrap it in gomega.Matcher

this is expected, HaveSameStateOf has been designed as a matcher to be used in tests.
But if you need to expose also HasSameState no objection

general +1, but if this is done i would also mark HaveSameStateOf as deprecated and remove it on the next minor release

having a matcher helps in writing test, I would Not deprecate it.

The only question is about conditions.v1beta2 - how close they are and does it make sense to improve the current code? It looks like those would not use the comparison helpers and meta.SetStatusCondition already returns the answer if the condition is changed.

New conditions helpers recently merged, see #10997

[fabriziopandini]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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:

• OWNERS [fabriziopandini]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[m-messiah]

New conditions helpers recently merged, see #10997

yes, I saw it, but was questioning when they would become the base, replacing the code I am fixing here.
If it was three weeks ago, I assume the move forward is not soon, so ok.

[sbueringer]

Yeah definitely fine to improve the current helpers while they are still around