test: [FEP-0001] spike — cluster claim workflow verification harness (#791) - #811
Draft
Yetkin Timocin (ytimocin) wants to merge 5 commits into
Draft
test: [FEP-0001] spike — cluster claim workflow verification harness (#791)#811Yetkin Timocin (ytimocin) wants to merge 5 commits into
Yetkin Timocin (ytimocin) wants to merge 5 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This was referenced Aug 12, 2026
Yetkin Timocin (ytimocin)
force-pushed
the
spike/fep0001-clusterclaim-verify
branch
from
August 18, 2026 23:20
7270d47 to
f2f939e
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Introduces an envtest spike for validating the FEP-0001 cluster-claim workflow and documenting unresolved contract gaps.
Changes:
- Adds fake provisioning, selector matching, and claim withdrawal prototypes.
- Adds 19 workflow, eligibility, lifecycle, and schema-validation specs.
- Documents findings and recommended follow-up work.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
SPIKE.md |
Documents scope, findings, and follow-ups. |
doc.go |
Describes the spike package. |
eligibility_test.go |
Tests scheduler eligibility timing. |
fakeprovisioner.go |
Simulates provisioning outcomes. |
lifecycle_test.go |
Tests deletion, ownership, and naming gaps. |
matcher.go |
Implements label selector matching. |
suite_test.go |
Configures the envtest suite. |
validation_test.go |
Tests schema and CEL behavior. |
withdrawer.go |
Prototypes claim withdrawal. |
workflow_test.go |
Tests end-to-end claim scenarios. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| This directory is a time-boxed spike for | ||
| [#791 — Verify the cluster claim workflow](https://github.com/kubefleet-dev/kubefleet/issues/791). | ||
| It is not production code and is not wired into any build target. It runs |
Comment on lines
+103
to
+107
| By("the provisioner creates a matching member cluster and the withdrawer deletes the claim") | ||
| Eventually(func() bool { | ||
| err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterRequest{}) | ||
| return err != nil && client.IgnoreNotFound(err) == nil | ||
| }, eventuallyTimeout, eventuallyInterval).Should(BeTrue(), "claim should be withdrawn (deleted)") |
Comment on lines
+131
to
+133
| if err := p.Create(ctx, memberCluster); err != nil && !errors.IsAlreadyExists(err) { | ||
| return err | ||
| } |
Prototype both sides of the FEP-0001 claim contract against envtest: a fake platform provisioner and a claim-withdrawer standing in for the placement policy controller slice, plus CEL/schema and workflow suites. Documents one API bug (field-level CEL immutability on clusterSelectorTerms is bypassed by unsetting the field) and the open contract questions the verification work needs answered; see test/spike/clusterclaim/SPIKE.md. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
Extends the cluster claim workflow spike with runnable demonstrations of the contract points round 1 left as open questions: - eligibility_test.go: the join-window gap. Raw-match withdrawal fires while the scheduler's own eligibility gate (clustereligibilitychecker) still rejects the cluster; an eligibility-gated withdrawer variant (toggleable in withdrawer.go) holds the claim through the join window. - lifecycle_test.go: provisioner finalizers turn withdrawal into a lingering Terminating state with undefined concurrency semantics; deleting the referenced PlacementPolicy orphans the claim; cross-scope ownerReferences are accepted at admission but invalid at GC time; deterministic claim names derived from policy names collide across namespaces. - validation_test.go: the CEL immutability bypass is symmetric (terms can be added after a no-terms create); Completed=True lands without provisionedClusterName and terminal states can be downgraded. Suite: 19 specs, all green. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
The happy-path spec could pass without the provisioner's half of the contract ever being honored: withdrawal needs only the member cluster to exist, and could race the status update recording fulfillment. The spec now holds withdrawal with the eligibility gate, asserts the claim was completed and names its cluster, and releases the hold the honest way -- the member agent reporting in, the same transition the eligibility specs drive. The provisioner no longer reports completion when the deterministic cluster name is already taken: a squatting cluster that does not satisfy the claim's own terms is a collision, not a fulfillment, and a new spec pins that. The suite joins the integration test target so the contract gaps it pins cannot regress unnoticed, cleanup now covers claims and resets the eligibility gate whatever a spec did, and the review pass surfaced one more gap for the findings list: time-based eligibility staleness never re-triggers claim evaluation, since the withdrawer only reacts to writes. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
Rebase the spike onto main after #803 renamed the cluster request API to cluster claims, and switch every reference in the spike to the new names: ClusterClaim/ClusterClaimList/ClusterClaimSpec, ClusterClaimCondTypeCompleted, and the prose in SPIKE.md and package docs. No behavioural change; the spike suite passes unchanged against the renamed CRD. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
Yetkin Timocin (ytimocin)
force-pushed
the
spike/fep0001-clusterclaim-verify
branch
from
August 27, 2026 00:40
724b46d to
6b3e12a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of your changes
Spike for #791: prototypes both sides of the FEP-0001 cluster claim contract and runs the full loop against envtest, so the verification matrix can be built before the controllers (#786/#788) land.
FakeProvisioner— the platform/cloud-provider role (Fulfill / Fail / Ignore policies); writes only the status fields that side of the contract owns.Withdrawer— prototype of the claim-management slice of [FEP-0001] Implement the Placement Policy controllers #786: withdraws claims once any member cluster satisfies the selector terms, refresheslastObservedMostRecentClusterCreationTimestampotherwise.eligibility_test.gopins the join-window gap (raw-match withdrawal fires whileclustereligibilitychecker.IsEligiblestill rejects the cluster; an eligibility-gated withdrawer variant holds the claim until the member agent reports in) andlifecycle_test.gopins the deletion/ownership gaps (provisioner finalizers → indefinite Terminating with undefined concurrency semantics; policy deletion orphans claims; cross-scope ownerRefs accepted at admission but invalid at GC; claim-name collisions across namespaces).Completed=True+provisionedClusterName) while withdrawal is held by the eligibility gate, and the release is the member agent genuinely reporting in; a name-squatting spec pins that a pre-existing cluster holding the deterministic provisioned name but not satisfying the claim is never reported as fulfillment (the fake provisioner now validates onAlreadyExistsinstead of assuming); and the suite is wired intomake integration-testso the pinned gaps cannot regress silently. The review also surfaced one more contract gap for the findings list: time-based eligibility (heartbeat staleness) never re-triggers claim evaluation, since the withdrawer only reacts to writes. 20 specs, all green.Findings are written up in
test/spike/clusterclaim/SPIKE.md, including the CEL immutability bypass now filed as #810 (pinned here as aKNOWN GAPspec to flip when fixed) and the open contract questions (delete-while-provisioning semantics, status ownership,Failedretry policy, freshness granularity).Part of #791. Not intended to merge as-is — the withdrawer gets replaced by the real #786 controller and the suites grow into the verification matrix; opening as a draft so the approach is visible and discussable.
I have:
make reviewableto ensure this PR is ready for review.How has this code been tested
KUBEBUILDER_ASSETS=... go test ./test/spike/clusterclaim/... -count=1(envtest, K8s 1.33) — 20/20 specs pass; also runs inmake integration-testnow.Special notes for your reviewer
The
KNOWN GAPspecs invalidation_test.gointentionally assert the current (buggy) behavior from #810 — both the removal and the add-after-create direction — so the suite stays green; flip them once the spec-level CEL guard lands (candidate vehicle: #803). Round-2 additions also pin two status-contract holes (Completed=True without provisionedClusterName; terminal-state downgrade) the same way.