Skip to content

bmctask controller#725

Draft
stefanhipfel wants to merge 5 commits into
mainfrom
feature/bmctask-controller
Draft

bmctask controller#725
stefanhipfel wants to merge 5 commits into
mainfrom
feature/bmctask-controller

Conversation

@stefanhipfel
Copy link
Copy Markdown
Contributor

Proposed Changes

A controller that track BMC tasks and updates the status of those tasks on the BMC CRD status.

Operators and other controllers can check the BMC Status for ongoing tasks.

Implements a dedicated controller for monitoring BMC task status with
configurable polling intervals. This controller provides consistent
task polling separate from controller lifecycle events.

Features:
- Dedicated BMCTaskReconciler with configurable poll interval (default 30s)
- Event filter to only reconcile BMCs with active tasks
- Automatic requeue for in-progress tasks
- Comprehensive test suite (15 test cases)

The controller is implemented but not yet integrated with existing
controllers, allowing for independent review and gradual rollout.

Components:
- internal/controller/bmctask_controller.go (188 lines)
- internal/controller/bmctask_controller_test.go (650+ lines)
- docs/bmc-task-tracking.md (architecture documentation)
Registers the BMCTaskReconciler with the controller manager and adds
configuration flag for poll interval.

Changes:
- cmd/main.go: Add --task-poll-interval flag (default 30s)
- cmd/main.go: Register BMCTaskReconciler with manager
- internal/controller/suite_test.go: Add controller to test suite

The controller is registered and will run but currently has no effect
as no controllers are creating tasks for it to monitor yet.
Add note indicating that BMCTask controller requires the Tasks API changes
to be functional. The controller can be merged independently and will
activate once the API changes are available.
Adds the necessary API changes for BMC task tracking:
- Add Tasks field to BMC.Status for tracking operations
- Add BMCTask type with TaskURI, TaskType, State, etc.
- Add BMCTaskType enum for different task types
- Add GetTaskStatus method to BMC interface
- Implement GetTaskStatus in Redfish implementations

These API changes enable the BMCTask controller to function.

Changes:
- api/v1alpha1/bmc_types.go: Add Tasks field and BMCTask types
- api/v1alpha1/zz_generated.deepcopy.go: Generated DeepCopy methods
- bmc/bmc.go: Add GetTaskStatus to BMC interface
- bmc/redfish.go: Implement GetTaskStatus in RedfishBaseBMC
- bmc/redfish_kube.go: Delegate GetTaskStatus to RedfishBaseBMC
- config/crd/bases/*.yaml: Generated CRD with Tasks field
- internal/controller/bmctask_controller.go: Update to use schemas.Task fields
@github-actions github-actions Bot added size/XXL api-change documentation Improvements or additions to documentation enhancement New feature or request labels Mar 6, 2026
This commit addresses all 4 CI failures blocking PR #725:

1. **Linter Error** (bmc/redfish.go:885)
   - Add nolint:errcheck comment to defer resp.Body.Close()
   - Matches existing pattern used in redfish_dell.go and redfish_lenovo.go

2. **Code Generation**
   - Regenerate API documentation with BMCTask and BMCTaskType types
   - Update docs/api-reference/api.md with 46 new lines documenting the Tasks API

3. **Coverage File Path** (Makefile)
   - Use absolute path $(PWD)/cover.out instead of relative path
   - Fixes "could not finalize profiles" error when tests run with sudo in CI

4. **Goroutine Leak** (bmctask_controller.go, suite_test.go)
   - Add context cancellation check at start of Reconcile to prevent new polls after shutdown
   - Add 200ms cleanup wait in test suite to allow in-flight reconciliations to complete
   - Prevents goroutine leaks during test teardown

All changes are defensive improvements with no impact on production behavior.
Tests compile successfully and linter reports 0 issues.
@stefanhipfel stefanhipfel requested a review from afritzler March 16, 2026 13:04
@stefanhipfel
Copy link
Copy Markdown
Contributor Author

@afritzler maybe we can discuss this in one of our meetings

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api-change area/metal-automation documentation Improvements or additions to documentation enhancement New feature or request size/XXL

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants