[CNF-5643] add capability name to machine crd #1505
Conversation
openshift-ci
bot
added
the
size/XS
| kind: CustomResourceDefinition | ||
| metadata: | ||
| annotations: | ||
| capability.openshift.io/name: MachineAPI+baremetal |
There was a problem hiding this comment.
The + syntax means that this Machine CRD will only be managed when both MachineAPI and baremetal are enabled. But it's not obvious to me why we wouldn't want Machine when MachineAPI alone was enabled and baremetal was not. I don't see anything in the commit message or pull request comments explaining how baremetal is involved. What is the context?
There was a problem hiding this comment.
The context here is that baremetal operator require this crd to exist even if machine-api is disabled and baremetal enabled.
Also, as I know, baremetal operator does not exist at SNO clusters
There was a problem hiding this comment.
Is there an or syntax for the capabilities?
There was a problem hiding this comment.
@JoelSpeed
No, according to the docs there is only "and"
There was a problem hiding this comment.
The lack of or syntax is intentional, because:
Our current plan is to only annotate resources that are not depended upon (aka "leaf" components)
Is there a description somewhere about what the baremetal component enables when MachineAPI is disabled? There's not enough detail in the baremetal docstring for me to understand what overlaps with MachineAPI and what is distinct from MachineAPI.
There was a problem hiding this comment.
@honza
Can you help with that? You added this capability and I think you might know more
There was a problem hiding this comment.
@wking
I did a quick research and found that baremetal isn't present on SNO installs.
MachineAPI is specially designed for SNO, so I think we can remove baremetal dependency and keep it as MachineAPI only. This would be documented as well
There was a problem hiding this comment.
@wking
Removed. Now it's only MachineAPI cap. It will be documented so no issues expected
@JoelSpeed
Are you ok with that?
qJkee
force-pushed
the
CNF-5643-make-operator-optional
branch
from
2e9e784 to
eaabf87
Compare
|
You need to also include the ControlPlaneMachineSet CRD in this right? |
qJkee
force-pushed
the
CNF-5643-make-operator-optional
branch
from
3c074b4 to
94a2b21
Compare
|
@JoelSpeed |
|
/test integration |
|
The integration tests timeout is at it's limit, it needs to be bumped |
|
@qJkee: all tests passed! 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. |
|
@JoelSpeed |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed, qJkee 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
bot
added
the
approved
4 participants
In order to merge openshift/machine-api-operator#1119 I need to add capability annotations to api files because they are directly copied at machine-api-operator