Skip to content

Commit

Permalink
Ensure inspection is disabled on day-2 spoke node BMHs (openshift#5406)
Browse files Browse the repository at this point in the history
Recently a validation was merged that prevents hardware details from
being set on a BMH when inspection is enabled. This is blocking BMHs
from being created on 4.14 spoke clusters when using converged flow with
the following error:

```
time="2023-08-01T17:52:14Z" level=error msg="failed to create or update spoke BareMetalHost" func="github.com/openshift/assisted-service/internal/controller/controllers.(*BMACReconciler).reconcileSpokeBMH" file="/assisted-service/internal/controller/controllers/bmh_agent_controller.go:1008" agent=a04cbfd5-729e-4103-b9e1-a97435e19420 agent_namespace=assisted-installer bare_metal_host=ostest-extraworker-0 bare_metal_host_namespace=assisted-installer error="admission webhook \"baremetalhost.metal3.io\" denied the request: inspection has to be disabled for HardwareDetailsAnnotation, check if {'inspect.metal3.io' : 'disabled'}" go-id=635 request_id=75ca96ab-30b2-4a3d-80e0-57ee370af475
```

This leads to the spoke CSR never being approved and the host never
installing.

This commit ensures that inspection is explicitly disabled on the BMH
created in the spoke cluster to avoid this validation error.

This is automatically set by our controller and typically set by users
when not using the converged flow, but with the converged flow we are
running ironic inspection.

Resolves https://issues.redhat.com/browse/MGMT-15382
  • Loading branch information
carbonin authored and danielerez committed Oct 15, 2023
1 parent c3b973e commit f56b738
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
3 changes: 3 additions & 0 deletions internal/controller/controllers/bmh_agent_controller.go
Expand Up @@ -1357,6 +1357,9 @@ func (r *BMACReconciler) newSpokeBMH(log logrus.FieldLogger, bmh *bmh_v1alpha1.B
},
}
mutateFn := func() error {
// inspection must be disabled when providing hardware details
// ensure it is always disabled even for converged
setAnnotation(&bmhSpoke.ObjectMeta, BMH_INSPECT_ANNOTATION, "disabled")
bmhSpoke.Spec = bmh.Spec
// remove the credentials from the spoke's Spec so
// that BMO will set the status to unmanaged
Expand Down
5 changes: 3 additions & 2 deletions internal/controller/controllers/bmh_agent_controller_test.go
Expand Up @@ -1068,7 +1068,7 @@ var _ = Describe("bmac reconcile", func() {
})

Context("when agent role worker and cluster deployment is set", func() {
It("should set spoke BMH when agent is not installing", func() {
It("should not create spoke BMH when agent is not installing", func() {
configMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "root-ca",
Expand Down Expand Up @@ -1105,7 +1105,7 @@ var _ = Describe("bmac reconcile", func() {
err = spokeClient.Get(ctx, types.NamespacedName{Name: machineName, Namespace: OPENSHIFT_MACHINE_API_NAMESPACE}, spokeMachine)
Expect(err).NotTo(BeNil())
})
It("should not set spoke BMH when agent is installing", func() {
It("should create spoke BMH when agent is installing", func() {
configMap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: "root-ca",
Expand Down Expand Up @@ -1143,6 +1143,7 @@ var _ = Describe("bmac reconcile", func() {
Expect(spokeBMH.ObjectMeta.Annotations).To(HaveKey(BMH_HARDWARE_DETAILS_ANNOTATION))
Expect(spokeBMH.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION]).To(Equal(updatedHost.ObjectMeta.Annotations[BMH_HARDWARE_DETAILS_ANNOTATION]))
Expect(spokeBMH.ObjectMeta.Annotations).ToNot(HaveKey(BMH_DETACHED_ANNOTATION))
Expect(spokeBMH.ObjectMeta.Annotations[BMH_INSPECT_ANNOTATION]).To(Equal("disabled"))
Expect(spokeBMH.Spec.Image).To(Equal(updatedHost.Spec.Image))
Expect(spokeBMH.Spec.ConsumerRef.Kind).To(Equal("Machine"))
Expect(spokeBMH.Spec.ConsumerRef.Name).To(Equal(machineName))
Expand Down

0 comments on commit f56b738

Please sign in to comment.