Skip to content

feat: implement pvc primitive#34

Open
sourcehawk wants to merge 21 commits intomainfrom
feature/pvc-primitive
Open

feat: implement pvc primitive#34
sourcehawk wants to merge 21 commits intomainfrom
feature/pvc-primitive

Conversation

@sourcehawk
Copy link
Owner

@sourcehawk sourcehawk commented Mar 22, 2026

Implements the pvc Kubernetes resource primitive following the pattern established by the existing ConfigMap and Deployment primitives.

Summary

  • Adds pvc primitive package under pkg/primitives/pvc/
  • Implements required lifecycle interfaces
  • Includes editors, mutator, flavors, and builder
  • Adds shared PVCSpecEditor under pkg/mutation/editors/
  • Updates docs/primitives.md to include PVC in the built-in primitives table

Checklist

  • Compiles cleanly
  • Tests pass
  • Follows naming conventions in CONTEXT.md

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Implements a new pvc primitive (PersistentVolumeClaim) that plugs into the framework’s integration-resource lifecycle, including typed mutation/editing support, default handlers, flavors, docs, and a runnable example.

Changes:

  • Added pkg/primitives/pvc/ (resource, builder, mutator, handlers, flavors) with unit tests.
  • Added a shared typed editor PVCSpecEditor under pkg/mutation/editors/ with tests.
  • Added a dedicated pvc-primitive example and new documentation page docs/primitives/pvc.md.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/primitives/pvc/resource.go Adds PVC integration Resource wrapper + default field applicator (immutable-field preservation).
pkg/primitives/pvc/mutator.go Adds PVC mutator with plan/apply pattern and typed spec/meta edits.
pkg/primitives/pvc/mutator_test.go Unit tests for mutator behavior and ordering.
pkg/primitives/pvc/handlers.go Default operational + suspension handlers for PVCs.
pkg/primitives/pvc/handlers_test.go Unit tests for default handlers.
pkg/primitives/pvc/flavors.go PVC flavors for preserving labels/annotations.
pkg/primitives/pvc/flavors_test.go Unit tests for flavors + builder integration check.
pkg/primitives/pvc/builder.go Fluent builder for PVC primitive configuration.
pkg/primitives/pvc/builder_test.go Unit tests for builder configuration/validation wiring.
pkg/mutation/editors/pvcspec.go Adds shared PVCSpecEditor for typed PVC spec mutations.
pkg/mutation/editors/pvcspec_test.go Unit tests for PVCSpecEditor.
examples/pvc-primitive/resources/pvc.go Example resource factory using pvc.Builder + mutations/flavors/extractor.
examples/pvc-primitive/README.md Documentation for running/understanding the PVC example.
examples/pvc-primitive/main.go Runnable fake-client example demonstrating versioning/suspension flows.
examples/pvc-primitive/features/mutations.go Example feature-gated PVC mutations.
examples/pvc-primitive/app/owner.go Re-export of shared ExampleApp types for the example.
examples/pvc-primitive/app/controller.go Example controller wiring the PVC resource into a Component.
docs/primitives/pvc.md Adds primitive documentation for PVC.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:
None

Intentionally ignored:

  • pvcspec.go line 6 (PR description/checklist mismatch): This comment is about PR metadata, not code. The PVCSpecEditor is a new file added to pkg/mutation/editors/ — it extends the shared editors package but does not modify any existing shared files. Adding a new editor alongside the primitive it supports is the intended pattern (consistent with how other primitives work). The PR description could be clarified to note this addition, but no code change is warranted and splitting into a separate PR would lose the cohesion of the feature.

<!-- claude-review-cycle -->

@sourcehawk sourcehawk force-pushed the feature/pvc-primitive branch from 1789aed to 2d07ac1 Compare March 23, 2026 00:49
Copilot AI review requested due to automatic review settings March 23, 2026 00:49
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • Added PVC primitive entry to the Built-in Primitives table in docs/primitives.md so users can discover the PVC primitive documentation from the main primitives page.
  • Merged latest changes from main into the feature branch.

Intentionally ignored:

  • Double pipe separator comment (pvc.md line 15): The capabilities table already uses single | separators, consistent with configmap.md. No double pipes exist in the file.
  • Mutator beginFeature export comment (mutator.go line 45): The unexported beginFeature() method is the same pattern used by all other primitives (deployment, configmap). The FeatureMutator interface in internal/generic uses an unexported method by design. Changing this would require modifying shared infrastructure and affects all primitives, not just PVC. This is a pre-existing architectural pattern, not a PVC-specific issue.

Copilot AI review requested due to automatic review settings March 23, 2026 16:05
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 5 comments.

Copilot AI review requested due to automatic review settings March 23, 2026 16:58
@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • resource.go:32 — PreserveStatus was already added in a prior commit; confirmed present and working
  • resource_test.go:136 — TestDefaultFieldApplicator_PreservesStatus regression test was already added in a prior commit; confirmed present and passing
  • docs/primitives/pvc.md:134 — Updated mutation ordering docs to accurately state that PVC mutations are applied in a single deterministic sequence without per-feature boundaries

Intentionally ignored:

  • mutator.go:51 (FeatureMutator unexported method) — Valid observation, but this is a pre-existing framework-level issue affecting all primitives (deployment, configmap, pvc), not specific to this PR. The unexported beginFeature() in generic.FeatureMutator prevents any external package from satisfying the interface. Should be addressed in a separate framework PR.
  • docs/primitives.md:134 (PR checklist accuracy) — This is about PR description metadata, not a code issue. The PR description can be updated separately if needed.

<!-- claude-review-cycle -->

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

"k8s.io/apimachinery/pkg/api/resource"
)

// Mutation defines a mutation that is applied to a pvc Mutator
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

Spelling/terminology: "a pvc Mutator" should be "a PVC Mutator" (PVC is an acronym) to match naming used elsewhere in the repo and in the rest of this package's GoDoc.

Suggested change
// Mutation defines a mutation that is applied to a pvc Mutator
// Mutation defines a mutation that is applied to a PVC Mutator

Copilot uses AI. Check for mistakes.
sourcehawk and others added 10 commits March 23, 2026 20:16
Typed editor providing SetStorageRequest, SetAccessModes, SetStorageClassName,
SetVolumeMode, SetVolumeName, and Raw() escape hatch. Follows the existing
editor patterns (DeploymentSpecEditor, ContainerEditor, etc.).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Implements PersistentVolumeClaim as an Integration primitive with:
- Builder with fluent API for mutations, flavors, status handlers
- Mutator with plan-and-apply pattern (metadata edits, spec edits)
- DefaultFieldApplicator preserving immutable fields (accessModes,
  storageClassName, volumeMode, volumeName) on existing PVCs
- DefaultOperationalStatusHandler (Bound/Pending/Lost → status mapping)
- DefaultSuspendMutationHandler (no-op, PVCs have no runtime state)
- DefaultSuspensionStatusHandler (always Suspended)
- DefaultDeleteOnSuspendHandler (false, preserve data)
- PreserveCurrentLabels and PreserveCurrentAnnotations flavors

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Covers capabilities, building, default field application (immutable field
preservation), mutations (boolean-gated, version-gated), internal ordering,
editors (PVCSpecEditor, ObjectMetaEditor), convenience methods, flavors,
status handlers, and usage guidance.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Demonstrates PVC builder with version labels, storage annotations,
conditional large-storage expansion, label/annotation flavors, data
extraction, and suspension. Follows the configmap and deployment
example patterns.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address Copilot review comment: DefaultFieldApplicator had no test
coverage for its immutable field preservation logic. Add tests
validating that:
- New PVCs (empty ResourceVersion) use all fields from desired
- Existing PVCs preserve immutable fields (accessModes,
  storageClassName, volumeMode, volumeName) from current
- Mutable fields (e.g. storage requests) come from desired

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…inFeature

Aligns with the fix applied to the deployment and configmap primitives,
preventing an empty leading feature plan when the mutator helper calls
beginFeature().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add generic.PreserveStatus call in DefaultFieldApplicator to prevent
  status clobbering on update
- Add TestDefaultFieldApplicator_PreservesStatus regression test
- Update pvc.md to accurately describe mutation ordering (no per-feature
  boundaries due to unexported FeatureMutator interface)
- Update DefaultFieldApplicator doc comment to mention Status preservation

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Align with upstream change in main that exported BeginFeature() on the
FeatureMutator interface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sourcehawk sourcehawk force-pushed the feature/pvc-primitive branch from afbb0a0 to dc9f30a Compare March 23, 2026 21:38
@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 24, 2026 02:08
@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 24, 2026 02:11
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Add integration tests for the Resource type covering Identity, Object
deep copy, Mutate with mutations and field flavors, ConvergingStatus
for Bound/Pending/Lost phases, default suspension behavior, and data
extraction. These tests validate the generic.IntegrationResource
plumbing end-to-end, matching coverage provided by other primitives.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sourcehawk
Copy link
Owner Author

Claude Review Cycle 1 Complete

Addressed:

  • Added resource-level behavioral tests to pkg/primitives/pvc/resource_test.go covering: Identity, Object deep copy, Mutate with a mutation and a field flavor, feature ordering, ConvergingStatus for Bound/Pending/Lost phases, DeleteOnSuspend (default=false), Suspend + SuspensionStatus (reports Suspended), and ExtractData. These mirror the patterns in the deployment primitive's resource_test.go.

Intentionally ignored:
None

<!-- claude-review-cycle -->

@sourcehawk sourcehawk requested review from Copilot and removed request for Copilot March 24, 2026 02:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

sourcehawk and others added 4 commits March 24, 2026 17:51
Align PVC mutator with deployment/configmap: NewMutator no longer
creates an initial feature plan. BeginFeature must be called before
registering mutations. Updates all tests accordingly and adds
constructor/feature-plan invariant tests.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The framework now uses Server-Side Apply instead of ctrl.CreateOrUpdate,
so field applicators and flavors are no longer needed. This removes
DefaultFieldApplicator, WithCustomFieldApplicator, WithFieldApplicationFlavor,
the FieldApplicationFlavor type, flavors.go, flavors_test.go, and updates
all tests to use the Object()-then-Mutate() pattern.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 25, 2026 14:10
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Comment on lines 142 to +148
## Built-in Primitives

| Primitive | Category | Documentation |
| --------------------------- | -------- | ----------------------------------------- |
| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) |
| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) |
| Primitive | Category | Documentation |
| --------------------------- | ----------- | ----------------------------------------- |
| `pkg/primitives/deployment` | Workload | [deployment.md](primitives/deployment.md) |
| `pkg/primitives/configmap` | Static | [configmap.md](primitives/configmap.md) |
| `pkg/primitives/pvc` | Integration | [pvc.md](primitives/pvc.md) |
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

PR description mentions the PVC primitive "includes ... flavors", but there doesn’t appear to be any flavor API/types added under pkg/primitives/pvc (or in the generic builders) in this change. Either implement the flavor functionality for PVC or update the PR description/checklist to match what was actually delivered.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants