Skip to content

pull from upstream#34

Merged
openshift-merge-robot merged 28 commits into
openshift:masterfrom
dhellmann:openshift-update-bmo-20190701
Jul 2, 2019
Merged

pull from upstream#34
openshift-merge-robot merged 28 commits into
openshift:masterfrom
dhellmann:openshift-update-bmo-20190701

Conversation

@dhellmann
Copy link
Copy Markdown

No description provided.

zaneb and others added 22 commits June 17, 2019 20:37
In order to ensure that fencing works correctly, we can't allow the
Machine object to be deleted while there is still a chance that the
BaremetalHost is still powered on and running. Delay successful
completion of the deletion until the actuator has verified that the Host
cannot be online.

Closes #84

Signed-off-by: Zane Bitter <zbitter@redhat.com>
If a BareMetalHost that is associated with a Machine changes, the Machine
should be reconciled. This can enable (not yet implemented) logic such as
waiting for a host to deprovision while a Machine is being deleted.
Ensure that a Machine associated with a Host remains associated at least
until the Host has been deprovisioned when the Machine is being deleted.
This will allow the Machine controller to continue to receive events
when the Host changes (which will reduce latency when detecting the
completion of deprovisioning) and ensure that another Machine cannot
claim the Host until the current one is ready to release it.

Signed-off-by: Zane Bitter <zbitter@redhat.com>
controller watches BareMetalHosts and enqueues corresponding Machines
We now have the Hostname from HardwareDetails. This adds it to the
MachineStatus.
Delay Machine deletion until BaremetalHost deprovisioned
Bring in the latest version of baremetal-operator and its dependencies
before the version that includes the host object API change.
Only map hosts with consumers that match the version we currently
support to reconcile requests for machines.
Instead of a hard-coded fake literal, use the value from the
cluster-api code.
Set the hostname from HardwareDetails
switch to using ConsumerRef instead of MachineRef
@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jul 1, 2019
@dhellmann
Copy link
Copy Markdown
Author

/cc @mhrivnak @stbenjam @russellb

@dhellmann
Copy link
Copy Markdown
Author

This fails to build for me locally with:

$ make test
go generate ./pkg/... ./cmd/...
go fmt ./pkg/... ./cmd/...
pkg/cloud/baremetal/actuators/machine/actuator.go
go vet ./pkg/... ./cmd/...
pkg/manager/wrapper/mapper.go:6:2: cannot find package "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" in any of:
	/home/dhellmann/go/src/github.com/metal3-io/cluster-api-provider-baremetal/vendor/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1 (vendor tree)
	/usr/lib/golang/src/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1 (from $GOROOT)
	/home/dhellmann/go/src/sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1 (from $GOPATH)
pkg/manager/wrapper/wrapper.go:18:2: cannot find package "sigs.k8s.io/controller-runtime/pkg/webhook/admission/types" in any of:
	/home/dhellmann/go/src/github.com/metal3-io/cluster-api-provider-baremetal/vendor/sigs.k8s.io/controller-runtime/pkg/webhook/admission/types (vendor tree)
	/usr/lib/golang/src/sigs.k8s.io/controller-runtime/pkg/webhook/admission/types (from $GOROOT)
	/home/dhellmann/go/src/sigs.k8s.io/controller-runtime/pkg/webhook/admission/types (from $GOPATH)
make: *** [vet] Error 1

I suspect this is due to the mismatch between cluster-api versions in openshift and our upstream version of this repo. I'm not sure yet how to resolve them.

@dhellmann
Copy link
Copy Markdown
Author

Adding a few custom changes to the imports is now giving me:

$ make test
go generate ./pkg/... ./cmd/...
go fmt ./pkg/... ./cmd/...
go vet ./pkg/... ./cmd/...
# github.com/metal3-io/cluster-api-provider-baremetal/pkg/manager/wrapper
pkg/manager/wrapper/wrapper.go:25:3: cannot use managerWrapper literal (type *managerWrapper) as type manager.Manager in return argument:
	*managerWrapper does not implement manager.Manager (missing GetAPIReader method)
pkg/manager/wrapper/wrapper.go:85:18: m.manager.GetAdmissionDecoder undefined (type manager.Manager has no field or method GetAdmissionDecoder)
pkg/manager/wrapper/wrapper.go:105:18: m.manager.GetRecorder undefined (type manager.Manager has no field or method GetRecorder)
# github.com/metal3-io/cluster-api-provider-baremetal/pkg/manager/wrapper [github.com/metal3-io/cluster-api-provider-baremetal/pkg/manager/wrapper.test]
pkg/manager/wrapper/wrapper.go:25:3: cannot use managerWrapper literal (type *managerWrapper) as type manager.Manager in return argument:
	*managerWrapper does not implement manager.Manager (missing GetAPIReader method)
pkg/manager/wrapper/wrapper.go:85:18: m.manager.GetAdmissionDecoder undefined (type manager.Manager has no field or method GetAdmissionDecoder)
pkg/manager/wrapper/wrapper.go:105:18: m.manager.GetRecorder undefined (type manager.Manager has no field or method GetRecorder)
make: *** [vet] Error 2

@mhrivnak
Copy link
Copy Markdown
Member

mhrivnak commented Jul 2, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jul 2, 2019
@openshift-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhellmann, mhrivnak

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-merge-robot openshift-merge-robot merged commit bc8436a into openshift:master Jul 2, 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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants