CONTRIBUTING.md

Rama master · réplica de lectura del repositorio original.

# Contributing to pillar-csi

Bug reports, fixes, and documentation changes are welcome. For a larger change, open an issue first so we can agree on the approach before you write the code.

Report security problems privately as described in [SECURITY.md](SECURITY.md), not in a public issue.

## Development setup

You need Go 1.27.3 (see [go.mod](go.mod)), `make`, and Docker for the end-to-end tests. The Makefile downloads its other tools (controller-gen, golangci-lint, buf, envtest, ginkgo) into `./bin` on first use. Run `make help` to list every target.

Dependency updates target the newest compatible stable releases under the existing module paths. Keep `github.com/google/cel-go` at v0.31.0, the last release on the old path; v0.32.0 uses `cel.dev/cel-go`. Kubernetes v0.37.1 requires SMD/v6 schema types, with `sigs.k8s.io/structured-merge-diff/v6` kept at v6.4.2. kube-openapi's SMD/v7 migration landed in [upstream PR #636](https://github.com/kubernetes/kube-openapi/pull/636) on September 4, making newer revisions type-incompatible with Kubernetes v0.37.1. Keep kube-openapi `v0.0.0-20260821135717-be32def86098`, the latest compatible baseline.

For the docs site, use the Node version pinned in `.github/workflows/pages.yml`. Update direct npm dependencies to their latest stable releases and refresh the lockfile within upstream dependency ranges. Keep required older transitive versions, such as Babel 7 for magicast and p-queue 9 for Astro; do not force incompatible majors with overrides. Exclude prereleases even when a registry's `latest` tag points to one.

```sh
make test-fast   # unit tests without envtest
make test        # full unit and integration suite, including envtest
make lint        # golangci-lint
```

With Node.js and Docker available, build the devcontainer using the same pinned CLI and feature lockfile as CI:

```sh
npx --yes --package @devcontainers/cli@0.89.0 devcontainer build \
  --workspace-folder . --frozen-lockfile --no-cache
```

This builds the Debian Trixie development image without starting its Docker-in-Docker container or running lifecycle hooks. Keep `.devcontainer/devcontainer-lock.json` checked in; the frozen build rejects feature resolution that would change it.

The runtime images use Alpine 3.24 packages at that distribution's latest available versions. These may trail upstream releases; we do not source-build replacements to bypass the Alpine package ceiling.

The API group and CSI provisioner name is `pillar-csi.bhyoo.com`. All CRDs are cluster-scoped: `PillarAgent`, `PillarStore`, `PillarProtocol`, `PillarStorageClass`, and the internal `PillarVolumeState`.

## Generated files

Do not edit these files by hand. CI regenerates them and fails on `git diff --exit-code` if they differ.

| Generated files | Source | Regenerate with |
|---|---|---|
| `config/crd/`, `config/rbac/role.yaml`, `config/webhook/`, `charts/pillar-csi/templates/crds.yaml` | Markers in `api/v1alpha1/*_types.go` and `//+kubebuilder:rbac` markers in the controllers | `make manifests` |
| `api/v1alpha1/zz_generated.deepcopy.go` | Fields in `api/v1alpha1/*_types.go` | `make generate` |
| `gen/go/` | `proto/` | `make proto-gen` |

