Skip to content

MGMT-21810: Support TNF cluster installation using kube-api - #8085

Merged
carbonin merged 1 commit into
openshift:masterfrom
giladravid16:MGMT-21810
Oct 22, 2025
Merged

carbonin merged 1 commit into
openshift:masterfrom
giladravid16:MGMT-21810

Conversation

@giladravid16

@giladravid16 giladravid16 commented Sep 30, 2025 •

Copy link
Copy Markdown
Contributor

Allow installing TNF clusters using the kube-api as described in our enhancement.

I deployed a cluster with MCE using my image and tested the following:

  1. Setting an agent's spec.fencingCredentialsSecretName and checking that the correct credentials were set for the host's fencing_credentials. I tired with and without certificateVerification in the secret.
  2. Setting an agent's spec.fencingCredentialsSecretName to a none existing secret and checking that the correct error was in the agent's status.
  3. Setting a label on the BMH for the fencing credentials secret name in order to simulate ZTP and checking that it was set on the agent.
  4. Without setting the agent's spec.fencingCredentialsSecretName and checking that the fencing credentials were set from the BMH and it's secret. I tried with and without disableCertificateVerification on the BMH.
  5. Late binding a host with fencing credentials only works if the cluster's openshift version is at least 4.20.

List all the issues related to this PR

Closes MGMT-21810

  • 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 openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Sep 30, 2025
@openshift-ci-robot

openshift-ci-robot commented Sep 30, 2025 •

Copy link
Copy Markdown

@giladravid16: This pull request references MGMT-21810 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Allow installing TNF clusters using the kube-api as described in our enhancement.

I deployed a cluster with MCE using my image and tested the following:

  1. Setting an agent's spec.fencingCredentialsSecretName and checking that the correct credentials were set for the host's fencing_credentials. I tired with and without certificateVerification in the secret.
  2. Setting an agent's spec.fencingCredentialsSecretName to a none existing secret and checking that the correct error was in the agent's status.
  3. Setting a label on the BMH for the fencing credentials secret name in order to simulate ZTP and checking that it was set on the agent.
  4. Without setting the agent's spec.fencingCredentialsSecretName and checking that the fencing credentials were set from the BMH and it's secret. I tried with and without disableCertificateVerification on the BMH.

List all the issues related to this PR

Closes MGMT-21810

  • 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 openshift-eng/jira-lifecycle-plugin repository.

1 similar comment
@openshift-ci-robot

openshift-ci-robot commented Sep 30, 2025 •

Copy link
Copy Markdown

@giladravid16: This pull request references MGMT-21810 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Allow installing TNF clusters using the kube-api as described in our enhancement.

I deployed a cluster with MCE using my image and tested the following:

  1. Setting an agent's spec.fencingCredentialsSecretName and checking that the correct credentials were set for the host's fencing_credentials. I tired with and without certificateVerification in the secret.
  2. Setting an agent's spec.fencingCredentialsSecretName to a none existing secret and checking that the correct error was in the agent's status.
  3. Setting a label on the BMH for the fencing credentials secret name in order to simulate ZTP and checking that it was set on the agent.
  4. Without setting the agent's spec.fencingCredentialsSecretName and checking that the fencing credentials were set from the BMH and it's secret. I tried with and without disableCertificateVerification on the BMH.

List all the issues related to this PR

Closes MGMT-21810

  • 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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci openshift-ci Bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 30, 2025
@openshift-ci openshift-ci Bot added api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Sep 30, 2025
@codecov

codecov Bot commented Sep 30, 2025 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.72727% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.16%. Comparing base (0798b59) to head (fce84b4).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
...nal/controller/controllers/bmh_agent_controller.go 73.33% 11 Missing and 5 partials ⚠️
...nternal/controller/controllers/agent_controller.go 72.72% 8 Missing and 4 partials ⚠️
internal/bminventory/inventory.go 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8085      +/-   ##
==========================================
+ Coverage   42.87%   43.16%   +0.29%     
==========================================
  Files         403      404       +1     
  Lines       69491    69869     +378     
