Skip to content

Conversation

@hardys
Copy link

@hardys hardys commented Mar 10, 2021

This is to align with the rebase proposed via:
openshift/baremetal-operator#133

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 10, 2021
@hardys hardys force-pushed the bmh-upstream-20210310 branch from ab0b95a to 1d67f3d Compare March 10, 2021 13:14
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hardys, kirankt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sadasu
Copy link
Contributor

sadasu commented Mar 10, 2021

This CRD change needs to be made here https://github.com/openshift/cluster-baremetal-operator/blob/master/api/v1alpha1/provisioning_types.go. "make manifests" will take care of actually generating the manifest.

@hardys
Copy link
Author

hardys commented Mar 12, 2021

This CRD change needs to be made here https://github.com/openshift/cluster-baremetal-operator/blob/master/api/v1alpha1/provisioning_types.go. "make manifests" will take care of actually generating the manifest.

This is updating the BMH CRD from the BMO repo, via hack/get-bmh-crd.sh - this just copies the yaml, and the provisioning_types.go does not contain any code definition of the BMH resource AFAICS:

$ grep type provisioning_types.go 
type ProvisioningNetwork string
type ProvisioningSpec struct {
type ProvisioningStatus struct {
type Provisioning struct {
type ProvisioningList struct {

That also wasn't the approach used in #105 #100 #78 #70 #64 #51 so perhaps @honza can confirm, but I think this is the right process?

This is to align with the rebase proposed via:
openshift/baremetal-operator#133
@hardys hardys force-pushed the bmh-upstream-20210310 branch from 1d67f3d to 077611d Compare March 12, 2021 10:37
@hardys hardys requested review from honza and kirankt March 12, 2021 10:37
@honza
Copy link
Member

honza commented Mar 12, 2021

You should be able to use the hack/get-bmh-crd.sh script to update the CRD. As mentioned above, this process isn't fully automatic. You'll need to tweak the CRD after running the script: the script deletes some annotations already in the CBO that we need to keep.

@hardys
Copy link
Author

hardys commented Mar 12, 2021

/retest

1 similar comment
@hardys
Copy link
Author

hardys commented Mar 12, 2021

/retest

@hardys
Copy link
Author

hardys commented Mar 15, 2021

/test e2e-metal-ipi

@honza
Copy link
Member

honza commented Mar 15, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 15, 2021
@sadasu
Copy link
Contributor

sadasu commented Mar 15, 2021

This CRD change needs to be made here https://github.com/openshift/cluster-baremetal-operator/blob/master/api/v1alpha1/provisioning_types.go. "make manifests" will take care of actually generating the manifest.

This is updating the BMH CRD from the BMO repo, via hack/get-bmh-crd.sh - this just copies the yaml, and the provisioning_types.go does not contain any code definition of the BMH resource AFAICS:

$ grep type provisioning_types.go 
type ProvisioningNetwork string
type ProvisioningSpec struct {
type ProvisioningStatus struct {
type Provisioning struct {
type ProvisioningList struct {

That also wasn't the approach used in #105 #100 #78 #70 #64 #51 so perhaps @honza can confirm, but I think this is the right process?

That is correct.

@openshift-merge-robot openshift-merge-robot merged commit 3bcebbe into openshift:master Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants