Machine API: Refactor provider status to use metav1.Condition

[RadekManak]

This PR refactors out provider-specific conditions in favour of metav1.Condition type from upstream Kubernetes.

Something that was picked up while the Machine API types were being moved into openshift/api is that we have a bunch of Condition structs that are all identical, we want to consolidate these and just use metav1.Condition instead. From an end-user perspective, there should be no change to the API as a result of this, but it makes our API a bit cleaner

metav1.Condition does not have LastProbeTime field but we believe this change to be safe because the only operators who have write access to these objects are the Machine API providers which are under our control and users/cluster admins don't have access to update these fields, hence there shouldn't be anyone setting the LastProbeTime field.

I have commits ready for all providers that require change.
RadekManak/machine-api-operator@62605c4
RadekManak/machine-api-provider-aws@83efffd
RadekManak/machine-api-provider-gcp@445538d
RadekManak/machine-api-provider-azure@d917f71

Jira: OCPCLOUD-1371
Previously discussed here: #1045

[JoelSpeed]

The only project that is affected by this api change is machine-api-operator for which I have commit ready.

I suspect each of the providers will also need an update as well (MAPA, MAPZ, MAPG)

Do the object serialize in the same way, are the fields within metav1.Condition matching the existing condition types that we have?

[RadekManak]

You are right. I tested against cluster-api-provider-* repos instead of machine-api-provider-*.

[RadekManak]

The other providers are also ready.
RadekManak/machine-api-provider-aws@83efffd
RadekManak/machine-api-provider-gcp@445538d
RadekManak/machine-api-provider-azure@d917f71

[alexander-demicev]

/lgtm

[JoelSpeed]

As these are API compatible from an end user perspective and the LastProbeTime field that is dropped was never actually set by the controllers using these conditions, I think this is ok (while it's technically breaking, users shouldn't have been relying on it as it was never set. This change should be transparent to and API consumer)

[lobziik]

/lgtm

[deads2k]

we believe this change to be safe because the only operators who have write access to these objects are the Machine API providers which are under our control and users/cluster admins don't have access to update these fields,

This tightens validation, but status writers are limited to those under our direct control (as distinct from spec). This means we know that the values we wrote are valid under the new schema. I'm willing to approve this, but if you detect any failure due to validation, you must revert this PR, fix the code in 4.11, and try again in 4.12.

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexander-demichev, deads2k, lobziik, RadekManak

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [deads2k]

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

[JoelSpeed]

This project is not a part of the no-ff process

/label px-approved
/label qe-approved
/label docs-approved

[openshift-ci]

@RadekManak: all tests passed!

Full PR test history. Your PR dashboard.

Details

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/test-infra repository. I understand the commands that are listed here.