OCPCLOUD-2563: Add Machine/MachineSet API for MAPI to CAPI migration #1818
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@JoelSpeed: This pull request references OCPCLOUD-2563 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.16.0" version, but no target version was set. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
Hello @JoelSpeed! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XXL
There was a problem hiding this comment.
These were in the base set of test files, but other platforms have their own test file, so, pulled these out
openshift-ci
bot
added
the
approved
|
/hold until @deads2k can review |
openshift-ci
bot
added
the
do-not-merge/hold
|
/retest |
|
LGTM at first pass. The Machine API Test files changes are unrelated to this change, right? Just a result of the new way that featuregates are being done? |
When adding feature gated fields, the generators now generate various feature gate versions of each CRD. Each CRD is required to have a test suite file, so I had to add them as I'm the first one putting a feature gated field on some of these resources |
|
I take it you're omitting MachineHealthCheck and ControlPlaneMachineSet here on purpose? |
Yes, my intention is that we build out the Machine and MachineSet core functionality for this feature first, and then once we have made progress on those, start looking more at the extensions like MHC and CPMS |
JoelSpeed
force-pushed
the
mapi-authoritative-api
branch
2 times, most recently
from
91c98da
to
8a8ed13
Compare
|
/test integration Change was a global timeout hit, not related to this PR |
openshift-merge-robot
added
the
needs-rebase
theobarberbany
force-pushed
the
mapi-authoritative-api
branch
from
8a8ed13
to
9190864
Compare
openshift-merge-robot
added
needs-rebase
theobarberbany
force-pushed
the
mapi-authoritative-api
branch
from
9190864
to
8a8ed13
Compare
|
I attempted to rebase this, then got confused as we are part way through changing how tests work (feature gates, files moving around). I don't think this is a blocker yet on our work so I have reverted to how Joel left it. New docs in https://github.com/openshift/api/pull/1834/files |
theobarberbany
force-pushed
the
mapi-authoritative-api
branch
from
8a8ed13
to
51c2e94
Compare
openshift-merge-robot
removed
the
needs-rebase
|
🤞 for getting this rebase right, this time |
openshift-ci
bot
added
size/M
|
That's a fun bug in the bot 😂 All CI failing : CI Bot: all tests passed! |
openshift-merge-robot
added
the
needs-rebase
JoelSpeed
force-pushed
the
mapi-authoritative-api
branch
from
b0f99e8
to
a78eafb
Compare
openshift-merge-robot
removed
the
needs-rebase
JoelSpeed
force-pushed
the
mapi-authoritative-api
branch
from
a78eafb
to
ce87825
Compare
|
/test verify-client-go |
1 similar comment
|
/test verify-client-go |
|
/retest |
|
/override ci/prow/verify-crd-schema |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema In response to this:
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. |
|
/lgtm |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: deads2k, JoelSpeed, racheljpg, theobarberbany 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 |
|
/override ci/prow/verify-crd-schema |
|
@JoelSpeed: Overrode contexts on behalf of JoelSpeed: ci/prow/verify-crd-schema In response to this:
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. |
|
@JoelSpeed: all tests passed! 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. |
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-cluster-config-api-container-v4.17.0-202405241743.p0.ge0056f2.assembly.stream.el9 for distgit ose-cluster-config-api. |
| @@ -224,6 +227,3 @@ type Condition struct { | |||
| // +optional | |||
| Message string `json:"message,omitempty"` | |||
| } | |||
|
|
|||
| // Conditions provide observations of the operational state of a Machine API resource. | |||
There was a problem hiding this comment.
This is probably on me, but did we intend to remove the Conditions type??
When im bringing this change into the MAO we're failing to build because this is missing.
There was a problem hiding this comment.
Yes we did, please substitute Conditions for []Condition as you bring this into MAO
This implements the first part of the API from openshift/enhancements#1465.
This adds a new
authoritativeAPIfield to the Machine and MachineSet spec and status, which will allow our new project to determine which of the two backends should be implementing the functionality of the machines.The feature will be behing the MachineAPIMigration feature gate. Not adding this to TechPreview until we have got something to show for the project, likely in 4.17.