OCPBUGS-7076: Copy Day2 BMH if Agent is installing

[CrystalChun]

OCPBUGS-7076
Previously the BMAC would copy the BMH and
Machine CRs of a Day2 worker to the spoke cluster before
the Agent began installing. If the Agent doesn't
start installing within the two hours these CRs
were copied over, the CSRs for these nodes will
not be approved by the machine approver pod.

To prevent this from happening, only copy
the BMH and Machine CRs to the spoke cluster
when the Agent has started installing.

List all the issues related to this PR

• 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?

/cc @eranco74

[openshift-ci-robot]

@CrystalChun: This pull request references Jira Issue OCPBUGS-7076, which is invalid:

• expected the bug to target the "4.14.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

OCPBUGS-7076
Previously the BMAC would copy the BMH and
Machine CRs of a Day2 worker to the spoke cluster before
the Agent began installing. If the Agent doesn't
start installing within the two hours these CRs
were copied over, the CSRs for these nodes will
not be approved by the machine approver pod.

To prevent this from happening, only copy
the BMH and Machine CRs to the spoke cluster
when the Agent has started installing.

List all the issues related to this PR

• 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?

/cc @eranco74

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 #5250 (8ae647f) into master (43219b9) will increase coverage by 1.44%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5250      +/-   ##
==========================================
+ Coverage   67.46%   68.91%   +1.44%     
==========================================
  Files         218      221       +3     
  Lines       32714    35497    +2783     
==========================================
+ Hits        22072    24462    +2390     
- Misses       8656     8915     +259     
- Partials     1986     2120     +134     

Impacted Files	Coverage Δ
...nal/controller/controllers/bmh_agent_controller.go	74.88% <100.00%> (+0.08%)	⬆️

... and 27 files with indirect coverage changes

[avishayt]

Why not wait until the node is fully installed? What happens if the installation itself takes more than 2-3 hours (for example, a long time to pull images, or the host booted into the wrong disk and it took time to address)?

[CrystalChun]

Why not wait until the node is fully installed? What happens if the installation itself takes more than 2-3 hours (for example, a long time to pull images, or the host booted into the wrong disk and it took time to address)?

@avishayt Thank you for the feedback!

I've tested this out by only checking for HostStatusInstalled, as suggested, and observed that the Agent gets stuck in Rebooting with the state installing-pending-user-action. The install never completes.

The spoke cluster's machine approver pod errors out with

E0602 00:52:57.290085       1 csr_check.go:263] csr-xbfck: failed to find machine for node extraworker-1, cannot approve
I0602 00:52:57.290119       1 controller.go:233] csr-xbfck: CSR not authorized

where extraworker-1 is my day 2 worker node that never gets installed.

Please correct me if I'm wrong, but it seems like copying the BMH & Machine of the day 2 worker node must be done while the Agent is in the installing phase. Otherwise, the CSR will never be approved and the installation will never complete.

[avishayt]

Your PR looks good - sorry for the noise. It slipped my mind that we won't set the status to installed until the node is Ready, and that won't happen until the CSR is approved.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

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

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 [CrystalChun,avishayt]

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

[CrystalChun]

Your PR looks good - sorry for the noise. It slipped my mind that we won't set the status to installed until the node is Ready, and that won't happen until the CSR is approved.

No problem! Thank you again for the review!

[CrystalChun]

/jira refresh

[openshift-ci-robot]

@CrystalChun: This pull request references Jira Issue OCPBUGS-7076, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.14.0) matches configured target version for branch (4.14.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @chadcrum

In response to this:

/jira refresh

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.

[openshift-ci]

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

[openshift-ci-robot]

@CrystalChun: Jira Issue OCPBUGS-7076: All pull requests linked via external trackers have merged:

• openshift/assisted-service#5250

Jira Issue OCPBUGS-7076 has been moved to the MODIFIED state.

In response to this:

OCPBUGS-7076
Previously the BMAC would copy the BMH and
Machine CRs of a Day2 worker to the spoke cluster before
the Agent began installing. If the Agent doesn't
start installing within the two hours these CRs
were copied over, the CSRs for these nodes will
not be approved by the machine approver pod.

To prevent this from happening, only copy
the BMH and Machine CRs to the spoke cluster
when the Agent has started installing.

List all the issues related to this PR

• 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?

/cc @eranco74

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.

[CrystalChun]

/cherry-pick release-ocm-2.7

[openshift-cherrypick-robot]

@CrystalChun: new pull request created: #5283

In response to this:

/cherry-pick release-ocm-2.7

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.

[CrystalChun]

/cherry-pick release-ocm-2.8

[openshift-cherrypick-robot]

@CrystalChun: new pull request created: #5286

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.

[openshift-merge-robot]

Fix included in accepted release 4.14.0-0.nightly-2023-09-11-201102

[openshift-merge-robot]

Fix included in accepted release 4.14.0-0.nightly-2023-09-12-024050

[openshift-merge-robot]

Fix included in accepted release 4.14.0-0.nightly-2023-09-15-101929