MGMT-15382: Ensure inspection is disabled on day-2 spoke node BMHs

[carbonin]

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.

List all the issues related to this PR

Resolves https://issues.redhat.com/browse/MGMT-15382

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see [CONTRIBUTING] guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

[openshift-ci-robot]

@carbonin: This pull request references MGMT-15382 which is a valid jira issue.

In response to this:

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.

List all the issues related to this PR

Resolves https://issues.redhat.com/browse/MGMT-15382

• New Feature
• Enhancement
• Bug fix
• Tests
• Documentation
• CI/CD

What environments does this code impact?

• Automation (CI, tools, etc)
• Cloud
• Operator Managed Deployments
• None

How was this code tested?

• assisted-test-infra environment
• dev-scripts environment
• Reviewer's test appreciated
• Waiting for CI to do a full test run
• Manual (Elaborate on how it was tested)
• No tests needed

Checklist

• Title and description added to both, commit and PR.
• Relevant issues have been associated (see [CONTRIBUTING] guide)
• This change does not require a documentation update (docstring, docs, README, etc)
• Does this change include unit-tests (note that code changes require unit-tests)

Reviewers Checklist

• Are the title and description (in both PR and commit) meaningful and clear?
• Is there a bug required (and linked) for this change?
• Should this PR be backported?

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.

[codecov]

Codecov Report

Merging #5406 (6a1fede) into master (c5e9d50) will increase coverage by 0.32%.
Report is 7 commits behind head on master.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5406      +/-   ##
==========================================
+ Coverage   67.59%   67.92%   +0.32%     
==========================================
  Files         226      226              
  Lines       33410    33966     +556     
==========================================
+ Hits        22584    23071     +487     
- Misses       8788     8806      +18     
- Partials     2038     2089      +51     

Files Changed	Coverage Δ
...nal/controller/controllers/bmh_agent_controller.go	75.33% <100.00%> (+0.59%)	⬆️

... and 5 files with indirect coverage changes

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avishayt, carbonin

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:

• OWNERS [avishayt,carbonin]

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]

/retest-required

Remaining retests: 0 against base HEAD 801ca4b and 2 for PR HEAD 6a1fede in total

[openshift-ci]

@carbonin: 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.

[carbonin]

/cherry-pick release-ocm-2.8

[openshift-cherrypick-robot]

@carbonin: new pull request created: #5419

In response to this:

/cherry-pick release-ocm-2.8

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.