==========================================
+ Hits        29793    30161     +368     
+ Misses      37020    37006      -14     
- Partials     2678     2702      +24     
Files with missing lines Coverage Δ
internal/bminventory/inventory.go 71.79% <66.66%> (+0.30%) ⬆️
...nternal/controller/controllers/agent_controller.go 76.81% <72.72%> (+0.07%) ⬆️
...nal/controller/controllers/bmh_agent_controller.go 75.75% <73.33%> (-0.20%) ⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@openshift-ci-robot

openshift-ci-robot commented Sep 30, 2025 •

Copy link
Copy Markdown

@giladravid16: This pull request references MGMT-21810 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.21.0" version, but no target version was set.

Details

In response to this:

Allow installing TNF clusters using the kube-api as described in our enhancement.

I deployed a cluster with MCE using my image and tested the following:

  1. Setting an agent's spec.fencingCredentialsSecretName and checking that the correct credentials were set for the host's fencing_credentials. I tired with and without certificateVerification in the secret.
  2. Setting an agent's spec.fencingCredentialsSecretName to a none existing secret and checking that the correct error was in the agent's status.
  3. Setting a label on the BMH for the fencing credentials secret name in order to simulate ZTP and checking that it was set on the agent.
  4. Without setting the agent's spec.fencingCredentialsSecretName and checking that the fencing credentials were set from the BMH and it's secret. I tried with and without disableCertificateVerification on the BMH.
  5. Late binding a host with fencing credentials only works if the cluster's openshift version is at least 4.20.

List all the issues related to this PR

Closes MGMT-21810

  • 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 openshift-eng/jira-lifecycle-plugin repository.

Comment thread internal/controller/controllers/agent_controller.go Outdated
Comment thread internal/controller/controllers/agent_controller.go Outdated
@openshift-ci openshift-ci Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 5, 2025
Comment thread internal/controller/controllers/bmh_agent_controller.go Outdated
Comment thread internal/controller/controllers/common.go Outdated
Comment thread api/v1beta1/agent_types.go Outdated
@openshift-ci openshift-ci Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 19, 2025
@giladravid16
giladravid16 force-pushed the MGMT-21810 branch 2 times, most recently from a47e633 to 7b1664a Compare October 19, 2025 14:29
Comment thread internal/controller/controllers/bmh_agent_controller.go Outdated
Comment thread internal/controller/controllers/bmh_agent_controller.go Outdated
Comment thread internal/controller/controllers/bmh_agent_controller.go
Comment thread internal/controller/controllers/bmh_agent_controller.go Outdated
Comment thread internal/controller/controllers/bmh_agent_controller.go Outdated
Comment thread internal/controller/controllers/bmh_agent_controller.go Outdated
@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Oct 21, 2025
@openshift-ci

openshift-ci Bot commented Oct 21, 2025

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

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

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:
  • OWNERS [carbonin,giladravid16]

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

Copy link
Copy Markdown

/retest-required

Remaining retests: 0 against base HEAD f456043 and 2 for PR HEAD fce84b4 in total

@giladravid16

Copy link
Copy Markdown
Contributor Author

/retest-required

@gamli75

gamli75 commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

/Override ci/prow/e2e-ai-operator-disconnected-capi ci/prow/e2e-ai-operator-ztp-capi

@openshift-ci

openshift-ci Bot commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

@gamli75: /override requires failed status contexts, check run or a prowjob name to operate on.
The following unknown contexts/checkruns were given:

  • ci/prow/e2e-ai-operator-disconnected-capi
  • ci/prow/e2e-ai-operator-ztp-capi

