Skip to content

Conversation

@russellb
Copy link
Contributor

This patch introduces support for the managed bare metal platform that
is currently under development under the metalkube github org.

More information about this platform type can be found in
documentation under https://github.com/metalkube/metalkube-docs/.

The actuator and other supporting components are still under active
devleopment, so we just point to an upstream metalkube image for the
actuator for now. Having this in the machine-api-operator earlier
will make end-to-end testing a bit easier as the platform continues to
be developed.

Signed-off-by: Russell Bryant rbryant@redhat.com

@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 27, 2019
from:
kind: DockerImage
name: docker.io/openshift/origin-libvirt-machine-controllers:v4.0.0
- name: baremetal-machine-controllers
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

until your image is built like all other release images in our payload, this will fail CI testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, Derek. I'll look into that next, then.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Images would be mirrored to docker.io once openshift/release#3066 is merged

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until ART has published baremetal-machine-controllers to quay this PR cannot be merged.

oc get istag -n ocp 4.0-art-latest:baremetal-machine-controllers
Error from server (NotFound): imagestreamtags.image.openshift.io "4.0-art-latest:baremetal-machine-controllers" not found

Please work with the aos-art channel

@russellb
Copy link
Contributor Author

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 28, 2019
@enxebre
Copy link
Member

enxebre commented Feb 28, 2019

/approve
overall looks good to me.
@russellb FYI for e2e testing we are building a suite https://github.com/openshift/cluster-api-actuator-pkg/tree/master/pkg/e2e which validates OpenShift/machine-API conformance user stories regardless of the implementation details.

@openshift-ci-robot
Copy link
Contributor

[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

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

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 28, 2019
@ingvagabund
Copy link
Member

ingvagabund commented Feb 28, 2019

@russellb I am afraid you will need to rebase after #220 gets merged

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 3, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 4, 2019
@russellb
Copy link
Contributor Author

russellb commented Mar 4, 2019

I've rebased this on master after #220 merged.

Alberto, thanks for taking a look! I'll file an issue that we need to integrate with that test suite at some point.

This is still on hold, pending getting an openshift fork of our actuator created and built as part of the openshift payload. I've just asked someone to help me with that this week.

@vrutkovs
Copy link

vrutkovs commented Mar 8, 2019

/retest

@vrutkovs
Copy link

vrutkovs commented Mar 8, 2019

mage file "/tmp/release-image-0.0.1-2019-03-08-202921826698875/machine-api-operator/image-references" referenced image "baremetal-machine-controllers" that is not part of the input images

That's odd

@booxter
Copy link

booxter commented Mar 9, 2019

@russellb is there a known technique on how to plug this into metalkube/dev-scripts flow? I suppose one would need to fetch the PR, build the machine operator image with the local checkout, probably push a custom image somewhere to Quay; but how one would plug it into kni-installer?

@vrutkovs
Copy link

vrutkovs commented Mar 9, 2019

Once the CI would build a release image (ci/prow/images test passes) this release can be used in dev-scripts - pass OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE=registry.svc.ci.openshift.org/openshift... to kni-installer.

@vrutkovs
Copy link

/retest

@smarterclayton
Copy link
Contributor

Continue to iterate, but putting

/hold

so this isn't accidentally merged until bare metal actuators is in ART.

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2019
@openshift-ci-robot openshift-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 13, 2019
@russellb
Copy link
Contributor Author

I've rebased, and also updated the image references to refer to the openshift image, since we've now forked this actuator into openshift

- name: baremetal-machine-controllers
from:
kind: DockerImage
name: docker.io/openshift/origin-baremetal-machine-controllers:v4.0.0

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
name: docker.io/openshift/origin-baremetal-machine-controllers:v4.0.0
name: quay.io/openshift/origin-baremetal-machine-controllers:v4.0.0

New images are no longer mirrored to docker.io

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I completed s/docker/quay/ in the patch

@russellb
Copy link
Contributor Author

russellb commented Mar 19, 2019

/test e2e-aws-operator

@vrutkovs
Copy link

/test e2e-aws

@ingvagabund
Copy link
Member

@russellb is the PR ready for review? I still see the hold label.

@russellb
Copy link
Contributor Author

@ingvagabund I think so. The actuator was added by ART and it's not breaking any CI jobs.

CC @smarterclayton since you put the /hold

This patch introduces support for the managed bare metal platform that
is currently under development under the metalkube github org.

More information about this platform type can be found in
documentation under https://github.com/metalkube/metalkube-docs/.
@vrutkovs
Copy link

/retest

@vrutkovs
Copy link

/test ci/prow/images

@vrutkovs
Copy link

/retest

@vrutkovs
Copy link

baremetal-machine-controllers was added to 4.0-art-latest-2019-03-22-164329
/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 22, 2019
@russellb
Copy link
Contributor Author

/retest

1 similar comment
@vrutkovs
Copy link

/retest

@vrutkovs
Copy link

/test ci/prow/images

@ingvagabund
Copy link
Member

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 26, 2019
@openshift-merge-robot openshift-merge-robot merged commit 9d4bb9c into openshift:master Mar 26, 2019
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants