-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add CI jobs for CAPI providers #24534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add CI jobs for CAPI providers #24534
Conversation
020dae0 to
afa167e
Compare
|
I see there are some images with image vendoring, can we fix those so that the tests pass or would you rather have the tests merged first? They look ok and as if they are working as far as I can tell /lgtm |
afa167e to
e3ee334
Compare
|
/retest |
1 similar comment
|
/retest |
e3ee334 to
e91d974
Compare
|
/retest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
admittedly there are two conflicting guidelines there, in this case, but i think we want the platform prefix to come first, so gcp-capi-controllers (i don't think we need "cluster" in the name) and similar for the other ones.
@smarterclayton agree?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, <platform>-<clear_name_describing_function>-<type_of_image>.
I don't think CAPI is sufficient on its own, i would probably say gcp-cluster-api-controllers is the pattern I'd expect
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, all fixed. My intention was to have all CAPI related images near each other in the list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood, we tilted this way because ultimately we want to be clear about what components are required on a given cluster, and like other cloud provider resources there can be only one active at a time.
e91d974 to
1a1badb
Compare
|
@alexander-demichev: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
|
I'll let ben approve. |
|
/approve thanks for fixing up the names |
Fedosin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demichev, bparees, Fedosin, JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@alexander-demichev: Updated the following 4 configmaps:
DetailsIn 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. |
Add CI jobs for CAPI providers, these components are a part of Cluster API integration and are described here openshift/enhancements#913.
All Machine API related code has been moved to https://github.com/openshift/machine-api-provider-azure, https://github.com/openshift/machine-api-provider-gcp, https://github.com/openshift/machine-api-provider-aws repos, this means that it's safe to use
cluster-api-provider-*repos for the upstream providershttps://github.com/kubernetes-sigs/cluster-api-provider-*.