From 117599a06b160bd7a44c361f62fa1a3d0de9219d Mon Sep 17 00:00:00 2001 From: Valentin Daviot Date: Wed, 1 Apr 2026 16:25:45 +0200 Subject: [PATCH 1/2] documentation: add basic AI generated doc Signed-off-by: Valentin Daviot --- CODEOWNERS | 2 + CONTRIBUTING.md | 187 ++++++++++++++++++++++++++++++++++ LICENSE | 199 +++++++++++++++++++++++++++++++++++++ README.md | 259 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 647 insertions(+) create mode 100644 CODEOWNERS create mode 100644 CONTRIBUTING.md create mode 100644 LICENSE diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 0000000..79f7afe --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,2 @@ +# Global owners +* @scality/metalk8s diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..64c6e76 --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,187 @@ +# Contributing to disk-management-agent + +Thank you for your interest in contributing. This document explains the +development workflow, coding standards, and process for submitting changes. + +## Code of Conduct + +All participants are expected to treat each other with respect and +professionalism. Harassment, discrimination, and disruptive behavior will not be +tolerated. Be constructive in code reviews and discussions. + +## Development Workflow + +1. Fork the repository and clone your fork. +2. Create a feature branch from `main`: + ```bash + git checkout -b feature/my-change + ``` +3. Make your changes, ensuring they follow the coding standards below. +4. Run the full validation suite before pushing: + ```bash + make manifests generate fmt vet lint test + ``` +5. Push your branch and open a pull request against `main`. + +## Prerequisites + +| Tool | Version | Purpose | +|---|---|---| +| Go | 1.25+ | Build and test | +| Make | any | Build system | +| Docker | any | Container image builds | +| Kind | any | End-to-end tests | +| kubectl | any | Cluster interaction | + +All other tools (`controller-gen`, `kustomize`, `setup-envtest`, `golangci-lint`) +are downloaded automatically by the Makefile into `./bin/`. + +## Setting Up the Development Environment + +```bash +# Clone and enter the repository +git clone https://github.com/scality/disk-management-agent.git +cd disk-management-agent + +# Download Go dependencies +go mod download + +# Generate manifests, deepcopy, format, and vet +make manifests generate fmt vet + +# Run unit tests to confirm everything works +make test +``` + +## Coding Standards + +### Clean Architecture + +The project follows clean architecture under `pkg/`: + +- **`pkg/domain/`** -- Core business entities. No external dependencies. +- **`pkg/service/`** -- Interface definitions (ports). Use cases depend on these. +- **`pkg/usecase/`** -- Application business logic. Must not import + infrastructure packages. +- **`pkg/infrastructure/`** -- Adapters that implement service interfaces. + +Dependency direction is always **inward**: infrastructure depends on use cases +and service interfaces, never the reverse. + +### Interface Naming + +Interfaces follow the `er` pattern: + +```go +type PhysicalDriveDiscoverer interface { ... } +type DiscoveredDriveCacheReader interface { ... } +``` + +Composite interfaces (repositories, services) may use broader names but must +embed small, focused interfaces. + +### Error Handling + +Wrap all errors with context using `errors.Wrap` or `fmt.Errorf` with `%w`: + +```go +return errors.Wrap(err, fmt.Sprintf("disk %s not accessible", diskID)) +``` + +Include relevant identifiers. Avoid duplicating context already present in the +error chain. + +### Interface Size + +Interfaces are limited to 1-2 methods. Only composite interfaces may have more +through embedding. + +### Linting + +The project uses [golangci-lint](https://golangci-lint.run/) with the +configuration in `.golangci.yml`. Run it locally before pushing: + +```bash +make lint +``` + +## Testing + +### Unit Tests + +Unit tests and controller tests (using envtest) run together: + +```bash +make test +``` + +Coverage output is written to `cover.out`. + +### End-to-End Tests + +E2E tests require a Kind cluster. The Makefile manages the cluster lifecycle: + +```bash +make test-e2e +``` + +This creates a Kind cluster named `disk-management-agent-test-e2e`, runs the +tests, and tears it down automatically. + +### Writing Tests + +- Controller tests use the + [envtest](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/envtest) + framework and Ginkgo/Gomega. +- Use case tests use [testify](https://github.com/stretchr/testify) with mock + implementations of service interfaces. +- Test files live alongside the code they test (`_test.go` suffix). + +## Pull Request Process + +1. Ensure all CI checks pass (lint + tests). +2. Keep PRs focused: one logical change per PR. +3. Write a clear PR description explaining **what** changed and **why**. +4. At least one approval from a code owner is required before merging. +5. Squash-merge is preferred for a clean commit history. + +### Branch Naming + +Use descriptive prefixes: + +- `feature/` -- New functionality +- `fix/` -- Bug fixes +- `improvement/` -- Refactoring or enhancements +- `docs/` -- Documentation changes + +## Issue Reporting + +### Bug Reports + +When reporting a bug, include: + +- Steps to reproduce +- Expected behavior +- Actual behavior +- Environment details (Go version, Kubernetes version, RAID controller type) +- Relevant logs or error messages + +### Feature Requests + +Describe the use case, the expected behavior, and why the existing functionality +does not cover it. + +## Documentation + +When making code changes, update relevant documentation: + +- If you add or modify CRD fields, regenerate manifests with `make manifests`. +- If you add environment variables, update the configuration table in + `README.md`. +- If you change architecture or add components, update the architecture diagram + in `README.md`. + +## License + +By contributing to this project, you agree that your contributions will be +licensed under the [Apache License 2.0](LICENSE). diff --git a/LICENSE b/LICENSE new file mode 100644 index 0000000..b50c829 --- /dev/null +++ b/LICENSE @@ -0,0 +1,199 @@ + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to the Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by the Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding any notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. Please also get an approval + for the project name and scheme from your group. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied. See the License for the specific language governing + permissions and limitations under the License. diff --git a/README.md b/README.md index 6fd70ea..0f6c0a0 100644 --- a/README.md +++ b/README.md @@ -1 +1,260 @@ # disk-management-agent + +[![Tests](https://github.com/scality/disk-management-agent/actions/workflows/test.yml/badge.svg)](https://github.com/scality/disk-management-agent/actions/workflows/test.yml) +[![Lint](https://github.com/scality/disk-management-agent/actions/workflows/lint.yml/badge.svg)](https://github.com/scality/disk-management-agent/actions/workflows/lint.yml) +[![Go Version](https://img.shields.io/github/go-mod/go-version/scality/disk-management-agent)](go.mod) +[![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) + +A Kubernetes node agent that discovers physical HDD drives behind hardware RAID +controllers and represents them as cluster-scoped `DiscoveredPhysicalDisk` +custom resources. + +Built with [Operator SDK](https://sdk.operatorframework.io/) and +[raidmgmt](https://github.com/scality/raidmgmt), the agent supports +**MegaRAID** (via `storcli64` / `perccli64`) and **HPE Smart Array** (via +`ssacli`) controllers. + +## Why + +Storage orchestration platforms such as +[MetalK8s](https://github.com/scality/metalk8s) need a reliable, Kubernetes-native +inventory of the physical disks available on each node. This agent runs as a +DaemonSet, periodically queries the hardware RAID CLIs installed on the host, +and publishes the results as `DiscoveredPhysicalDisk` CRs that higher-level +controllers can consume. + +## Architecture + +```mermaid +flowchart TD + subgraph Node["Node Agent (per node)"] + Ticker["DiscoveryTicker\n(every 5 min)"] + DiscoverUC["DiscoverPhysicalDrives\n(use case)"] + Cache["InMemory Cache"] + ReconcileUC["ReconcileDiscoveredPhysicalDisk\n(use case)"] + Reconciler["K8s Reconciler"] + end + + subgraph RAID["RAID CLI Tools"] + Storcli["storcli64"] + Perccli["perccli64"] + Ssacli["ssacli"] + end + + subgraph K8s["Kubernetes API"] + CRD["DiscoveredPhysicalDisk CRs"] + end + + Ticker -->|"triggers"| DiscoverUC + DiscoverUC -->|"calls via raidmgmt"| Storcli + DiscoverUC -->|"calls via raidmgmt"| Perccli + DiscoverUC -->|"calls via raidmgmt"| Ssacli + DiscoverUC -->|"Replace"| Cache + DiscoverUC -->|"Get/Create"| CRD + Ticker -->|"GenericEvent"| Reconciler + Reconciler -->|"Execute"| ReconcileUC + ReconcileUC -->|"Load"| Cache + Reconciler -->|"Status Update"| CRD +``` + +1. A **DiscoveryTicker** fires every 5 minutes and runs the + **DiscoverPhysicalDrives** use case. +2. The use case invokes every registered `PhysicalDriveDiscoverer` and + `LogicalVolumeDiscoverer` (MegaRAID + Smart Array adapters), filters to HDD + only, enriches device paths from logical volume metadata, replaces the + in-memory cache, and creates any missing `DiscoveredPhysicalDisk` CRs. +3. For CRs that already exist the ticker sends a `GenericEvent` to the + **Reconciler**, which reads the latest snapshot from the cache and updates + the CR `.status` (vendor, model, serial, size, paths, etc.). +4. A **validating webhook** restricts CR creation/update to the agent's own + service account. + +## CRD Reference + +**Group:** `metalk8s.scality.com` +**Version:** `v1alpha1` +**Kind:** `DiscoveredPhysicalDisk` +**Scope:** Cluster + +### Spec (immutable, set at creation) + +| Field | Type | Description | +|---|---|---| +| `spec.nodeName` | `string` | Node where the disk was discovered | +| `spec.controller.type` | `string` | RAID controller type (`MegaRAID`, `SmartArray`) | +| `spec.controller.id` | `int` | Controller index | +| `spec.id` | `string` | Disk identifier as reported by the controller | +| `spec.slot.port` | `string` | Port number | +| `spec.slot.enclosure` | `string` | Enclosure number | +| `spec.slot.bay` | `string` | Bay number | + +### Status (updated by the reconciler) + +| Field | Type | Description | +|---|---|---| +| `status.available` | `*bool` | Whether the drive is present in the slot | +| `status.vendor` | `*string` | Disk manufacturer | +| `status.model` | `*string` | Disk model name | +| `status.serial` | `*string` | Disk serial number | +| `status.wwn` | `*string` | World Wide Name | +| `status.size` | `*uint64` | Capacity in bytes | +| `status.type` | `*string` | Media type: `HDD`, `SSD`, or `NVMe` | +| `status.jbod` | `*bool` | Whether the disk is in JBOD (passthrough) mode | +| `status.status` | `*string` | Current disk status | +| `status.reason` | `*string` | Additional context for the status | +| `status.devicePath` | `*string` | OS device path (e.g. `/dev/sda`) | +| `status.permanentPath` | `*string` | Stable device path (e.g. `/dev/disk/by-id/wwn-0x...`) | + +### Example + +```yaml +apiVersion: metalk8s.scality.com/v1alpha1 +kind: DiscoveredPhysicalDisk +metadata: + name: node1-megaraid-0-0-32-0 +spec: + nodeName: node1 + controller: + type: MegaRAID + id: 0 + id: "0:32:0" + slot: + port: "0" + enclosure: "32" + bay: "0" +status: + available: true + vendor: SEAGATE + model: ST1000NX0453 + serial: WFK1234 + wwn: "0x5000c500abcdef01" + size: 1000204886016 + type: HDD + jbod: true + status: Online + devicePath: /dev/sda + permanentPath: /dev/disk/by-id/wwn-0x5000c500abcdef01 +``` + +## Configuration + +The agent is configured through environment variables: + +| Variable | Required | Default | Description | +|---|---|---|---| +| `NODE_NAME` | Yes | -- | Kubernetes node name (typically injected via the downward API) | +| `POD_NAMESPACE` | No | -- | Pod namespace, used to build the webhook allowed service account | +| `POD_SERVICE_ACCOUNT` | No | -- | Pod service account name, used for webhook authorization | +| `STORCLI_PATH` | No | `/host/libexec/MegaRAID/storcli/storcli64` | Path to the storcli binary | +| `PERCCLI_PATH` | No | `/host/libexec/MegaRAID/perccli/perccli64` | Path to the perccli binary | +| `SSACLI_PATH` | No | `/host/libexec/ssacli` | Path to the ssacli binary | + +The application version is injected at build time via ldflags (defaults to +`dev`). + +## Getting Started + +### Prerequisites + +- Go 1.25+ +- A Kubernetes cluster (v1.33+) +- `make` +- Docker or compatible container runtime +- At least one supported RAID CLI tool installed on the target nodes + +### Build + +```bash +make build +``` + +### Install CRDs + +```bash +make install +``` + +### Run locally (out-of-cluster, uses current kubeconfig) + +```bash +NODE_NAME=my-node make run +``` + +### Build and push the container image + +```bash +make docker-build docker-push IMG=/disk-management-agent: +``` + +### Deploy to a cluster + +```bash +make deploy IMG=/disk-management-agent: +``` + +### Uninstall + +```bash +make undeploy +``` + +## Project Structure + +``` +disk-management-agent/ +├── api/v1alpha1/ # CRD type definitions (DiscoveredPhysicalDisk) +├── cmd/ +│ ├── config/ # Environment configuration loading +│ └── main.go # Application entry point +├── config/ # Kustomize manifests (CRDs, RBAC, manager, webhook) +├── internal/ +│ ├── controller/ # Kubernetes reconciler and discovery ticker +│ └── webhook/v1alpha1/ # Validating admission webhook +├── pkg/ +│ ├── domain/ # Core business entities (DiscoveredPhysicalDrive, DiscoveredLogicalVolume) +│ ├── service/ # Interface definitions (ports) +│ ├── usecase/ # Application business logic +│ └── infrastructure/ # Adapters: RAID discoverers, Kubernetes store, cache, DI container +├── test/e2e/ # End-to-end tests (Kind) +├── Dockerfile +├── Makefile +└── go.mod +``` + +The project follows **clean architecture** principles under `pkg/`: use cases +depend only on domain entities and service interfaces (ports), while +infrastructure adapters implement those interfaces. The Operator SDK scaffold +(`cmd/`, `api/`, `internal/`, `config/`) provides the Kubernetes integration +layer. + +## Development + +```bash +# Generate CRD manifests and RBAC +make manifests + +# Generate DeepCopy methods +make generate + +# Format and vet +make fmt vet + +# Run unit tests (with envtest for controller tests) +make test + +# Run linter (golangci-lint) +make lint + +# Run end-to-end tests (requires Kind) +make test-e2e +``` + +## Contributing + +Contributions are welcome. See [CONTRIBUTING.md](CONTRIBUTING.md) for +development workflow, coding standards, and pull request guidelines. + +## License + +This project is licensed under the Apache License 2.0. See [LICENSE](LICENSE) +for details. From 13d5a9d80ef9e231f20932c92df77be04ad0125d Mon Sep 17 00:00:00 2001 From: Valentin Daviot Date: Thu, 2 Apr 2026 15:12:48 +0200 Subject: [PATCH 2/2] documentation: update documentation format Signed-off-by: Valentin Daviot --- CONTRIBUTING.md | 284 ++++++++++++++++++++++++++++++++++++++++-------- README.md | 224 ++++++++++++++++++-------------------- 2 files changed, 346 insertions(+), 162 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 64c6e76..dc4d6cc 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,11 @@ # Contributing to disk-management-agent -Thank you for your interest in contributing. This document explains the -development workflow, coding standards, and process for submitting changes. +Thank you for considering a contribution. Whether you are fixing a bug, adding +support for a new RAID controller, improving tests, or updating documentation, +your help is appreciated. + +This guide covers everything you need to get started, understand the +architecture, and submit a high-quality pull request. ## Code of Conduct @@ -9,21 +13,9 @@ All participants are expected to treat each other with respect and professionalism. Harassment, discrimination, and disruptive behavior will not be tolerated. Be constructive in code reviews and discussions. -## Development Workflow +## Getting Started -1. Fork the repository and clone your fork. -2. Create a feature branch from `main`: - ```bash - git checkout -b feature/my-change - ``` -3. Make your changes, ensuring they follow the coding standards below. -4. Run the full validation suite before pushing: - ```bash - make manifests generate fmt vet lint test - ``` -5. Push your branch and open a pull request against `main`. - -## Prerequisites +### Prerequisites | Tool | Version | Purpose | |---|---|---| @@ -36,37 +28,221 @@ tolerated. Be constructive in code reviews and discussions. All other tools (`controller-gen`, `kustomize`, `setup-envtest`, `golangci-lint`) are downloaded automatically by the Makefile into `./bin/`. -## Setting Up the Development Environment +### Setup ```bash -# Clone and enter the repository git clone https://github.com/scality/disk-management-agent.git cd disk-management-agent -# Download Go dependencies go mod download -# Generate manifests, deepcopy, format, and vet make manifests generate fmt vet -# Run unit tests to confirm everything works make test ``` -## Coding Standards +If the tests pass you are ready to contribute. + +## Development Workflow + +1. Fork the repository and clone your fork. +2. Create a feature branch from `main`: + +```bash +git checkout -b feature/my-change +``` + +3. Make your changes following the coding standards below. +4. Run the full validation suite before pushing: + +```bash +make manifests generate fmt vet lint test +``` + +5. Push your branch and open a pull request against `main`. + +### Useful Make Targets + +| Target | Description | +|---|---| +| `make build` | Build the manager binary | +| `make run` | Run locally against your current kubeconfig (`NODE_NAME` required) | +| `make manifests` | Regenerate CRD, RBAC, and webhook manifests | +| `make generate` | Regenerate DeepCopy methods | +| `make fmt vet` | Format and vet Go code | +| `make lint` | Run golangci-lint | +| `make lint-fix` | Run golangci-lint and auto-fix what it can | +| `make test` | Unit and controller tests (envtest) | +| `make test-e2e` | End-to-end tests on a Kind cluster | +| `make docker-build` | Build the container image | +| `make install` | Apply CRDs to the cluster in your kubeconfig | +| `make deploy` | Deploy the full stack (CRDs + RBAC + DaemonSet + webhook) | +| `make build-installer` | Generate a single `dist/install.yaml` manifest | + +Run `make help` for the complete list. + +## Architecture + +### Project Structure + +``` +disk-management-agent/ +├── api/v1alpha1/ # CRD type definitions (DiscoveredPhysicalDisk) +├── cmd/ +│ ├── config/ # Environment configuration loading +│ └── main.go # Application entry point +├── config/ # Kustomize manifests (CRDs, RBAC, manager, webhook) +├── internal/ +│ ├── controller/ # Kubernetes reconciler and discovery ticker +│ └── webhook/v1alpha1/ # Validating admission webhook +├── pkg/ +│ ├── domain/ # Core business entities +│ ├── service/ # Interface definitions (ports) +│ ├── usecase/ # Application business logic +│ └── infrastructure/ # Adapters: RAID discoverers, K8s store, cache, DI +├── test/e2e/ # End-to-end tests (Kind) +├── Dockerfile +├── Makefile +└── go.mod +``` + +### Clean Architecture Layers + +The code under `pkg/` follows clean architecture. The dependency direction is +always **inward** -- outer layers depend on inner layers, never the reverse. -### Clean Architecture +```mermaid +flowchart LR + Presentation["internal/controller\ninternal/webhook"] --> UseCase["pkg/usecase"] + UseCase --> Service["pkg/service\n(interfaces)"] + UseCase --> Domain["pkg/domain"] + Infrastructure["pkg/infrastructure"] --> Service + Infrastructure --> Domain +``` + +| Layer | Path | Responsibility | +|---|---|---| +| **Domain** | `pkg/domain/` | Core entities (`DiscoveredPhysicalDrive`, `DiscoveredLogicalVolume`). No external dependencies. | +| **Service** | `pkg/service/` | Interface definitions (ports) that use cases depend on and infrastructure implements. | +| **Use Case** | `pkg/usecase/` | Business logic orchestration. **Must never import infrastructure packages.** | +| **Infrastructure** | `pkg/infrastructure/` | Concrete adapters: RAID discoverers, Kubernetes store, in-memory cache, DI container. | +| **Presentation** | `internal/controller/`, `internal/webhook/` | Kubernetes reconciler, discovery ticker, and validating webhook. | -The project follows clean architecture under `pkg/`: +The `cmd/main.go` entry point wires everything together through the DI container +(`pkg/infrastructure/di/`). + +### Key Interfaces (Ports) + +| Interface | File | Purpose | +|---|---|---| +| `PhysicalDriveDiscoverer` | `pkg/service/physical_drive_discoverer.go` | Discover physical drives from a specific RAID controller type | +| `LogicalVolumeDiscoverer` | `pkg/service/logical_volume_discoverer.go` | Discover logical volumes (used to enrich drive paths) | +| `DiscoveredPhysicalDiskStore` | `pkg/service/discovered_physical_disk_store.go` | Get/Create `DiscoveredPhysicalDisk` CRs in Kubernetes | +| `DiscoveredDriveCacheWriter` | `pkg/service/discovered_drive_cache_writer.go` | Write discovered drives to the in-memory cache | +| `DiscoveredDriveCacheReader` | `pkg/service/discovered_drive_cache_reader.go` | Read a drive from the cache by CR name | -- **`pkg/domain/`** -- Core business entities. No external dependencies. -- **`pkg/service/`** -- Interface definitions (ports). Use cases depend on these. -- **`pkg/usecase/`** -- Application business logic. Must not import - infrastructure packages. -- **`pkg/infrastructure/`** -- Adapters that implement service interfaces. +## Adding a New RAID Controller -Dependency direction is always **inward**: infrastructure depends on use cases -and service interfaces, never the reverse. +Adding support for a new RAID controller type is the most common kind of +contribution. Here is the process step by step. + +Suppose you want to add support for a fictional **Adaptec** controller that uses +a CLI tool called `arcconf`. + +### 1. Implement the discoverers + +Create `pkg/infrastructure/physicaldrivediscoverer/adaptec.go`: + +```go +package physicaldrivediscoverer + +import ( + "github.com/pkg/errors" + "github.com/scality/raidmgmt/pkg/domain/ports" + + "disk-management-agent/pkg/domain" + "disk-management-agent/pkg/service" +) + +const adaptecControllerType = "Adaptec" + +type Adaptec struct { + rc ports.RAIDController +} + +var _ service.PhysicalDriveDiscoverer = &Adaptec{} + +func NewAdaptec(rc ports.RAIDController) *Adaptec { + return &Adaptec{rc: rc} +} + +func (d *Adaptec) DiscoverPhysicalDrives() ([]*domain.DiscoveredPhysicalDrive, error) { + controllers, err := d.rc.Controllers() + if err != nil { + return nil, errors.Wrap(err, "failed to list Adaptec controllers") + } + + var drives []*domain.DiscoveredPhysicalDrive + + for _, ctrl := range controllers { + pds, err := d.rc.PhysicalDrives(ctrl.Metadata) + if err != nil { + return nil, errors.Wrapf(err, "Adaptec controller %d physical drives", ctrl.ID) + } + + for _, pd := range pds { + drives = append(drives, &domain.DiscoveredPhysicalDrive{ + ControllerType: adaptecControllerType, + ControllerID: ctrl.ID, + PhysicalDrive: pd, + }) + } + } + + return drives, nil +} +``` + +Create the matching `pkg/infrastructure/logicalvolumediscoverer/adaptec.go` +following the same pattern. + +### 2. Wire it in the DI container + +Add a new field and getter in `pkg/infrastructure/di/` following the existing +MegaRAID/SmartArray pattern: + +- Add command runner, RAID controller adapter, and discoverer fields to + `container.go`. +- Create getter methods in `physical_drive_discoverer.go`, + `logical_volume_discoverer.go`, `raid_controller.go`, and + `command_runner.go`. +- Register the new discoverers in the slices inside + `GetDiscoverPhysicalDrivesUseCase()` in `usecase.go`. + +### 3. Add configuration + +Add an environment variable (e.g. `ARCCONF_PATH`) in `cmd/config/environment.go` +and pass it through the DI container constructor. + +### 4. Update deployment manifests + +If the new CLI tool requires a host mount, add the volume and volume mount to +`config/manager/manager.yaml`. + +### 5. Write tests + +- Add a unit test in `pkg/infrastructure/physicaldrivediscoverer/` for the new + adapter. +- Extend the use case tests if necessary. + +### 6. Update documentation + +- Add the new controller to the **Features** and **Prerequisites** sections of + `README.md`. +- Add the new environment variable to the **Configuration** table in + `README.md`. + +## Coding Standards ### Interface Naming @@ -80,6 +256,11 @@ type DiscoveredDriveCacheReader interface { ... } Composite interfaces (repositories, services) may use broader names but must embed small, focused interfaces. +### Interface Size + +Interfaces are limited to 1-2 methods. Only composite interfaces may have more +through embedding. + ### Error Handling Wrap all errors with context using `errors.Wrap` or `fmt.Errorf` with `%w`: @@ -91,10 +272,19 @@ return errors.Wrap(err, fmt.Sprintf("disk %s not accessible", diskID)) Include relevant identifiers. Avoid duplicating context already present in the error chain. -### Interface Size +### Acronym Casing -Interfaces are limited to 1-2 methods. Only composite interfaces may have more -through embedding. +Acronyms are either fully uppercase or fully lowercase, never mixed: + +```go +// Correct +type MegaRAID struct { ... } +var httpClient *http.Client + +// Incorrect +type MegaRaid struct { ... } +var HttpClient *http.Client +``` ### Linting @@ -130,12 +320,19 @@ tests, and tears it down automatically. ### Writing Tests -- Controller tests use the - [envtest](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/envtest) - framework and Ginkgo/Gomega. -- Use case tests use [testify](https://github.com/stretchr/testify) with mock - implementations of service interfaces. -- Test files live alongside the code they test (`_test.go` suffix). +| Area | Framework | Location | +|---|---|---| +| Controller reconciliation | [envtest](https://pkg.go.dev/sigs.k8s.io/controller-runtime/pkg/envtest) + Ginkgo/Gomega | `internal/controller/*_test.go` | +| Webhook validation | Standard `testing` + admission context injection | `internal/webhook/v1alpha1/*_test.go` | +| Use cases | [testify](https://github.com/stretchr/testify) with mock service implementations | `pkg/usecase/*_test.go` | +| Infrastructure adapters | testify or Ginkgo | `pkg/infrastructure/**/*_test.go` | +| End-to-end | Ginkgo + kubectl/Kind | `test/e2e/` | + +Test files live alongside the code they test with a `_test.go` suffix. + +When writing controller tests, use the shared `k8sClient` from the envtest +suite and create your CRs in isolated namespaces or with unique names to avoid +interference between tests. ## Pull Request Process @@ -175,11 +372,12 @@ does not cover it. When making code changes, update relevant documentation: -- If you add or modify CRD fields, regenerate manifests with `make manifests`. +- If you add or modify CRD fields, regenerate manifests with `make manifests` + and update the CRD reference table in `README.md`. - If you add environment variables, update the configuration table in `README.md`. -- If you change architecture or add components, update the architecture diagram - in `README.md`. +- If you change the architecture or add components, update the architecture + diagram in `README.md` and the project structure tree in this file. ## License diff --git a/README.md b/README.md index 0f6c0a0..5d48d01 100644 --- a/README.md +++ b/README.md @@ -6,24 +6,81 @@ [![License](https://img.shields.io/badge/License-Apache_2.0-blue.svg)](LICENSE) A Kubernetes node agent that discovers physical HDD drives behind hardware RAID -controllers and represents them as cluster-scoped `DiscoveredPhysicalDisk` -custom resources. +controllers and exposes them as cluster-scoped `DiscoveredPhysicalDisk` custom +resources. Storage orchestration platforms (such as +[MetalK8s](https://github.com/scality/metalk8s)) and operators can consume +these CRs to build a reliable, Kubernetes-native inventory of every physical +disk in the cluster. Built with [Operator SDK](https://sdk.operatorframework.io/) and -[raidmgmt](https://github.com/scality/raidmgmt), the agent supports -**MegaRAID** (via `storcli64` / `perccli64`) and **HPE Smart Array** (via -`ssacli`) controllers. +[raidmgmt](https://github.com/scality/raidmgmt). -## Why +## Features -Storage orchestration platforms such as -[MetalK8s](https://github.com/scality/metalk8s) need a reliable, Kubernetes-native -inventory of the physical disks available on each node. This agent runs as a -DaemonSet, periodically queries the hardware RAID CLIs installed on the host, -and publishes the results as `DiscoveredPhysicalDisk` CRs that higher-level -controllers can consume. +- **MegaRAID support** -- discovers drives via `storcli64` and `perccli64` + (Broadcom / Dell PERC controllers) +- **HPE Smart Array support** -- discovers drives via `ssacli` +- **Periodic discovery** -- scans every 5 minutes; new drives get CRs + automatically, existing CRs have their status refreshed +- **Validating webhook** -- restricts CR creation and updates to the agent's own + service account +- **Cluster-scoped CRD** -- immutable spec (node, controller, slot) with a + status sub-resource reflecting live hardware state +- **Clean architecture** -- business logic is decoupled from Kubernetes and RAID + infrastructure through ports and adapters -## Architecture +## Prerequisites + +| Requirement | Details | +|---|---| +| Kubernetes | v1.33+ | +| cert-manager | Required for webhook TLS (installed by the default Kustomize overlay) | +| RAID CLI tools | At least one of `storcli64`, `perccli64`, or `ssacli` installed on target nodes | +| Container runtime | Docker or compatible (for building images) | + +## Quick Start + +### Install CRDs + +```bash +make install +``` + +### Build and push the container image + +```bash +make docker-build docker-push IMG=/disk-management-agent: +``` + +### Deploy the agent + +```bash +make deploy IMG=/disk-management-agent: +``` + +This deploys a DaemonSet into the `disk-management-agent-system` namespace. +After a few minutes you should see `DiscoveredPhysicalDisk` resources appear: + +```bash +kubectl get discoveredphysicaldisks +``` + +### Generate a single install manifest + +If you prefer applying a single YAML file instead of using `make deploy`: + +```bash +make build-installer IMG=/disk-management-agent: +kubectl apply -f dist/install.yaml +``` + +### Uninstall + +```bash +make undeploy +``` + +## How It Works ```mermaid flowchart TD @@ -57,18 +114,25 @@ flowchart TD Reconciler -->|"Status Update"| CRD ``` +The agent runs as a DaemonSet -- one pod per node. Each instance only manages +disks for its own node. + 1. A **DiscoveryTicker** fires every 5 minutes and runs the **DiscoverPhysicalDrives** use case. 2. The use case invokes every registered `PhysicalDriveDiscoverer` and - `LogicalVolumeDiscoverer` (MegaRAID + Smart Array adapters), filters to HDD - only, enriches device paths from logical volume metadata, replaces the + `LogicalVolumeDiscoverer` adapter (MegaRAID + Smart Array), keeps only HDD + drives, enriches device paths from logical volume metadata, replaces the in-memory cache, and creates any missing `DiscoveredPhysicalDisk` CRs. 3. For CRs that already exist the ticker sends a `GenericEvent` to the **Reconciler**, which reads the latest snapshot from the cache and updates the CR `.status` (vendor, model, serial, size, paths, etc.). -4. A **validating webhook** restricts CR creation/update to the agent's own +4. A **validating webhook** restricts CR creation and updates to the agent's own service account. +If a RAID CLI tool is not installed on a node (e.g. `storcli64` on a +SmartArray-only host), the corresponding discoverer logs the error and is +skipped -- it does not block other discoverers. + ## CRD Reference **Group:** `metalk8s.scality.com` @@ -138,121 +202,43 @@ status: ## Configuration -The agent is configured through environment variables: +The agent is configured through environment variables. When deployed via the +default Kustomize manifests, `NODE_NAME`, `POD_NAMESPACE`, and +`POD_SERVICE_ACCOUNT` are injected automatically. | Variable | Required | Default | Description | |---|---|---|---| -| `NODE_NAME` | Yes | -- | Kubernetes node name (typically injected via the downward API) | -| `POD_NAMESPACE` | No | -- | Pod namespace, used to build the webhook allowed service account | -| `POD_SERVICE_ACCOUNT` | No | -- | Pod service account name, used for webhook authorization | -| `STORCLI_PATH` | No | `/host/libexec/MegaRAID/storcli/storcli64` | Path to the storcli binary | -| `PERCCLI_PATH` | No | `/host/libexec/MegaRAID/perccli/perccli64` | Path to the perccli binary | -| `SSACLI_PATH` | No | `/host/libexec/ssacli` | Path to the ssacli binary | - -The application version is injected at build time via ldflags (defaults to -`dev`). - -## Getting Started - -### Prerequisites - -- Go 1.25+ -- A Kubernetes cluster (v1.33+) -- `make` -- Docker or compatible container runtime -- At least one supported RAID CLI tool installed on the target nodes +| `NODE_NAME` | Yes | -- | Kubernetes node name (injected via the downward API) | +| `POD_NAMESPACE` | No | -- | Pod namespace; used to build the webhook allowed service account | +| `POD_SERVICE_ACCOUNT` | No | -- | Pod service account name; used for webhook authorization | +| `STORCLI_PATH` | No | `/host/libexec/MegaRAID/storcli/storcli64` | Path to the `storcli64` binary | +| `PERCCLI_PATH` | No | `/host/libexec/MegaRAID/perccli/perccli64` | Path to the `perccli64` binary | +| `SSACLI_PATH` | No | `/host/libexec/ssacli` | Path to the `ssacli` binary | -### Build - -```bash -make build -``` - -### Install CRDs - -```bash -make install -``` - -### Run locally (out-of-cluster, uses current kubeconfig) - -```bash -NODE_NAME=my-node make run -``` - -### Build and push the container image - -```bash -make docker-build docker-push IMG=/disk-management-agent: -``` - -### Deploy to a cluster - -```bash -make deploy IMG=/disk-management-agent: -``` - -### Uninstall - -```bash -make undeploy -``` - -## Project Structure - -``` -disk-management-agent/ -├── api/v1alpha1/ # CRD type definitions (DiscoveredPhysicalDisk) -├── cmd/ -│ ├── config/ # Environment configuration loading -│ └── main.go # Application entry point -├── config/ # Kustomize manifests (CRDs, RBAC, manager, webhook) -├── internal/ -│ ├── controller/ # Kubernetes reconciler and discovery ticker -│ └── webhook/v1alpha1/ # Validating admission webhook -├── pkg/ -│ ├── domain/ # Core business entities (DiscoveredPhysicalDrive, DiscoveredLogicalVolume) -│ ├── service/ # Interface definitions (ports) -│ ├── usecase/ # Application business logic -│ └── infrastructure/ # Adapters: RAID discoverers, Kubernetes store, cache, DI container -├── test/e2e/ # End-to-end tests (Kind) -├── Dockerfile -├── Makefile -└── go.mod -``` - -The project follows **clean architecture** principles under `pkg/`: use cases -depend only on domain entities and service interfaces (ports), while -infrastructure adapters implement those interfaces. The Operator SDK scaffold -(`cmd/`, `api/`, `internal/`, `config/`) provides the Kubernetes integration -layer. +The application version is injected at build time via `-ldflags` and defaults to +`dev`. ## Development ```bash -# Generate CRD manifests and RBAC -make manifests - -# Generate DeepCopy methods -make generate - -# Format and vet -make fmt vet - -# Run unit tests (with envtest for controller tests) -make test - -# Run linter (golangci-lint) -make lint - -# Run end-to-end tests (requires Kind) -make test-e2e +make build # Build the manager binary +NODE_NAME=my-node make run # Run locally against your current kubeconfig +make manifests # Regenerate CRD, RBAC, and webhook manifests +make generate # Regenerate DeepCopy methods +make fmt vet # Format and vet +make lint # Run golangci-lint +make test # Unit + controller tests (envtest) +make test-e2e # End-to-end tests (Kind) ``` +See [CONTRIBUTING.md](CONTRIBUTING.md) for the full development guide, coding +standards, architecture overview, and pull request process. + ## Contributing -Contributions are welcome. See [CONTRIBUTING.md](CONTRIBUTING.md) for -development workflow, coding standards, and pull request guidelines. +Contributions are welcome -- whether it is a bug fix, a new RAID controller +adapter, documentation improvements, or anything else. See +[CONTRIBUTING.md](CONTRIBUTING.md) to get started. ## License