Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MGMT-18288: Approve CSRs for agents with baremetal platform without MAPI #7076

Merged

Conversation

CrystalChun
Copy link
Contributor

@CrystalChun CrystalChun commented Dec 12, 2024

There are no spoke bmh and machines created for a host that's installed to a cluster with baremetal platform without MAPI capability. We need to approve their CSRs too.

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?

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Dec 12, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 12, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 12, 2024

@CrystalChun: This pull request references MGMT-18288 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.19.0" version, but no target version was set.

In response to this:

There are no spoke bmh and machines created for a host that's installed to a cluster with baremetal platform without MAPI capability. We need to approve their CSRs too.

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?

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.

Copy link

openshift-ci bot commented Dec 12, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@CrystalChun
Copy link
Contributor Author

/test all

@openshift-ci openshift-ci bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 12, 2024
@openshift-ci-robot
Copy link

openshift-ci-robot commented Dec 12, 2024

@CrystalChun: This pull request references MGMT-18288 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.19.0" version, but no target version was set.

In response to this:

There are no spoke bmh and machines created for a host that's installed to a cluster with baremetal platform without MAPI capability. We need to approve their CSRs too.

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?

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-robot
Copy link

openshift-ci-robot commented Dec 12, 2024

@CrystalChun: This pull request references MGMT-18288 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.19.0" version, but no target version was set.

In response to this:

There are no spoke bmh and machines created for a host that's installed to a cluster with baremetal platform without MAPI capability. We need to approve their CSRs too.

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?

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-robot
Copy link

openshift-ci-robot commented Dec 12, 2024

@CrystalChun: This pull request references MGMT-18288 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.19.0" version, but no target version was set.

In response to this:

There are no spoke bmh and machines created for a host that's installed to a cluster with baremetal platform without MAPI capability. We need to approve their CSRs too.

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?

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-robot
Copy link

openshift-ci-robot commented Dec 12, 2024

@CrystalChun: This pull request references MGMT-18288 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.19.0" version, but no target version was set.

In response to this:

There are no spoke bmh and machines created for a host that's installed to a cluster with baremetal platform without MAPI capability. We need to approve their CSRs too.

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?

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 approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 12, 2024
Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 62.06897% with 11 lines in your changes missing coverage. Please review.

Project coverage is 67.75%. Comparing base (a5016f1) to head (eb81629).
Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
internal/controller/controllers/common.go 63.15% 4 Missing and 3 partials ⚠️
...nternal/controller/controllers/agent_controller.go 55.55% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7076      +/-   ##
==========================================
- Coverage   67.83%   67.75%   -0.09%     
==========================================
  Files         289      293       +4     
  Lines       39660    39934     +274     
==========================================
+ Hits        26905    27058     +153     
- Misses      10328    10436     +108     
- Partials     2427     2440      +13     
Files with missing lines Coverage Δ
...nal/controller/controllers/bmh_agent_controller.go 77.11% <100.00%> (+0.32%) ⬆️
...nternal/controller/controllers/agent_controller.go 76.43% <55.55%> (-0.18%) ⬇️
internal/controller/controllers/common.go 77.12% <63.15%> (-0.40%) ⬇️

... and 15 files with indirect coverage changes

@CrystalChun CrystalChun force-pushed the approve-non-mapi-csrs branch 2 times, most recently from abb77de to f601500 Compare December 12, 2024 17:59
@CrystalChun
Copy link
Contributor Author

/test all

@CrystalChun
Copy link
Contributor Author

/test ?

Copy link

openshift-ci bot commented Dec 12, 2024

@CrystalChun: The following commands are available to trigger required jobs:

/test e2e-agent-compact-ipv4
/test edge-assisted-operator-catalog-publish-verify
/test edge-ci-index
/test edge-e2e-ai-operator-ztp
/test edge-e2e-ai-operator-ztp-sno-day2-workers
/test edge-e2e-ai-operator-ztp-sno-day2-workers-late-binding
/test edge-e2e-metal-assisted-4-12
/test edge-e2e-metal-assisted-4-18
/test edge-e2e-metal-assisted-4-control-planes-4-18
/test edge-e2e-metal-assisted-5-control-planes-4-18
/test edge-e2e-metal-assisted-cnv-4-16
/test edge-e2e-metal-assisted-cnv-4-17
/test edge-e2e-metal-assisted-lvm-4-18
/test edge-e2e-metal-assisted-mtv-4-17
/test edge-e2e-metal-assisted-odf-4-16
/test edge-e2e-metal-assisted-odf-4-17
/test edge-images
/test edge-lint
/test edge-operator-publish-verify
/test edge-subsystem-aws
/test edge-subsystem-kubeapi-aws
/test edge-unit-test
/test edge-verify-generated-code
/test images
/test mce-images

The following commands are available to trigger optional jobs:

/test e2e-agent-4control-ipv4
/test e2e-agent-5control-ipv4
/test e2e-agent-ha-dualstack
/test e2e-agent-sno-ipv6
/test edge-e2e-ai-operator-disconnected-capi
/test edge-e2e-ai-operator-ztp-3masters
/test edge-e2e-ai-operator-ztp-4masters
/test edge-e2e-ai-operator-ztp-5masters
/test edge-e2e-ai-operator-ztp-capi
/test edge-e2e-ai-operator-ztp-compact-day2-masters
/test edge-e2e-ai-operator-ztp-compact-day2-workers
/test edge-e2e-ai-operator-ztp-disconnected
/test edge-e2e-ai-operator-ztp-hypershift-zero-nodes
/test edge-e2e-ai-operator-ztp-multiarch-3masters-ocp
/test edge-e2e-ai-operator-ztp-multiarch-sno-ocp
/test edge-e2e-ai-operator-ztp-node-labels
/test edge-e2e-ai-operator-ztp-remove-node
/test edge-e2e-ai-operator-ztp-sno-day2-masters
/test edge-e2e-ai-operator-ztp-sno-day2-workers-ignitionoverride
/test edge-e2e-metal-assisted-4-13
/test edge-e2e-metal-assisted-4-14
/test edge-e2e-metal-assisted-4-15
/test edge-e2e-metal-assisted-4-16
/test edge-e2e-metal-assisted-4-17
/test edge-e2e-metal-assisted-4-masters-none-4-18
/test edge-e2e-metal-assisted-bond-4-14
/test edge-e2e-metal-assisted-bond-4-18
/test edge-e2e-metal-assisted-day2-4-18
/test edge-e2e-metal-assisted-day2-arm-workers-4-18
/test edge-e2e-metal-assisted-day2-sno-4-18
/test edge-e2e-metal-assisted-external-4-14
/test edge-e2e-metal-assisted-external-4-18
/test edge-e2e-metal-assisted-ipv4v6-4-18
/test edge-e2e-metal-assisted-ipv6-4-18
/test edge-e2e-metal-assisted-kube-api-late-binding-sno-4-18
/test edge-e2e-metal-assisted-kube-api-late-unbinding-sno-4-18
/test edge-e2e-metal-assisted-kube-api-net-suite-4-18
/test edge-e2e-metal-assisted-mce-4-16
/test edge-e2e-metal-assisted-mce-sno-4-16
/test edge-e2e-metal-assisted-metallb-4-18
/test edge-e2e-metal-assisted-none-4-18
/test edge-e2e-metal-assisted-onprem-4-18
/test edge-e2e-metal-assisted-osc-4-17
/test edge-e2e-metal-assisted-osc-sno-4-17
/test edge-e2e-metal-assisted-sno-4-18
/test edge-e2e-metal-assisted-static-ip-suite-4-14
/test edge-e2e-metal-assisted-static-ip-suite-4-18
/test edge-e2e-metal-assisted-tang-4-18
/test edge-e2e-metal-assisted-tpmv2-4-18
/test edge-e2e-metal-assisted-upgrade-agent-4-18
/test edge-e2e-nutanix-assisted-2workers-4-18
/test edge-e2e-nutanix-assisted-4-14
/test edge-e2e-nutanix-assisted-4-18
/test edge-e2e-oci-assisted-4-14
/test edge-e2e-oci-assisted-4-18
/test edge-e2e-oci-assisted-iscsi-4-18
/test edge-e2e-vsphere-assisted-4-14
/test edge-e2e-vsphere-assisted-4-15
/test edge-e2e-vsphere-assisted-4-16
/test edge-e2e-vsphere-assisted-4-18
/test edge-e2e-vsphere-assisted-umn-4-18
/test okd-scos-e2e-aws-ovn
/test okd-scos-images
/test push-pr-image

Use /test all to run the following jobs that were automatically triggered:

pull-ci-openshift-assisted-service-master-e2e-agent-compact-ipv4
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-18
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-images
pull-ci-openshift-assisted-service-master-mce-images
pull-ci-openshift-assisted-service-master-okd-scos-e2e-aws-ovn

In response to this:

/test ?

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.

@CrystalChun
Copy link
Contributor Author

/test edge-e2e-ai-operator-ztp-node-labels

@CrystalChun CrystalChun marked this pull request as ready for review December 13, 2024 02:08
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 13, 2024
@CrystalChun
Copy link
Contributor Author

/test edge-e2e-ai-operator-ztp-node-labels

@CrystalChun
Copy link
Contributor Author

/cc @carbonin @eranco74

@openshift-ci openshift-ci bot requested review from carbonin and eranco74 December 13, 2024 15:04
Copy link
Member

@carbonin carbonin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any unit tests we can or should add here?

@CrystalChun CrystalChun force-pushed the approve-non-mapi-csrs branch from 87740e5 to f29e95c Compare December 13, 2024 18:04
@openshift-ci openshift-ci bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 13, 2024
@CrystalChun CrystalChun force-pushed the approve-non-mapi-csrs branch from f29e95c to 60da91b Compare December 13, 2024 18:51
@CrystalChun
Copy link
Contributor Author

Are there any unit tests we can or should add here?

Yes! Added two for the Agent controller:

  1. Approve host added to the cluster with baremetal without MAPI capability
  2. Do not approve host added to cluster with just baremetal

The BMAC has a test from the last PR.

Let me know if you think additional tests are needed!

@CrystalChun
Copy link
Contributor Author

/test edge-e2e-ai-operator-ztp-node-labels

@CrystalChun
Copy link
Contributor Author

/retest-required

1 similar comment
@CrystalChun
Copy link
Contributor Author

/retest-required

@CrystalChun
Copy link
Contributor Author

/retest

2 similar comments
@CrystalChun
Copy link
Contributor Author

/retest

@CrystalChun
Copy link
Contributor Author

/retest

},
expectedStatus: models.HostStatusInstalling,
expectedStage: models.HostStageJoined,
clusterInstall: newAciBaremetalWithoutMAPI("test-cluster-aci", testNamespace),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this say "WithoutMAPI" but the test name says "with MAPI"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, the naming is incorrect, modified it!

There are no spoke bmh and machines created for a host that's installed
to a cluster with baremetal platform without MAPI capability.
We need to approve their CSRs too.
@CrystalChun CrystalChun force-pushed the approve-non-mapi-csrs branch from 60da91b to eb81629 Compare December 17, 2024 18:30
@CrystalChun
Copy link
Contributor Author

/test edge-e2e-ai-operator-ztp-node-labels

Copy link

openshift-ci bot commented Dec 18, 2024

@CrystalChun: 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 eb81629 link false /test okd-scos-e2e-aws-ovn

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

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 18, 2024
Copy link

openshift-ci bot commented Dec 18, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: carbonin, 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,carbonin]

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

@openshift-merge-bot openshift-merge-bot bot merged commit 96a1434 into openshift:master Dec 18, 2024
19 of 20 checks passed
@openshift-bot
Copy link
Contributor

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-api-server
This PR has been included in build ose-agent-installer-api-server-container-v4.19.0-202412190006.p0.g96a1434.assembly.stream.el9.
All builds following this will include this PR.

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. 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.

5 participants