Only the following failed contexts/checkruns were expected:

  • ci/prow/e2e-agent-compact-ipv4
  • ci/prow/edge-assisted-operator-catalog-publish-verify
  • ci/prow/edge-ci-index
  • ci/prow/edge-e2e-ai-operator-disconnected-capi
  • ci/prow/edge-e2e-ai-operator-ztp
  • ci/prow/edge-e2e-ai-operator-ztp-capi
  • ci/prow/edge-e2e-metal-assisted-4-20
  • ci/prow/edge-images
  • ci/prow/edge-lint
  • ci/prow/edge-subsystem-aws
  • ci/prow/edge-subsystem-kubeapi-aws
  • ci/prow/edge-unit-test
  • ci/prow/edge-verify-generated-code
  • ci/prow/images
  • ci/prow/mce-images
  • ci/prow/okd-scos-e2e-aws-ovn
  • ci/prow/okd-scos-images
  • ci/prow/verify-deps
  • pull-ci-openshift-assisted-service-assisted-version-placeholder-images
  • pull-ci-openshift-assisted-service-master-e2e-agent-compact-ipv4
  • pull-ci-openshift-assisted-service-master-edge-assisted-operator-catalog-publish-verify
  • pull-ci-openshift-assisted-service-master-edge-ci-index
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-disconnected-capi
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp
  • pull-ci-openshift-assisted-service-master-edge-e2e-ai-operator-ztp-capi
  • pull-ci-openshift-assisted-service-master-edge-e2e-metal-assisted-4-20
  • pull-ci-openshift-assisted-service-master-edge-images
  • pull-ci-openshift-assisted-service-master-edge-lint
  • pull-ci-openshift-assisted-service-master-edge-subsystem-aws
  • pull-ci-openshift-assisted-service-master-edge-subsystem-kubeapi-aws
  • pull-ci-openshift-assisted-service-master-edge-unit-test
  • pull-ci-openshift-assisted-service-master-edge-verify-generated-code
  • pull-ci-openshift-assisted-service-master-mce-images
  • pull-ci-openshift-assisted-service-master-okd-scos-e2e-aws-ovn
  • pull-ci-openshift-assisted-service-master-okd-scos-images
  • pull-ci-openshift-assisted-service-master-verify-deps
  • tide

If you are trying to override a checkrun that has a space in it, you must put a double quote on the context.

Details

In response to this:

/Override ci/prow/e2e-ai-operator-disconnected-capi ci/prow/e2e-ai-operator-ztp-capi

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-sigs/prow repository.

@giladravid16

Copy link
Copy Markdown
Contributor Author

/override ci/prow/edge-e2e-ai-operator-disconnected-capi ci/prow/edge-e2e-ai-operator-ztp-capi

@openshift-ci

openshift-ci Bot commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

@giladravid16: Overrode contexts on behalf of giladravid16: ci/prow/edge-e2e-ai-operator-disconnected-capi, ci/prow/edge-e2e-ai-operator-ztp-capi

Details

In response to this:

/override ci/prow/edge-e2e-ai-operator-disconnected-capi ci/prow/edge-e2e-ai-operator-ztp-capi

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-sigs/prow repository.

@giladravid16

Copy link
Copy Markdown
Contributor Author

/override ci/prow/edge-e2e-ai-operator-disconnected-capi ci/prow/edge-e2e-ai-operator-ztp-capi

@openshift-ci

openshift-ci Bot commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

@giladravid16: Overrode contexts on behalf of giladravid16: ci/prow/edge-e2e-ai-operator-disconnected-capi, ci/prow/edge-e2e-ai-operator-ztp-capi

Details

In response to this:

/override ci/prow/edge-e2e-ai-operator-disconnected-capi ci/prow/edge-e2e-ai-operator-ztp-capi

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-sigs/prow repository.

@openshift-ci

openshift-ci Bot commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

@giladravid16: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/okd-scos-e2e-aws-ovn fce84b4 link false /test okd-scos-e2e-aws-ovn

Full PR test history. Your PR dashboard.

Details

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-sigs/prow repository. I understand the commands that are listed here.

@carbonin

Copy link
Copy Markdown
Member

Need to get this in before feature freeze so I'm merging it manually.
It was previously in the merge pool and it's been testing for over 24 hours 😫

@carbonin
carbonin merged commit 3934e5b into openshift:master Oct 22, 2025
21 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-review Categorizes an issue or PR as actively needing an API review. approved Indicates a PR has been approved by an approver from all required OWNERS files. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants