CORS-3281: IBMCloud: initial CAPI infrastructure #8090
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@cjschaef: This pull request references CORS-3281 which is a valid jira issue. 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. |
|
/retest |
| clusterapi.InfraProvider | ||
| } | ||
|
|
||
| var _ clusterapi.PreProvider = Provider{} |
There was a problem hiding this comment.
In case we change the Provider interface in the future:
| var _ clusterapi.PreProvider = Provider{} | |
| var _ clusterapi.Provider = Provider{} | |
| var _ clusterapi.PreProvider = Provider{} |
There was a problem hiding this comment.
Updated, I dropped PreProvider in favor of just Provider.
There was a problem hiding this comment.
But I see you still have an empty PreProvision function. If you intend on implementing that, I don't see an issue with keeping also the guard for the PreProvider interface.
There was a problem hiding this comment.
I suggest to either:
- remove the
PreProvisionfunction since it's doing nothing. Later on, when you implement it, add also thevar _ clusterapi.PreProvider = Provider{}guard. - Leave the empty
PreProvisionfunction and add thePreProviderguard since you intend on implementing the functionality in follow-up PRs.
There was a problem hiding this comment.
Updated.
Okay, I believe we need a PreProvision, to setup the RHCOS image, so I will expect to add that functionality to PreProvision later, and re-incorporated the PreProvider guard.
|
/test e2e-ibm-capi-ovn |
|
@r4f4: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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/test-infra repository. |
|
/test altinfra-e2e-ibm-capi-ovn |
|
@r4f4: The specified target(s) for
The following commands are available to trigger optional jobs:
Use
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/test-infra repository. |
|
/test altinfra-e2e-ibmcloud-capi-ovn 3rd time's the charm |
| // Provider is the IBM Cloud implementation of the clusterapi InfraProvider. | ||
| type Provider struct { | ||
| clusterapi.InfraProvider | ||
| } |
There was a problem hiding this comment.
I don't think you need this (I believe this may have been based on some earlier code I put together that confused things).
| clusterapi.InfraProvider | ||
| } | ||
|
|
||
| var _ clusterapi.Provider = Provider{} |
There was a problem hiding this comment.
| var _ clusterapi.Provider = Provider{} | |
| var _ clusterapi.Provider = (*Provider)(nil) |
There was a problem hiding this comment.
Updated, and PreProvider var as well.
| ) | ||
|
|
||
| var _ clusterapi.PreProvider = (*Provider)(nil) | ||
| var _ clusterapi.Provider = (*Provider)(nil) |
There was a problem hiding this comment.
Missing the provider definition:
| var _ clusterapi.Provider = (*Provider)(nil) | |
| var _ clusterapi.Provider = (*Provider)(nil) | |
| // Provider implements IBMCloud CAPI installation. | |
| type Provider struct{} |
There was a problem hiding this comment.
Trying to make sure I handle this comment as well, see what the expectation is regarding these
#8090 (comment)
There was a problem hiding this comment.
You still need to define a Provider type on which you're going to implement the required interfaces. @patrickdillon 's comment was regarding embedding the clusterapi.InfraProvider fields not being needed anymore.
| var _ clusterapi.PreProvider = (*Provider)(nil) | ||
| var _ clusterapi.Provider = (*Provider)(nil) | ||
|
|
||
| // Provider implements AWS CAPI installation. |
There was a problem hiding this comment.
| // Provider implements AWS CAPI installation. | |
| // Provider implements IBMCloud CAPI installation. |
There was a problem hiding this comment.
oops, fixed my copy-pasta
Adds the initial provider and infrastructure framework for the IBM Cloud CAPI install process. Related: https://issues.redhat.com/browse/CORS-3281
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: r4f4 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 |
openshift-ci
bot
added
the
approved
|
@cjschaef: The following tests 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/test-infra repository. I understand the commands that are listed here. |
|
Install failing as expected since the capi manifests generation hasn't been added yet: /lgtm |
openshift-merge-bot
bot
merged commit a9da892
into
openshift:master
|
[ART PR BUILD NOTIFIER] This PR has been included in build ose-installer-altinfra-container-v4.16.0-202403060709.p0.ga9da892.assembly.stream.el8 for distgit ose-installer-altinfra. |
Adds the initial provider and infrastructure framework for the IBM Cloud CAPI install process.
Related: https://issues.redhat.com/browse/CORS-3281