Scaffold new CRDs, controllers, and webhooks with the `kubebuilder` CLI; the `PROJECT` file tracks the scaffolding. Hand-written code lives in `api/v1alpha1/*_types.go`, `internal/controller/`, `internal/webhook/v1alpha1/`, `internal/agent/` (including the LIO iSCSI target in `internal/agent/lio/`), `internal/csi/`, `internal/iscsi/` (the node's in-process iSCSI initiator), and `proto/`.

When you change `proto/`, run `make proto-lint` and `make proto-breaking` to check wire compatibility.

## Handling failures

pillar-csi writes to kernel interfaces, and a swallowed error there leaves storage in a state nobody expects.

- Never drop a failed `configfs` or `sysfs` write with `continue`, a silent ignore, or a debug log.
- When the resulting state matters, read the value back right after the write and return an error if it differs.
- On cleanup and rollback paths, return any failure that affects storage or connection state to the caller, or at least log it with `log.Error`.
- Include the operation (`write`, `disconnect`, `expand`), the target (`path`, `NQN`, `IQN`, `device`, `volumeID`), and the cause (`%w`) in every error message.

## End-to-end test cases

Each TC ID in [docs/E2E-TESTCASES.md](docs/E2E-TESTCASES.md) maps one-to-one to a Ginkgo node name in `test/e2e/`. When you add or remove a test case, update both and run `make verify-tc-ids`. Gate test cases with `Fail()` or `Expect()`; do not use a conditional `Skip()`.

Check cleanup against the resource owned by the test. For TCP listeners, verify that the retained listener rejects `Accept()` with `net.ErrClosed`; rebinding a released ephemeral address can race with another parallel worker.

## CI checks

Pull requests and pushes to `master` run the same CI workflow. Code changes run lint, unit and integration tests, CSI conformance, chart rendering, benchmarks, and both Kind and upstream External Storage E2E suites. Ordinary source PRs compile all eight platforms through the Dockerfile's builder stage, which declares `TARGETOS`/`TARGETARCH`/`TARGETVARIANT` after `COPY . .` so the architecture-independent `go mod download` step shares one vertex across platforms while the build args remain in scope for each architecture-specific compile. Image-contract changes, master pushes, and failed change detection run the full eight-platform OCI build. Documentation-only changes skip the code checks; `docs/E2E-TESTCASES.md` still counts as code. Failed change detection runs the checks rather than skipping them. Superseded pull-request runs are cancelled; master and daily runs are retained.

The `CI gate` job runs on every pull request and is the CI workflow's required status check. It aggregates the jobs that block merge: Lint, Test, Build (all platforms), the Kind E2E suite, the runtime-image build that E2E consumes, and the devcontainer build. Each required job must succeed. A skipped code job passes only when change detection succeeded and found a documentation-only change, or, for E2E, when a pushed commit message contains `[skip e2e]` or `[e2e skip]`. Any other skip, such as one caused by cancelling the run, fails the gate. The other CI jobs report results but do not block merge. To make a job blocking, add it to the gate's `needs` list and result check in `.github/workflows/ci.yml`.

The devcontainer build runs only for `.devcontainer/**` or `.github/workflows/ci.yml` changes. It also runs if change detection fails or its devcontainer output is missing. The gate accepts a skipped devcontainer build only when detection succeeded and explicitly found no relevant change, so ordinary source PRs do not rebuild the devcontainer's Git feature. This uses the existing `CI gate` required check; no separate required status check is needed.

TC coverage verification runs once through `make test`, and its report appears in the Test job summary. The daily workflow also runs the separate Docker multi-node data-path suite.

CI builds the exact checkout's amd64 runtime images once in a job parallel to the platform build. Kind and External Storage E2E consume a run-scoped archive after verifying the source SHA, image tag, and image IDs, then load the images into fresh clusters. Local and daily runs still build their own images. Set `E2E_PREBUILT_IMAGES=true` only when the expected images are already loaded in the Docker daemon; the harness still performs Kind loading. CI also supports manual dispatch for measurements.

The csi-sanity job may opt in with `make test-csi-sanity CSI_SANITY_SKIP_VET=true`. The flag drops only the duplicate untagged `go vet ./...` prerequisite; the Test job remains the vet authority through `make test`, and the tagged `go test` run still vets the sanity package itself. Local and default runs keep vet.

The Docker suite's internal and external topologies run as independent matrix jobs on separate GitHub-hosted VMs, so storage kernel and configfs state are isolated. Both jobs run the existing NVMe-oF, iSCSI, and data-path scenarios and clean up independently; a failure in one does not cancel the other. Each job repeats image builds and setup, which may increase total job minutes; wall-clock savings must be measured. Local `make test-docker-e2e` still runs both topologies sequentially by default.

Docker suite tests run serially by default. `PILLAR_E2E_PARALLEL=true make test-docker-e2e` runs the top-level tests that call `parallelDockerE2E` (topology, admission, NVMe and iSCSI online expansion, periodic and iSCSI trim, and the wrong-CHAP-secret check) up to four at a time (`go test -parallel=4`). An opted-in test owns its namespace, volumes, targets, and Secrets and never asserts that a node has no connection or controller to a target port, because those checks see every socket to the shared NVMe/TCP and iSCSI ports. Every other test runs alone: Go runs all serial top-level tests while the opted-in tests are paused, so `TestLocalAttachForceDetachFencing`, which stops the storage kubelet and taints the node, and the handoff, disconnect, local-attach, and initiator-rejection tests never overlap another test. New tests stay serial unless they call `parallelDockerE2E` and meet those rules.

Daily E2E enables this opt-in in each isolated topology job. Local Docker suite runs remain serial unless `PILLAR_E2E_PARALLEL=true` is set.

Local `make test-docker-e2e` runs keep the nested Docker's pulled images, BuildKit cache (including the Go build cache mount), and the harness's Go module and build caches in Compose volumes, so a rerun rebuilds only what changed. Each run still starts and ends without Kind clusters, containers, networks, or E2E-owned host storage state. Base image tags such as `golang:1.27.3-alpine3.24` are not re-pulled while cached; run `make clean-docker-e2e-cache` to drop every cache and start from scratch. CI runners start with empty volumes.

External Storage E2E uses four workers when invoked by the CI and Daily callers; the reusable workflow defaults to two workers when called standalone. Local defaults remain one worker and 5 GiB. On a Linux host with the required storage modules, set `VG_SIZE=15G` when bootstrapping and `GINKGO_PROCS=2` when running `make test-external-e2e` to use the reusable-workflow settings locally. Parallel execution uses the matching Ginkgo binary from the upstream Kubernetes test bundle and keeps the same focus and skip filters.

CI caches versioned tools and test bundles. The multi-platform build reuses a Go compiler cache keyed by the Dockerfile and Go dependencies, not each commit. The runtime-image job has a separate amd64 compiler cache under the same dependency-key scheme. Go still recompiles changed source packages. Pull requests and manual non-master runs restore these caches without exporting them; master refreshes them when their dependency keys change. The Lint job restores a single archive that pairs the Go build cache (`GOCACHE`) with golangci-lint's analysis cache (`~/.cache/golangci-lint`), and the Test job restores a separate `GOCACHE`-only archive; their keys pin the runner, installed Go version, and `go.mod`/`go.sum` digests, with `.golangci.yml` in the Lint fingerprint. Go revalidates stored objects against source and flag hashes, so changed code still recompiles, and a master push publishes a refreshed archive only after the job succeeds on a primary-key miss. A dependency change creates a new archive, while eviction or the first run rebuilds either cache from scratch on that branch. Runtime image layers use target-specific caches for the full matrix and a separate scope for the shared amd64 artifact, exported by master. Release builds restore the full-matrix compiler and target-specific runtime caches as read-only consumers; they do not export either cache.

Before installing host kernel modules, the Kind, External Storage, and daily Docker E2E jobs run `hack/ci-prefer-ubuntu-mirror.sh`. On GitHub-hosted Ubuntu runners it changes only the priorities in `/etc/apt/apt-mirrors.txt` so APT tries `archive.ubuntu.com` before `azure.archive.ubuntu.com`. Every mirror stays as a fallback, and sources, signing keys, and packages are unchanged. If the file layout is anything else, the script leaves it alone and logs a notice.

## Before you commit

- `make lint` must report 0 issues.
- If you changed a `_types.go` file, an RBAC marker, or `proto/`, run the matching generator and commit its output with your change.
- Run `make test-fast`, and `make test` for changes that touch controllers or webhooks.
- Start the commit subject with a conventional prefix such as `fix:`, `feat:`, `test:`, `docs:`, or `build(deps):`, and explain why the change is needed rather than restating the diff.

## License

By contributing, you agree that your contributions are licensed under the [Apache License 2.0](LICENSE).