Adding a new CRD for configuration needed for Baremetal provisioning#460
Conversation
openshift-ci-robot
added
do-not-merge/work-in-progress
sadasu
force-pushed
the
new-metal3-CR
branch
3 times, most recently
from
8bde4ad to
fbb4aeb
Compare
|
/retest |
sadasu
force-pushed
the
new-metal3-CR
branch
2 times, most recently
from
7ec2b94 to
d49ef11
Compare
|
/retest |
| at the desired state | ||
| type: integer | ||
| format: int32 | ||
| version: v1alpha1 |
There was a problem hiding this comment.
are you planning going alpha or promoting beta later on?
There was a problem hiding this comment.
Promoting beta later on. But at this point want to leave it as alpha for this PR.
|
/retest |
| subresources: | ||
| scale: | ||
| labelSelectorPath: .status.labelSelector | ||
| specReplicasPath: .spec.replicas |
There was a problem hiding this comment.
Again, not a blocker for your own API, but we haven't generally placed scaling options here for operator-y things. I just encourage you to be sure you want to.
There was a problem hiding this comment.
Again, not a blocker for your own API, but we haven't generally placed scaling options here for operator-y things. I just encourage you to be sure you want to.
this still looks questionable. especially since you don't have spec.replicas in your schema.
|
A couple optional suggestions for consideration from me. Nothing that I would consider blocking. |
sadasu
force-pushed
the
new-metal3-CR
branch
3 times, most recently
from
278a918 to
3dedb8c
Compare
sadasu
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
/test e2e-aws-upgrade |
|
/test e2e-aws-upgrade |
1 similar comment
|
/test e2e-aws-upgrade |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: enxebre 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 |
openshift-ci-robot
added
the
approved
| validation: | ||
| openAPIV3Schema: | ||
| description: 'Provisioning contains configuration used by the Provisioning | ||
| service (Ironic) to provision baremetal hosts. \n Provisioning is created |
There was a problem hiding this comment.
nit I would just say "the metal3 provisioning services", Ironic is supposed to be an implementation detail.
|
Small nit in the description but this lgtm, we can adjust the description in some subsequent PR if this is otherwise good to merge. |
|
let's move on and do any farther interaction in follow ups |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test e2e-aws |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
12 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
imain
left a comment
imain
left a comment
There was a problem hiding this comment.
Just a few description nits. Nice work Sandhya.
| properties: | ||
| provisioningInterface: | ||
| description: provisioningInterface is the name of the network interface | ||
| on a baremetal server to the provisioning network. It can have values |
There was a problem hiding this comment.
This sentence seems awkward to me. I think I would say "proisioningInterface is the name of the network interface on a baremetal server that is used to provision new machines"
There was a problem hiding this comment.
Thank you for the comment @imain . I will take care of it in the next MAO patch containing changes to read this CR.
| provisioningInterface: | ||
| description: provisioningInterface is the name of the network interface | ||
| on a baremetal server to the provisioning network. It can have values | ||
| like eth1 or ens3. |
This is so baremetal can fetch the provisioning.metal3.io object and parametarise their deployment. Related openshift/api#540 and openshift#460
This is so baremetal can fetch the provisioning.metal3.io object and parametarise their deployment. Related openshift/api#540 and openshift#460
8 participants
A new CRD is being added to metal3.io API group that contains config
necessary to provision baremetal hosts.