Machine API: Add cloud providers' specs #1005
Conversation
|
/assign |
machine/v1beta1/types_awsprovider.go
Outdated
| // MachineCreation indicates whether the machine has been created or not. If not, | ||
| // it should include a reason and message for the failure. | ||
| MachineCreation AWSMachineProviderConditionType = "MachineCreation" | ||
| AWSMachineCreation AWSMachineProviderConditionType = "MachineCreation" |
There was a problem hiding this comment.
Is it worth sharing some of these in a shared common_consts.go kind of file?
alexander-demicev
force-pushed
the
providerspecs
branch
from
c28a021 to
0535cb1
Compare
JoelSpeed
left a comment
JoelSpeed
left a comment
There was a problem hiding this comment.
One quick ask on the new conditions and I think I'm happy with this
| // MachineCreated indicates whether the machine has been created or not. If not, | ||
| // it should include a reason and message for the failure. | ||
| MachineCreation ConditionType = "MachineCreation" |
There was a problem hiding this comment.
I think we should add a note that this is used on certain platforms and it's preferred to use the other condition type MachineCreated
alexander-demicev
force-pushed
the
providerspecs
branch
from
0535cb1 to
8706515
Compare
|
/lgtm |
soltysh
left a comment
soltysh
left a comment
There was a problem hiding this comment.
You're missing a lot of godocs and a few other comments
alexander-demicev
force-pushed
the
providerspecs
branch
2 times, most recently
from
f031a5b to
7f351d5
Compare
alexander-demicev
force-pushed
the
providerspecs
branch
3 times, most recently
from
28165b0 to
39d51e9
Compare
soltysh
left a comment
soltysh
left a comment
There was a problem hiding this comment.
A nit about missing // +optional tag and you're good to go
| // CredentialsSecret is a reference to the secret with Azure credentials. | ||
| CredentialsSecret *corev1.SecretReference `json:"credentialsSecret,omitempty"` | ||
| // Location is the region to use to create the instance | ||
| Location string `json:"location,omitempty"` |
There was a problem hiding this comment.
You need // +optional above each optional, iow. each omitempty
There was a problem hiding this comment.
Thanks, all should be fixed now
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexander-demichev, JoelSpeed, soltysh 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
4 participants
This PR adds cloud providers' specs, these definitions are a copy-paste from different repos.
https://github.com/openshift/machine-api-operator/tree/master/pkg/apis/vsphereprovider/v1beta1
https://github.com/openshift/cluster-api-provider-azure/tree/master/pkg/apis/azureprovider/v1beta1
https://github.com/openshift/cluster-api-provider-gcp/tree/master/pkg/apis/gcpprovider/v1beta1