diff --git a/.agents/reference/code-structure.md b/.agents/reference/code-structure.md new file mode 100644 index 0000000000..5928e4db2b --- /dev/null +++ b/.agents/reference/code-structure.md @@ -0,0 +1,56 @@ +# Code Structure + +## Architecture + +The operator consists of four main binaries: + +1. **capi-operator** (`cmd/capi-operator/`) - Manages installation of Cluster API components and providers. Runs the ClusterOperator, Revision, and Installer controllers. +2. **capi-controllers** (`cmd/capi-controllers/`) - Manages CAPI cluster resources, infrastructure integration, secret synchronization, and kubeconfig management. Runs the CoreCluster, InfraCluster, SecretSync, and Kubeconfig controllers plus webhooks. +3. **machine-api-migration** (`cmd/machine-api-migration/`) - Handles migration between Machine API and Cluster API resources. Only runs when the MachineAPIMigration feature gate is enabled. Currently supports AWS and OpenStack platforms. +4. **crd-compatibility-checker** (`cmd/crd-compatibility-checker/`) - Validates CRD compatibility requirements, runs object validation/pruning webhooks, and installs static resources for the compatibility requirements system. + +The repository also includes: + +5. **manifests-gen** (`manifests-gen/`) - Standalone tool that generates admission policy profiles from upstream Cluster API provider manifests for embedding into the operator image. + +## Key Controllers + +### capi-operator Controllers +- **ClusterOperator Controller** (`pkg/controllers/clusteroperator/`) - Manages the operator's ClusterOperator status resource. Always runs, even on unsupported platforms. +- **Revision Controller** (`pkg/controllers/revision/`) - Manages OLM revision resources for tracking installed provider versions and triggering upgrades. +- **Installer Controller** (`pkg/controllers/installer/`) - Handles installation and lifecycle management of CAPI components and providers using the boxcutter framework. + +### capi-controllers Controllers +- **Core Cluster Controller** (`pkg/controllers/corecluster/`) - Manages CAPI Cluster resources representing the OpenShift cluster +- **Infra Cluster Controller** (`pkg/controllers/infracluster/`) - Manages infrastructure-specific cluster resources (AWS, Azure, GCP, etc.) +- **Secret Sync Controller** (`pkg/controllers/secretsync/`) - Synchronizes secrets between MAPI and CAPI namespaces +- **Kubeconfig Controller** (`pkg/controllers/kubeconfig/`) - Manages kubeconfig secrets for cluster access + +### crd-compatibility-checker Controllers +- **CRD Compatibility Controller** (`pkg/controllers/crdcompatibility/`) - Reconciles CompatibilityRequirement resources and validates CRD create/update/delete operations via webhooks. Includes object validation and pruning sub-controllers. +- **Static Resource Installer Controller** (`pkg/controllers/staticresourceinstaller/`) - Installs static Kubernetes resources from embedded asset files on startup. + +### machine-api-migration Controllers +- **Machine Migration Controller** (`pkg/controllers/machinemigration/`) - Handles handover of AuthoritativeAPI and object pausing for machine migration +- **MachineSet Migration Controller** (`pkg/controllers/machinesetmigration/`) - Handles handover of AuthoritativeAPI and object pausing for machineset migration +- **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine related resources between APIs +- **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset related objects between APIs + +### Shared Packages +- **Sync Common** (`pkg/controllers/synccommon/`) - Shared apply-configuration helpers and migration status logic used by the machine and machineset sync/migration controllers + +## Conversion Framework +- **MAPI to CAPI Conversion** (`pkg/conversion/mapi2capi/`) - Converts MAPI resources to CAPI (supports AWS, OpenStack) +- **CAPI to MAPI Conversion** (`pkg/conversion/capi2mapi/`) - Converts CAPI resources to MAPI (supports AWS, OpenStack) +- **Conversion Utilities** (`pkg/conversion/util/`, `pkg/conversion/consts/`) - Shared constants and helper functions +- **Conversion Test Utilities** (`pkg/conversion/test/`) - Fuzz testing and test matchers for conversion logic + +## File Structure +- `manifests/` - OpenShift manifests for operator deployment +- `capi-operator-manifests/` - Upstream CAPI provider manifests consumed by manifests-gen +- `admission-policies/` - Kustomize overlays for admission policy profiles (default, AWS) +- `manifests-gen/` - Tool for generating admission policy profiles +- `hack/` - Development and testing scripts +- `docs/controllers/` - Detailed controller documentation +- `e2e/` - End-to-end tests for each supported platform +- `vendor/` - Vendored dependencies (use `make vendor` to update) diff --git a/.agents/reference/style-guide.md b/.agents/reference/style-guide.md new file mode 100644 index 0000000000..dbb594844c --- /dev/null +++ b/.agents/reference/style-guide.md @@ -0,0 +1,9 @@ +# Style Guide + +## Coding Style +- Use early returns +- Descriptive names +- Helper functions over inline code +- Minimal comments (only for non-obvious decisions) +- Simple code over complex language features +- For user-facing text like logs and errors, use "Cluster API" and "Machine API". For code and internal identifiers, use "CAPI" and "MAPI". diff --git a/.agents/reference/tasks.md b/.agents/reference/tasks.md new file mode 100644 index 0000000000..64202d7259 --- /dev/null +++ b/.agents/reference/tasks.md @@ -0,0 +1,57 @@ +# Tasks + +## Essential Commands +```bash +# Build and test +make build # Build all binaries +make test # Run verification (fmt + lint) then unit tests +make unit # Run unit tests with coverage +make verify # Run fmt, lint, and verify-ocp-manifests +make lint # Run linting (golangci-lint) +make fmt # Format code (golangci-lint --fix) +make vendor # Vendor dependencies +make ocp-manifests # Generate admission policy profiles +``` + +## Running Tests + +**Do not use `go test` or `ginkgo` directly.** Tests use `envtest` which requires `KUBEBUILDER_ASSETS` +to point at downloaded API server and etcd binaries. The Makefile handles this: `make unit` depends on +the `.localtestenv` target (which runs `setup-envtest` to download binaries and writes their path to +`.localtestenv`), and `hack/test.sh` sources that file before invoking ginkgo. Running `go test` +directly will fail because the envtest `Environment` cannot locate the binaries. + +```bash +make unit # All unit tests +make unit TEST_DIRS="./pkg/controllers/installer/..." # Specific package +make unit TEST_DIRS="./pkg/controllers/machinesync/..." # Another specific package +``` + +**Important:** Ginkgo functional tests are slow and produce verbose output that will exceed +context limits. Always redirect output to a log file and use multi-pass processing: +```bash +make unit TEST_DIRS="./pkg/..." 2>&1 | tee /tmp/test-output.log +# Then check results: +tail -20 /tmp/test-output.log # Summary +grep -E 'FAIL|PASSED' /tmp/test-output.log # Pass/fail status +grep 'FAIL' /tmp/test-output.log # Find failures +``` + +### Default ginkgo arguments +The default ginkgo args in `hack/test.sh` are: +- `-r -v -p --randomize-all --randomize-suites --keep-going --race --trace --timeout=${TIMEOUT}` +- The timeout defaults to `20m` for unit tests (set by the Makefile) and `120m` for e2e tests. +- In CI (`OPENSHIFT_CI=true`), `-p` is replaced with `--procs=4`. + +Prefer using `GINKGO_EXTRA_ARGS` to pass additional arguments to ginkgo. Use `GINKGO_ARGS` when you need to override the default values entirely. + +### Focused Testing +```go +// Focus specific tests (REMOVE before committing!) +FIt("test name", func() { /* test */ }) +FContext("context name", func() { /* tests */ }) +``` + +### Test Environment +- Each controller has a `suite_test.go` that bootstraps an `envtest.Environment` +- See "Running Tests" above for why `make unit` is required diff --git a/.agents/reference/testing.md b/.agents/reference/testing.md new file mode 100644 index 0000000000..a82653fe73 --- /dev/null +++ b/.agents/reference/testing.md @@ -0,0 +1,159 @@ +# Testing + +## Choosing the right test level + +Pick the cheapest level that can adequately cover the behaviour. Do not escalate without reason. + +**Unit test** (no client or fake client, no envtest) — for pure logic, conversions, single-function behaviour, error paths that don't depend on server-side behaviour. Use `fake.NewClientBuilder()` only when the test doesn't depend on realistic API server responses (field defaulting, status subresource semantics, conflict errors, SSA merge, etc.). These run in milliseconds. + +**Integration test** (envtest) — for anything that interacts with a Kubernetes API: controller reconciliation loops, multi-resource interactions, watching, status updates, and any scenario where fake client behaviour diverges from a real API server. Prefer envtest over fakes when in doubt — faking accurately is hard and flaky fakes waste more time than the slower test. Use `pkg/test.StartEnvTest()` in `suite_test.go`. These run in seconds. + +**E2E test** (`e2e/`) — only for behaviour that requires real infrastructure: actual machine provisioning, cloud API interactions, cross-component migration flows. These run in minutes. + +Rules of thumb: +- If you're testing "does this function return the right value/error" and it doesn't need a client → unit test. +- If you're testing any controller or client interaction → integration test (envtest). +- If you're testing "does a real machine appear in the cloud" → e2e test. +- If envtest can reproduce the scenario, do not write an e2e test. + +## Use existing shared helpers + +Before writing new test utilities, builders, matchers, or setup code, search the repo for existing ones — particularly in `pkg/test/`, `pkg/conversion/test/`, `pkg/admissionpolicy/testutils/`, `e2e/framework/`, and the vendored `testutils/resourcebuilder/` package. Do not duplicate what already exists. If you need a variant, extend the existing helper rather than creating a parallel one. + +## Ginkgo/Gomega Best Practices + +Use **Ginkgo/Gomega** framework and prefer built-in features over custom implementations: +- Use `DescribeTable` with `Entry` for table-driven tests instead of manual loops +- Use `HaveField`, `HaveValue`, `HaveKey` for struct/map assertions instead of manual field checks +- Use `ConsistOf` for unordered slice matching instead of sorting + `Equal` +- Use `MatchError` for error checking instead of string contains +- Use `BeNumerically` for numeric comparisons instead of manual range checks + +## Test Organization + +- **Nested Contexts**: Organize related test scenarios with nested `Context()` blocks + ```go + Context("when migrating from MachineAPI to ClusterAPI", func() { + Context("when status is not paused", func() { + // Test cases + }) + }) + ``` +- **Descriptive test names**: Describe expected behaviour, not implementation details. Use "should..." format: + ```go + // good — describes behaviour + It("should reject machines with duplicate provider IDs", func() { ... }) + + // bad — describes implementation + It("should return an error from validateProviderID", func() { ... }) + ``` +- **Use `By()` for test steps**: Document distinct phases within a test with `By("Setting up namespaces for the test")` + +## Async Assertions with Komega + +Use **Komega** for Kubernetes object assertions: +```go +// Use komega.Object for async assertions +Eventually(k.Object(myResource)).Should(HaveField("ObjectMeta.ResourceVersion", Equal(expectedRV))) + +// Update resources with komega helpers +Eventually(k.UpdateStatus(myResource, func() { + myResource.Status.SomeField = "value" +})).Should(Succeed()) +``` + +## Resource Management + +- **Resource builders**: Use `cluster-api-actuator-pkg/testutils/resourcebuilder` for creating test objects. + Builders are organized by API group (e.g., `machine/v1beta1`, `cluster-api/core/v1beta2`, `cluster-api/infrastructure/v1beta2`, `config/v1`, `core/v1`). + ```go + mapiMachine = mapiMachineBuilder. + WithNamespace(namespace). + WithName("foo"). + WithAuthoritativeAPI(machinev1beta1.MachineAuthorityMachineAPI). + Build() + ``` +- **Standard cleanup**: Use `testutils.CleanupResources()` in AfterEach (from `cluster-api-actuator-pkg/testutils`) + ```go + testutils.CleanupResources(Default, ctx, cfg, k8sClient, namespace, + &machinev1beta1.Machine{}, + &clusterv1.Machine{}, + ) + ``` + +## Assertions + +Prefer precise matchers over multiple loose ones. Combine related assertions into a single matcher (e.g., `SatisfyAll`, `ConsistOf`). With `Eventually`, each separate assertion polls with its own timeout — multiple assertions checking the same object multiply the wait time on failure. + +```go +// good — single assertion, exact match +Expect(err).To(MatchError(expectedErr)) + +// bad — two assertions, string matching +Expect(err).To(HaveOccurred()) +Expect(err).To(MatchError(ContainSubstring("connection refused"))) +``` + +When an expected error is reused across multiple test cases, declare it as a variable rather than duplicating the literal. + +- **Complex assertions**: Combine matchers with `SatisfyAll` + ```go + Eventually(komega.Object(resource)).Should(SatisfyAll( + HaveField("Status.AuthoritativeAPI", Equal(expected)), + HaveField("Status.SynchronizedGeneration", BeZero()), + )) + ``` +- **Checking absence**: Use `ShouldNot` with appropriate matchers + ```go + Eventually(komega.Object(resource)).ShouldNot( + HaveField("ObjectMeta.Annotations", ContainElement(HaveKeyWithValue(key, value)))) + ``` +- **Nested field checks**: Chain `HaveField` for nested assertions + ```go + HaveField("Status.Conditions", ContainElement(SatisfyAll( + HaveField("Type", Equal("Paused")), + HaveField("Status", Equal(corev1.ConditionTrue)), + ))) + ``` + +## Debuggable Failures + +Every test failure must be debuggable from the output alone — without reading test source code. + +**Assertion messages.** If a failure's stack trace and default matcher output wouldn't tell you what went wrong, add a description. This applies especially to generic matchers like `BeNil()`, `BeTrue()`, `HaveLen()` where the default output doesn't convey intent. + +```go +// good — failure output explains the scenario +Expect(transport).To(BeNil(), "expected nil transport when additionalTrustedCA is not set") + +// bad — failure output is just "expected nil, got &http.Transport{...}" +Expect(transport).To(BeNil()) +``` + +**Stack traces.** Do not call `Expect`, `Fail`, or panic from helper functions — failures will point at the helper, not the test that called it. Return errors to the calling test instead. + +If assertions inside a helper are unavoidable, use `GinkgoHelper()` so the stack trace shows the caller: + +```go +func expectResourceReady(obj client.Object) { + GinkgoHelper() + Expect(obj.GetAnnotations()).To(HaveKey("ready")) +} +``` + +## No Sleeps, No Timeout Bumps + +In event-driven systems, tests should wait for conditions, not for time to pass. + +- **Never use `time.Sleep()`**. Use `Eventually` with a condition that checks the actual state you're waiting for. +- **Do not bump `Eventually` timeouts to fix flaky tests.** A flaky test means the test is waiting for the wrong condition or the code has a race. Fix the root cause. +- **`Consistently` durations should be meaningful.** Too-short durations prove nothing — the condition might change immediately after. Use a duration long enough to cover at least a few reconciliation cycles. + +```go +// good — waits for the actual state change +Eventually(komega.Object(machine)).Should(HaveField("Status.Phase", Equal("Running"))) + +// bad — arbitrary sleep hoping the controller has finished +time.Sleep(5 * time.Second) +Expect(machine.Status.Phase).To(Equal("Running")) +``` diff --git a/.claude/skills/deep-review/SKILL.md b/.claude/skills/deep-review/SKILL.md index 6d46c612cf..76e479526e 100644 --- a/.claude/skills/deep-review/SKILL.md +++ b/.claude/skills/deep-review/SKILL.md @@ -41,7 +41,7 @@ Launch the following agents **in parallel** using the Agent tool, with `run_in_b - Correctness and bugs - Error handling - Architecture and edge cases - - Adherence to project conventions (see CLAUDE.md) + - Adherence to project conventions (see AGENTS.md and .agents/reference/style-guide.md) - Naming and clarity 2. **gemini agent** (`subagent_type: gemini`) — Gemini via CLI. Independent second opinion from a different model: @@ -59,7 +59,7 @@ Launch the following agents **in parallel** using the Agent tool, with `run_in_b **If the changes include test files** (`_test.go`, `suite_test.go`, or test helper files), ask the user whether to also launch a dedicated test quality reviewer. If yes, launch an additional agent: 4. **code-reviewer agent** (`subagent_type: code-reviewer`) — test quality focus: - - Read and apply `.claude/skills/test-standards/SKILL.md` as the review checklist + - Read and apply `.agents/reference/testing.md` as the review checklist - Test level appropriateness (unit vs integration vs e2e) - Debuggable failures (assertion messages, GinkgoHelper, stack traces) - Flakiness risks (sleeps, timeouts, shared state, ordering dependencies) diff --git a/.claude/skills/test-standards/SKILL.md b/.claude/skills/test-standards/SKILL.md deleted file mode 100644 index d599759f09..0000000000 --- a/.claude/skills/test-standards/SKILL.md +++ /dev/null @@ -1,121 +0,0 @@ ---- -name: test-standards -description: Apply when writing, modifying, or reviewing test files (_test.go). Covers test level selection, assertion quality, BDD naming, and use of shared helpers. ---- - -When writing tests, follow these standards. When reviewing tests, flag violations and suggest fixes with code examples. - -## Choosing the right test level - -Pick the cheapest level that can adequately cover the behaviour. Do not escalate without reason. - -**Unit test** (no client or fake client, no envtest) — for pure logic, conversions, single-function behaviour, error paths that don't depend on server-side behaviour. Use `fake.NewClientBuilder()` only when the test doesn't depend on realistic API server responses (field defaulting, status subresource semantics, conflict errors, SSA merge, etc.). These run in milliseconds. - -**Integration test** (envtest) — for anything that interacts with a Kubernetes API: controller reconciliation loops, multi-resource interactions, watching, status updates, and any scenario where fake client behaviour diverges from a real API server. Prefer envtest over fakes when in doubt — faking accurately is hard and flaky fakes waste more time than the slower test. Use `pkg/test.StartEnvTest()` in `suite_test.go`. These run in seconds. - -**E2E test** (`e2e/`) — only for behaviour that requires real infrastructure: actual machine provisioning, cloud API interactions, cross-component migration flows. These run in minutes. - -Rules of thumb: -- If you're testing "does this function return the right value/error" and it doesn't need a client → unit test. -- If you're testing any controller or client interaction → integration test (envtest). -- If you're testing "does a real machine appear in the cloud" → e2e test. -- If envtest can reproduce the scenario, do not write an e2e test. - -## Use existing shared helpers - -Before writing new test utilities, builders, matchers, or setup code, search the repo for existing ones — particularly in `pkg/test/`, `pkg/conversion/test/`, `pkg/admissionpolicy/testutils/`, `e2e/framework/`, and the vendored `testutils/resourcebuilder/` package. Do not duplicate what already exists. If you need a variant, extend the existing helper rather than creating a parallel one. - -## Debuggable failures - -Every test failure must be debuggable from the output alone — without reading test source code. This means two things: - -**1. Assertion messages.** If a failure's stack trace and default matcher output wouldn't tell you what went wrong, add a description. This applies especially to generic matchers like `BeNil()`, `BeTrue()`, `HaveLen()` where the default output doesn't convey intent. - -```go -// good — failure output explains the scenario -Expect(transport).To(BeNil(), "expected nil transport when additionalTrustedCA is not set") - -// bad — failure output is just "expected nil, got &http.Transport{...}" -Expect(transport).To(BeNil()) -``` - -**2. Stack traces.** Do not call `Expect`, `Fail`, or panic from helper functions — failures will point at the helper, not the test that called it. Return errors to the calling test instead. - -If assertions inside a helper are unavoidable, use `GinkgoHelper()` so the stack trace shows the caller: - -```go -func expectResourceReady(obj client.Object) { - GinkgoHelper() - Expect(obj.GetAnnotations()).To(HaveKey("ready")) -} -``` - -## BDD test names - -Describe **expected behaviour**, not implementation details. Names should read as specifications. - -```go -// good — describes behaviour -It("should reject machines with duplicate provider IDs", func() { ... }) - -// bad — describes implementation -It("should return an error from validateProviderID", func() { ... }) -``` - -Use nested `Context` blocks with `when` to set up preconditions: - -```go -Context("when the infrastructure cluster is not ready", func() { - It("should requeue after 30 seconds", func() { ... }) -}) -``` - -Use `By()` to document distinct phases within a test. - -## Concise assertions - -Prefer precise matchers over multiple loose ones. Combine into one `Expect` when possible. - -```go -// good — single assertion, exact match -Expect(err).To(MatchError(expectedErr)) - -// bad — two assertions, string matching -Expect(err).To(HaveOccurred()) -Expect(err).To(MatchError(ContainSubstring("connection refused"))) -``` - -When an expected error is reused across multiple test cases, declare it as a variable rather than duplicating the literal: - -```go -// good — declared once, reused across cases -expectedErr := fmt.Errorf("connection refused") -Expect(err).To(MatchError(expectedErr)) - -// bad — same string repeated in every test case -Expect(err).To(MatchError("connection refused")) -``` - -Combine related assertions into a single matcher (e.g., `SatisfyAll`, `ConsistOf`). With `Eventually`, each separate assertion polls with its own timeout — multiple assertions checking the same object multiply the wait time on failure. - -## No sleeps, no timeout bumps - -In event-driven systems, tests should wait for conditions, not for time to pass. - -- **Never use `time.Sleep()`**. Use `Eventually` with a condition that checks the actual state you're waiting for. -- **Do not bump `Eventually` timeouts to fix flaky tests.** A flaky test means the test is waiting for the wrong condition or the code has a race. Fix the root cause. -- **`Consistently` durations should be meaningful.** Too-short durations prove nothing — the condition might change immediately after. Use a duration long enough to cover at least a few reconciliation cycles. - -```go -// good — waits for the actual state change -Eventually(komega.Object(machine)).Should(HaveField("Status.Phase", Equal("Running"))) - -// bad — arbitrary sleep hoping the controller has finished -time.Sleep(5 * time.Second) -Expect(machine.Status.Phase).To(Equal("Running")) -``` - -## General - -- Follow patterns from CLAUDE.md (Komega, DescribeTable, resource builders, testutils cleanup). -- Remove `FIt`/`FContext` before committing. diff --git a/AGENTS.md b/AGENTS.md index 55b9607fa7..2396147581 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,183 +2,21 @@ Instructions for AI Agents when working with the cluster-capi-operator project. -## Quick Reference - -### Essential Commands -```bash -# Build and test -make build # Build all binaries -make test # Run all tests -make unit # Run unit tests with coverage -make lint # Run linting -make fmt # Format code - ## Project Overview The Cluster CAPI Operator manages the installation and lifecycle of Cluster API components on OpenShift clusters. It serves as a bridge between OpenShift's Machine API (MAPI) and the upstream Cluster API (CAPI), providing forward compatibility and migration capabilities. -### Architecture - -The operator consists of three main binaries: - -1. **capi-operator** (`cmd/capi-operator/`) - Manages installation of Cluster API components and providers -2. **capi-controllers** (`cmd/capi-controllers/`) - Main controllers managing CAPI component lifecycle, cluster resources, and synchronization -3. **machine-api-migration** (`cmd/machine-api-migration/`) - Handles migration between Machine API and Cluster API resources - -The repository also includes: - -4. **manifests-gen** (`manifests-gen/`) - Standalone tool that transforms upstream Cluster API provider manifests into OpenShift-compatible format, applying OpenShift-specific annotations, replacing cert-manager with service-ca, and generating provider ConfigMaps with compressed components - -### Key Controllers - -#### capi-operator Controllers -- **CAPI Installer Controller** (`pkg/controllers/capiinstaller/`) - Handles installation of CAPI components and providers - -#### capi-controllers Controllers -- **ClusterOperator Controller** (`pkg/controllers/clusteroperator/`) - Manages the operator's status in the cluster -- **Core Cluster Controller** (`pkg/controllers/corecluster/`) - Manages CAPI Cluster resources representing the OpenShift cluster -- **Infra Cluster Controller** (`pkg/controllers/infracluster/`) - Manages infrastructure-specific cluster resources (AWS, Azure, GCP, etc.) -- **Secret Sync Controller** (`pkg/controllers/secretsync/`) - Synchronizes secrets between MAPI and CAPI namespaces -- **Kubeconfig Controller** (`pkg/controllers/kubeconfig/`) - Manages kubeconfig secrets for cluster access - -#### machine-api-migration Controllers -- **Machine Migration Controller** (`pkg/controllers/machinemigration/`) - Handles handover of AuthoritativeAPI and object pausing for machine migration -- **MachineSet Migration Controller** (`pkg/controllers/machinesetmigration/`) - Handles handover of AuthoritativeAPI and object pausing for machineset migration -- **Machine Sync Controller** (`pkg/controllers/machinesync/`) - Synchronizes individual machine related resources between APIs -- **MachineSet Sync Controller** (`pkg/controllers/machinesetsync/`) - Synchronizes machineset related objects resources between APIs - -#### Conversion Framework -- **MAPI to CAPI Conversion** (`pkg/conversion/mapi2capi/`) - Library implementing Conversion of MAPI resources to CAPI -- **CAPI to MAPI Conversion** (`pkg/conversion/capi2mapi/`) - Library implementing conversion of CAPI resources to MAPI - -### File Structure -- `manifests/` - Contains OpenShift manifests for operator deployment -- `manifests-gen/` - Tool for generating customized provider manifests -- `hack/` - Development and testing scripts -- `docs/controllers/` - Detailed controller documentation -- `e2e/` - End-to-end tests for each supported platform - ## Development Rules -**- ALWAYS check for existing patterns, and use them if found** - -### Coding Style -- Use early returns -- Descriptive names -- Helper functions over inline code -- Minimal comments (only for non-obvious decisions) -- Simple code over complex language features -- For user-facing text like logs and errors, use "Cluster API" and "Machine API". For code and internal identifiers, use "CAPI" and "MAPI". - -## Testing - -### Running Tests - -**Do not use `go test` or `ginkgo` directly.** Tests use `envtest` which requires `KUBEBUILDER_ASSETS` -to point at downloaded API server and etcd binaries. The Makefile handles this: `make unit` depends on -the `.localtestenv` target (which runs `setup-envtest` to download binaries and writes their path to -`.localtestenv`), and `hack/test.sh` sources that file before invoking ginkgo. Running `go test` -directly will fail because the envtest `Environment` cannot locate the binaries. - -```bash -make unit # All unit tests -make unit TEST_DIRS="./pkg/controllers/installer/..." # Specific package -make unit TEST_DIRS="./pkg/controllers/machinesync/..." # Another specific package -``` - -**Important:** Ginkgo functional tests are slow and produce verbose output that will exceed -context limits. Always redirect output to a log file and use multi-pass processing: -```bash -make unit TEST_DIRS="./pkg/..." 2>&1 | tee /tmp/test-output.log -# Then check results: -tail -20 /tmp/test-output.log # Summary -grep -E 'FAIL|PASSED' /tmp/test-output.log # Pass/fail status -grep 'FAIL' /tmp/test-output.log # Find failures -``` -#### Default ginkgo arguments -- `GINKGO_ARGS="-r -v --randomize-all --randomize-suites --keep-going --race --trace --timeout=10m"` -Prefer using `GINKGO_EXTRA_ARGS` to pass additional arguments to ginkgo. Use `GINKGO_ARGS` when you need to override the default values entirely. - -### Test Patterns - -#### Ginkgo/Gomega Best Practices -Use **Ginkgo/Gomega** framework and prefer built-in features over custom implementations: -- Use `DescribeTable` with `Entry` for table-driven tests instead of manual loops -- Use `HaveField`, `HaveValue`, `HaveKey` for struct/map assertions instead of manual field checks -- Use `ConsistOf` for unordered slice matching instead of sorting + `Equal` -- Use `MatchError` for error checking instead of string contains -- Use `BeNumerically` for numeric comparisons instead of manual range checks - -#### Async Assertions with Komega -Use **Komega** for Kubernetes object assertions: -```go -// Use komega.Object for async assertions -Eventually(k.Object(myResource)).Should(HaveField("ObjectMeta.ResourceVersion", Equal(expectedRV))) - -// Update resources with komega helpers -Eventually(k.UpdateStatus(myResource, func() { - myResource.Status.SomeField = "value" -})).Should(Succeed()) -``` - -#### Test Organization -- **Nested Contexts**: Organize related test scenarios with nested `Context()` blocks - ```go - Context("when migrating from MachineAPI to ClusterAPI", func() { - Context("when status is not paused", func() { - // Test cases - }) - }) - ``` -- **Descriptive test names**: Use "should..." format: `It("should do nothing", func() {...})` -- **Use `By()` for test steps**: Document test phases with `By("Setting up namespaces for the test")` - -#### Resource Management -- **Resource builders**: Use testutils resource builders for creating test objects - ```go - mapiMachine = mapiMachineBuilder. - WithNamespace(namespace). - WithName("foo"). - WithAuthoritativeAPI(machinev1beta1.MachineAuthorityMachineAPI). - Build() - ``` -- **Standard cleanup**: Use `testutils.CleanupResources()` in AfterEach - ```go - testutils.CleanupResources(Default, ctx, cfg, k8sClient, namespace, - &machinev1beta1.Machine{}, - &clusterv1.Machine{}, - ) - ``` - -#### Assertions -- **Complex assertions**: Combine matchers with `SatisfyAll` - ```go - Eventually(komega.Object(resource)).Should(SatisfyAll( - HaveField("Status.AuthoritativeAPI", Equal(expected)), - HaveField("Status.SynchronizedGeneration", BeZero()), - )) - ``` -- **Checking absence**: Use `ShouldNot` with appropriate matchers - ```go - Eventually(komega.Object(resource)).ShouldNot( - HaveField("ObjectMeta.Annotations", ContainElement(HaveKeyWithValue(key, value)))) - ``` -- **Nested field checks**: Chain `HaveField` for nested assertions - ```go - HaveField("Status.Conditions", ContainElement(SatisfyAll( - HaveField("Type", Equal("Paused")), - HaveField("Status", Equal(corev1.ConditionTrue)), - ))) - ``` +- **ALWAYS check for existing patterns, and use them if found** +- Before writing new test utilities, builders, matchers, or setup code, search the repo for existing ones +- Never commit focused tests (`FIt`, `FContext`, `FDescribe`) -### Focused Testing -```go -// Focus specific tests (REMOVE before committing!) -FIt("test name", func() { /* test */ }) -FContext("context name", func() { /* tests */ }) -``` +## Reference -### Test Environment -- Each controller has a `suite_test.go` that bootstraps an `envtest.Environment` -- See "Running Tests" above for why `make unit` is required +Detailed guidance is split into reference files. Consult these when working in the relevant area: +- [Code Structure](.agents/reference/code-structure.md) — Architecture, binaries, controllers, conversion framework, file structure +- [Style Guide](.agents/reference/style-guide.md) — Coding conventions and naming rules +- [Tasks](.agents/reference/tasks.md) — Make targets, running tests, ginkgo arguments +- [Testing](.agents/reference/testing.md) — Test patterns, assertion conventions, resource builders, test-level selection