Add baremetal capability annotations #249
Conversation
|
/retest |
honza
force-pushed
the
baremetal-capability
branch
from
c31012f
to
40b60e6
Compare
honza
force-pushed
the
baremetal-capability
branch
from
40b60e6
to
8f43aca
Compare
|
/retest |
1 similar comment
|
/retest |
|
Does the BMH CRD also need this annonation? That would mean that we have to add this capability annotation to https://github.com/openshift/baremetal-operator/blob/master/config/crd/ocp/ocp_kustomization.yaml too. |
|
Or should it be an annotation on the container in CBO? |
|
In case it helps, recent nightly: $ oc adm release extract --to manifests registry.ci.openshift.org/ocp/release:4.11.0-0.nightly-2022-03-09-235248
Extracted release payload from digest sha256:05d922e0df6b70ec6170602c4ae374b04a94dd0fe552ea668a20782c2afe5158 created at 2022-03-09T23:56:42Z
$ ls manifests/*metal*
manifests/0000_30_baremetal-operator_01_baremetalhost.crd.yaml
manifests/0000_31_cluster-baremetal-operator_01_images.configmap.yaml
manifests/0000_31_cluster-baremetal-operator_01_trusted-ca-configmap.yaml
manifests/0000_31_cluster-baremetal-operator_02_metal3provisioning.crd.yaml
manifests/0000_31_cluster-baremetal-operator_03_service.yaml
manifests/0000_31_cluster-baremetal-operator_03_webhookservice.yaml
manifests/0000_31_cluster-baremetal-operator_04_serviceaccount.yaml
manifests/0000_31_cluster-baremetal-operator_05_kube-rbac-proxy-config.yaml
manifests/0000_31_cluster-baremetal-operator_05_prometheus_rbac.yaml
manifests/0000_31_cluster-baremetal-operator_05_rbac.yaml
manifests/0000_31_cluster-baremetal-operator_06_deployment.yaml
manifests/0000_31_cluster-baremetal-operator_07_clusteroperator.cr.yaml
manifests/0000_90_cluster-baremetal-operator_03_servicemonitor.yamlUnclear to me why the BareMetalHost CRD is at level 30. Looks like that's coming from here. Perhaps it should be adjusted to use a |
@honza This capability annotation is only required on resources managed by CVO, afaik. The BMO container, which is part of the metal3 deployment is a resource managed by CBO and hence doesn't need this annotation. We still need to add the capability annotation to the the BMH CRD because that is a manifest that is generated here : https://github.com/openshift/baremetal-operator/blob/master/Dockerfile.ocp and installed by CVO. So, adding the capability annotation to https://github.com/openshift/baremetal-operator/blob/master/config/crd/ocp/ocp_kustomization.yaml would make sure the generated manifest would have it. |
@wking Your list is correct and that is what we are working towards. We wanted to make sure manifests/0000_30_baremetal-operator_01_baremetalhost.crd.yaml is installed before the rest of the CBO manifests hence the level 30. The manifest is generated by openshift/baremetal-operator (https://github.com/openshift/baremetal-operator/blob/master/Dockerfile.ocp#L11) as you correctly pointed out hence the name of the manifest has "baremetal-operator" and not "cluster-baremetal-operator". The rest of the manifests in that list are provided by the cluster-baremetal-operator. |
Oh, yes, I remember now! |
In openshift/baremetal-operator, the annotation is added with "baremetal" (https://github.com/openshift/baremetal-operator/pull/212/files) and here in CBO it is without " ". Which one should it be? |
|
Fixed by removing the quotes. But I can't imagine that it would matter? |
The CVO collects manifests into task nodes, which share the same level/name prefix. So you currently have two task-nodes, one of |
|
/retest |
5 similar comments
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
/retest |
|
It seems to me that the errors in e2e-metal-ipi-ovn-ipv6 aren't related to the changes in this PR. Given that the other jobs passed on the first try, I think we're safe to override here. /override ci/prow/e2e-metal-ipi-ovn-ipv6 |
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dtantsur, honza 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
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/override ci/prow/e2e-metal-ipi-ovn-ipv6 |
|
@honza: Overrode contexts on behalf of honza: ci/prow/e2e-metal-ipi-ovn-ipv6 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. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
3 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@honza: all tests passed! 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. |
Thanks for the explanation. I opened a follow-up PR: openshift/baremetal-operator#216 |
The capability has already been registered with the API.