diff --git a/Makefile b/Makefile index 089512c02..ae8ef0453 100644 --- a/Makefile +++ b/Makefile @@ -159,7 +159,8 @@ integration-test: $(ENVTEST) ## Run integration tests export CGO_ENABLED=1 && \ export KUBEBUILDER_ASSETS="$(shell $(ENVTEST) use $(ENVTEST_K8S_VERSION) -p path)" && \ ginkgo -v -p --race --cover --coverpkg=./pkg/scheduler/... -coverprofile=scheduler-it.out ./test/scheduler && \ - ginkgo -v -p --race --cover --coverpkg=./apis/ -coverprofile=api-validation-it.out ./test/apis/... + ginkgo -v -p --race --cover --coverpkg=./apis/ -coverprofile=api-validation-it.out ./test/apis/... && \ + ginkgo -v -p --race ./test/spike/... .PHONY: kubebuilder-assets-path kubebuilder-assets-path: $(ENVTEST) ## Get the path to kubebuilder assets diff --git a/test/spike/clusterclaim/SPIKE.md b/test/spike/clusterclaim/SPIKE.md new file mode 100644 index 000000000..a99c15f26 --- /dev/null +++ b/test/spike/clusterclaim/SPIKE.md @@ -0,0 +1,222 @@ +# Spike: verifying the cluster claim workflow (FEP-0001, issue #791) + +This directory is a time-boxed spike for +[#791 — Verify the cluster claim workflow](https://github.com/kubefleet-dev/kubefleet/issues/791). +It is spike code held to test-quality standards: the suite runs in +`make integration-test` (and therefore CI) so the contract gaps it pins +cannot regress silently. It also runs +against the API types merged in #781 (`ClusterClaim`, as renamed +by #803). + +## What the spike does + +The FEP-0001 claim workflow is a contract between two independent controllers: +KubeFleet creates a claim when a cluster selector can't be fulfilled, an +external platform provisions a matching cluster and reports back, and +KubeFleet withdraws the claim the moment the selector is satisfied by *any* +cluster. Since neither controller exists yet (#786/#788), the spike prototypes +the minimum of both sides and runs the whole loop against envtest: + +- `fakeprovisioner.go` — the platform side (the role a CAPI- or ARM-based + provisioner plays in production). Policies: `Fulfill` (create a matching + `MemberCluster`, set `Completed=True` + `provisionedClusterName`), `Fail` + (`Completed=False`, reason `Failed`), `Ignore`. It only writes the status + fields that side of the contract owns. +- `withdrawer.go` — a prototype of the claim-management slice of #786: + re-evaluates every claim on any `MemberCluster` event, withdraws fulfilled + claims regardless of their status, refreshes + `lastObservedMostRecentClusterCreationTimestamp` on unfulfilled ones. +- `matcher.go` — selector-term matching (labels only; property expressions + need the property provider and are out of spike scope). +- `validation_test.go` — pins the CEL/schema contract of the claim API. +- `workflow_test.go` — pins the behavioral contract: happy path, + withdraw-by-other-cluster, provisioning failure, staleness refresh. +- `eligibility_test.go` (round 2) — pins the join-window gap: raw-match + withdrawal vs. withdrawal gated on the scheduler's real eligibility check. +- `lifecycle_test.go` (round 2) — pins the deletion/ownership gaps: + provisioner finalizers, policy-deletion orphans, cross-scope ownerRefs, + claim-name collisions. + +## Run it + +```sh +KUBEBUILDER_ASSETS="$(setup-envtest use 1.33.0 -p path)" \ + go test ./test/spike/clusterclaim/... -count=1 +``` + +## Findings + +### 1. CEL immutability on `clusterSelectorTerms` can be bypassed (API bug) + +`spec.clusterSelectorTerms` carries a field-level +`+kubebuilder:validation:XValidation:rule="self == oldSelf"`. Kubernetes skips +field-scoped transition rules when the field is absent from the new object, so +an update that *removes* the field entirely is accepted — and since "no terms" +means "any cluster satisfies this claim", the mutation silently widens the +claim. Appending, dropping one term, and reordering are all correctly +rejected; full removal is not. Fix: duplicate the guard at the spec level, +e.g. `has(self.clusterSelectorTerms) == has(oldSelf.clusterSelectorTerms)`. +#803 (the rename PR) is a natural place. The +`KNOWN GAP` spec in `validation_test.go` pins the current behavior and should +be flipped when the fix lands. (`placementPolicyRef` is not affected — it is +required, so removal fails on that axis.) + +### 2. Contract points the FEP leaves open (verification blockers) + +These surfaced while writing the tests; each needs a design answer before +#786's reconciler hard-codes one: + +1. **Delete-while-provisioning.** Claims carry no finalizer, and withdrawal is + a hard delete "regardless of the status of the cluster claim." Nothing + tells a provisioner whether deletion means cancel-and-clean-up or + finish-and-orphan. Prior art (CAPI `Machine`, Karpenter `NodeClaim`) uses + finalizers so that "delete means stop and clean up" is explicit. +2. **Status ownership.** The provisioner owns `Completed` + + `provisionedClusterName`; KubeFleet owns + `lastObservedMostRecentClusterCreationTimestamp`. Two writers on one status + subresource works (the spike does it) but the field boundary is convention + only — it should be written down, and server-side apply field managers + considered. +3. **`Failed` has no follow-up policy.** A failed claim sits forever, + consuming the default 1-in-flight budget. Retry via fresh claim? Backoff? + Give up and surface on the placement? Related: no TTL for claims whose + provisioner crashed or was never installed. +4. **`Completed=True` with a non-matching cluster.** The status field's doc + comment implies re-evaluating an already-completed claim; the FEP never + defines what that means. +5. **Config surfaces don't exist.** The admin "eligible keys" allowlist and + the per-placement/per-fleet concurrency limits are prose-only — no field or + flag anywhere yet (the repo's pattern would be + `cmd/hubagent/options/featureflags.go`). +6. **Timestamp freshness is coarse.** `metav1.Time` is second-resolution, so + same-second joins tie; the marker is fleet-global, so unrelated joins mark + claims stale; and the two controllers read it through independent caches. + It works as an advisory skip-hint (the spike treats it that way) but should + be documented as advisory-only, or rebased on `resourceVersion`. + +### 3. Lessons from a production provisioner-side implementation + +An internal controller that provisions managed clusters from a claim-like CRD +(same two-controller shape as this contract) was reviewed alongside the spike. +The patterns most worth carrying into the claim design: + +- **Status atomicity via CEL.** Its CRD enforces "terminal state implies its + evidence" at the API server: `Failed` requires a failure reason and last + error; `Ready` requires the cluster's identity fields. The claim equivalent + — `Completed=True` requires `provisionedClusterName` — would turn a + controller-discipline convention into an apiserver guarantee. +- **Terminal states are CEL-pinned.** Once parked in a permanent failure the + phase cannot be downgraded, and identity fields (`clusterName`) are + immutable once set. Remediation is delete-and-recreate — which for a claim + API means the claimant must be prepared to re-issue. +- **Restart-safe idempotency needs no in-memory state.** The long-running + provisioning operation's poller is deliberately discarded; the only durable + state is a deterministic resource name plus the name recorded in status + (always preferred over re-derivation), and progress is re-derived from the + provider on every pass. Get-then-create, never blind create. +- **Transient vs. permanent error classification is a first-class table**, and + "transient" is only safe because every caller is idempotent. A claim + `Completed`/`Failed` reason set should make the terminal/retryable split + explicit rather than leaving it to string conventions. +- **The need disappearing does not cancel in-flight provisioning.** That + controller garbage-collects only after the cluster reaches Ready, with a + generous grace window and a three-stage uncached re-confirmation before any + destructive action — because tearing down a cluster someone still references + is far worse than leaking one for minutes. FEP-0001's + withdraw-at-any-moment-with-no-finalizer stance is notably more aggressive + than what that team found safe in production; it also had to add a + shared-resource guard after an incident where deleting one claim destroyed a + sibling's live cluster. Finding 2.1 above is not hypothetical. + +## Round 2: contract gaps verified empirically + +A second pass turned the open contract questions from finding 2 into pinned, +runnable demonstrations wherever envtest allows: + +### 4. The withdrawal predicate fires before the cluster is usable (join window) + +The FEP withdraws a claim once a matching cluster "is joined to the fleet", +without defining "joined". A provisioner-created `MemberCluster` matches its +selector at object-creation time; the scheduler's real gate +(`pkg/scheduler/clustereligibilitychecker`: member agent online, recent +heartbeat, `Joined=True`, healthy) passes minutes later, and taints are a +further, separate filter (the claim spec carries no tolerations at all). +`eligibility_test.go` demonstrates both readings side by side with the actual +checker: + +- as written, the claim is withdrawn while `IsEligible` still returns false — + the placement stays unschedulable with no outstanding claim (and no + concurrency-budget entry) to explain why; +- gated on `IsEligible`, the claim survives the join window and withdraws only + when the member agent reports in — the semantics #786 should implement. + +Withdrawal and the "still unfulfilled" check that gates *new* claims should +both reuse the scheduler's predicate. (Round 1's suite missed this because +envtest never populates agent status — the naive matcher and the real gate +agree vacuously there.) + +### 5. Deletion and ownership are unspecified, with sharp edges (`lifecycle_test.go`) + +- **Provisioner finalizers "work" but mean nothing.** The API machinery lets a + provisioner protect in-flight work with a finalizer; withdrawal then only + marks the claim Terminating, indefinitely. The contract must pick: does a + Terminating claim still occupy the concurrency slot (starvation) or is it + replaced (double-provisioning)? Neither answer exists today. +- **Policy deletion orphans claims forever.** Cross-scope ownerReferences + (namespaced `PlacementPolicy` owning a cluster-scoped claim) are invalid in + Kubernetes GC — yet the API server *accepts* them at admission, failing only + at GC time (`OwnerRefInvalidNamespace`, object skipped), which makes them an + attractive trap for #786. Cleanup has to be explicit reconciliation, and the + FEP doesn't specify it. `placementPolicyRef` also records no UID, so a + deleted-and-recreated same-named policy silently inherits the orphan. +- **Claim names collide across namespaces.** Claims are cluster-scoped; + policies are namespaced; the FEP's own examples reuse the policy name `app`. + Any deterministic name derived from the policy name alone collides — and a + deterministic name is exactly what restart-safe provisioners need (see the + get-or-create lesson in section 3). The naming scheme must include the + namespace (or a hash of it). + +### 6. The status contract has no teeth (additions to `validation_test.go`) + +- The immutability bypass is symmetric: terms can be *added* after a + no-terms create, silently narrowing the claim (and `[]` serializes as + absent via `omitempty`, so empty-list creates stay mutable too). The + spec-level guard fixes both directions. +- `Completed=True` with no `provisionedClusterName` is accepted; so is + downgrading a completed claim back to in-progress. Status atomicity and + terminal-state pinning (section 3) need to be CEL rules, not conventions. + +## Round 3: review hardening + +The review pass on this harness surfaced one more contract gap and tightened +two specs: + +- **No periodic re-evaluation of time-based eligibility.** The withdrawer + re-evaluates claims only on `ClusterClaim`/`MemberCluster` write events, + but the eligibility checker's heartbeat-staleness test is time-based: a + cluster can silently cross the staleness threshold with no corresponding + write, and nothing re-enqueues the claim to notice. The gated-withdrawal + reading needs a resync interval (or a periodic requeue) that the FEP does + not currently call for. +- The happy-path spec now proves the provisioner's half of the contract + (`Completed=True`, `provisionedClusterName`) while withdrawal is held by + the eligibility gate, and the release is the member agent genuinely + reporting in -- the same transition the eligibility specs drive -- rather + than a harness bypass. +- 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. + +## Suggested next steps + +1. File finding 1 against the API (fix in #803 or a follow-up), flip the + `KNOWN GAP` specs when merged (removal *and* add-after-create). +2. Raise findings 2.1–2.6 and 4–6 on the FEP/issue for maintainer decisions — + the round-2 findings (withdrawal predicate, deletion protocol, policy + cleanup, naming, status CEL) are the ones #786 would otherwise hard-code + answers to implicitly. +3. Once #786/#788 land, replace `withdrawer.go` with the real controller and + grow `workflow_test.go` into the full #791 verification matrix (concurrency + limits, eligible-keys gating, `whenUnfulfilled: KeepSearching`, + delete-races, policy-deletion cleanup) — keeping the eligibility-gated + specs as the acceptance bar for the withdrawal predicate. diff --git a/test/spike/clusterclaim/doc.go b/test/spike/clusterclaim/doc.go new file mode 100644 index 000000000..c9d912696 --- /dev/null +++ b/test/spike/clusterclaim/doc.go @@ -0,0 +1,41 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +// Package clusterclaim is a SPIKE for kubefleet-dev/kubefleet#791 ([FEP-0001] +// Verify the cluster claim workflow). It is NOT production code and is not +// wired into any build target. +// +// The spike demonstrates, against the merged v1alpha1 API types +// (ClusterClaim, as renamed in PR #803), the full claim +// lifecycle with the two controllers the FEP-0001 contract implies: +// +// - FakeProvisioner plays the platform/cloud-provider role (the part +// a platform claim reconciler or a CAPI-based controller would +// play in production): it watches claims and, per test policy, fulfills +// them (creates a matching MemberCluster, sets Completed=True and +// provisionedClusterName), fails them, or ignores them. +// - Withdrawer prototypes the KubeFleet placement-policy-controller slice +// under verification (issue #786 will implement it for real): it watches +// MemberClusters, matches them against each claim's clusterSelectorTerms, +// withdraws (deletes) fulfilled claims regardless of their status, and +// refreshes lastObservedMostRecentClusterCreationTimestamp on claims that +// remain unfulfilled. +// +// The test suites double as the seed of the #791 verification matrix: +// validation_test.go pins the CEL/schema contract; workflow_test.go pins the +// behavioral contract (fulfillment by the provisioned cluster, fulfillment by +// an unrelated cluster, provisioning failure, staleness refresh). +package clusterclaim diff --git a/test/spike/clusterclaim/eligibility_test.go b/test/spike/clusterclaim/eligibility_test.go new file mode 100644 index 000000000..00bde9fb4 --- /dev/null +++ b/test/spike/clusterclaim/eligibility_test.go @@ -0,0 +1,147 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" + "github.com/kubefleet-dev/kubefleet/pkg/scheduler/clustereligibilitychecker" +) + +// The join window: FEP-0001 withdraws a claim once a matching cluster "is +// joined to the fleet", but never says whether "joined" means the +// MemberCluster object exists or the cluster actually passes the scheduler's +// eligibility gate (member agent online, recent heartbeat, Joined=True, +// healthy — pkg/scheduler/clustereligibilitychecker). A provisioner-created +// MemberCluster matches its selector at object-creation time, minutes before +// the member agent comes up. These specs pin the difference between the two +// readings. +var _ = Describe("join-window eligibility gap", Ordered, func() { + var counter int + + newClaim := func(region string) *placementv1alpha1.ClusterClaim { + counter++ + return &placementv1alpha1.ClusterClaim{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("elig-claim-%d", counter)}, + Spec: placementv1alpha1.ClusterClaimSpec{ + PlacementPolicyRef: &placementv1alpha1.ObjectReference{ + Name: "app", + Namespace: "work", + APIGroup: "placement.kubefleet.dev", + APIVersion: "v1alpha1", + Kind: "PlacementPolicy", + }, + ClusterSelectorTerms: []placementv1alpha1.ClusterLabelAndPropertySelectorTerm{ + {MatchLabels: map[string]string{"topology.kubernetes.io/region": region}}, + }, + }, + } + } + + newMemberCluster := func(name string, labels map[string]string) *clusterv1beta1.MemberCluster { + return &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels}, + Spec: clusterv1beta1.MemberClusterSpec{ + Identity: rbacv1.Subject{ + Kind: "ServiceAccount", + Name: name, + Namespace: "fleet-system", + }, + }, + } + } + + AfterEach(func() { + withdrawer.SetEligibilityGate(false) + mcList := &clusterv1beta1.MemberClusterList{} + Expect(k8sClient.List(ctx, mcList)).Should(Succeed()) + for i := range mcList.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &mcList.Items[i]))).Should(Succeed()) + } + Eventually(func() (int, error) { + if err := k8sClient.List(ctx, mcList); err != nil { + return -1, err + } + return len(mcList.Items), nil + }, eventuallyTimeout, eventuallyInterval).Should(Equal(0)) + }) + + // FINDING (spike round 2): under the FEP-as-written reading, the claim — + // the only visible "a cluster is still needed" signal, and the thing + // budgeted against the concurrency limit — is withdrawn while the matching + // cluster is still unusable to the scheduler. The placement stays + // unscheduled with no outstanding claim to explain why. + It("KNOWN GAP: raw-match withdrawal fires for a cluster the scheduler cannot use yet", func() { + claim := newClaim("eastus2") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("a provisioned cluster registers with matching labels but its member agent has not reported yet") + mc := newMemberCluster("registered-not-joined", map[string]string{"topology.kubernetes.io/region": "eastus2"}) + Expect(k8sClient.Create(ctx, mc)).Should(Succeed()) + + By("the scheduler's own eligibility gate would reject this cluster right now") + eligible, reason := clustereligibilitychecker.New().IsEligible(mc) + Expect(eligible).Should(BeFalse()) + Expect(reason).Should(ContainSubstring("not online")) + + By("yet the claim is withdrawn — the 'still needed' signal is gone while nothing is schedulable") + Eventually(func() bool { + err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + }) + + It("recommended reading: eligibility-gated withdrawal holds the claim through the join window", func() { + withdrawer.SetEligibilityGate(true) + claim := newClaim("westus2") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("a matching but not-yet-joined cluster does not withdraw the claim") + mc := newMemberCluster("joining-westus2", map[string]string{"topology.kubernetes.io/region": "westus2"}) + Expect(k8sClient.Create(ctx, mc)).Should(Succeed()) + Consistently(func() error { + return k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + }, time.Second*2, eventuallyInterval).Should(Succeed()) + + By("once the member agent reports joined + healthy + heartbeating, the claim is withdrawn") + Expect(k8sClient.Get(ctx, clientKey(mc.Name), mc)).Should(Succeed()) + now := metav1.Now() + mc.Status.AgentStatus = []clusterv1beta1.AgentStatus{{ + Type: clusterv1beta1.MemberAgent, + Conditions: []metav1.Condition{ + {Type: string(clusterv1beta1.AgentJoined), Status: metav1.ConditionTrue, Reason: "AgentJoined", Message: "spike: simulated join", LastTransitionTime: now}, + {Type: string(clusterv1beta1.AgentHealthy), Status: metav1.ConditionTrue, Reason: "AgentHealthy", Message: "spike: simulated health", LastTransitionTime: now}, + }, + LastReceivedHeartbeat: now, + }} + Expect(k8sClient.Status().Update(ctx, mc)).Should(Succeed()) + + Eventually(func() bool { + err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + }) +}) diff --git a/test/spike/clusterclaim/fakeprovisioner.go b/test/spike/clusterclaim/fakeprovisioner.go new file mode 100644 index 000000000..e24d50342 --- /dev/null +++ b/test/spike/clusterclaim/fakeprovisioner.go @@ -0,0 +1,175 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "context" + "fmt" + "maps" + "sync" + + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" +) + +// ProvisionPolicy controls how the FakeProvisioner responds to a claim. +type ProvisionPolicy string + +const ( + // PolicyFulfill provisions a MemberCluster whose labels are copied from the + // claim's first selector term's MatchLabels, then reports completion. + PolicyFulfill ProvisionPolicy = "Fulfill" + // PolicyFail reports a terminal provisioning failure. + PolicyFail ProvisionPolicy = "Fail" + // PolicyIgnore leaves the claim untouched (simulates no provisioner, or a + // very slow one). + PolicyIgnore ProvisionPolicy = "Ignore" +) + +// FakeProvisioner is the test stand-in for the platform/cloud-provider +// controller in the FEP-0001 claim contract (the role Meridian's +// claim reconciler plays for AKS). It only ever writes the fields +// that side of the contract owns: the Completed status condition and +// status.provisionedClusterName — never the claim spec, and never +// lastObservedMostRecentClusterCreationTimestamp. +type FakeProvisioner struct { + client.Client + + mu sync.Mutex + policy ProvisionPolicy + // provisioned records claim name -> MemberCluster name for restart-idempotency. + provisioned map[string]string +} + +func NewFakeProvisioner(c client.Client) *FakeProvisioner { + return &FakeProvisioner{Client: c, policy: PolicyIgnore, provisioned: map[string]string{}} +} + +// SetPolicy switches the provisioning behavior for subsequently observed claims. +func (p *FakeProvisioner) SetPolicy(policy ProvisionPolicy) { + p.mu.Lock() + defer p.mu.Unlock() + p.policy = policy +} + +func (p *FakeProvisioner) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + claim := &placementv1alpha1.ClusterClaim{} + if err := p.Get(ctx, req.NamespacedName, claim); err != nil { + // A vanished claim is a normal outcome in this contract: KubeFleet may + // withdraw at any moment, regardless of provisioning state. + return ctrl.Result{}, client.IgnoreNotFound(err) + } + if meta.FindStatusCondition(claim.Status.Conditions, placementv1alpha1.ClusterClaimCondTypeCompleted) != nil { + return ctrl.Result{}, nil + } + + p.mu.Lock() + policy := p.policy + p.mu.Unlock() + + switch policy { + case PolicyFulfill: + return ctrl.Result{}, p.fulfill(ctx, claim) + case PolicyFail: + meta.SetStatusCondition(&claim.Status.Conditions, metav1.Condition{ + Type: placementv1alpha1.ClusterClaimCondTypeCompleted, + Status: metav1.ConditionFalse, + Reason: "Failed", + Message: "spike: simulated terminal provisioning failure", + ObservedGeneration: claim.Generation, + }) + return ctrl.Result{}, p.Status().Update(ctx, claim) + default: + return ctrl.Result{}, nil + } +} + +func (p *FakeProvisioner) fulfill(ctx context.Context, claim *placementv1alpha1.ClusterClaim) error { + p.mu.Lock() + clusterName, ok := p.provisioned[claim.Name] + if !ok { + clusterName = fmt.Sprintf("provisioned-%s", claim.Name) + p.provisioned[claim.Name] = clusterName + } + p.mu.Unlock() + + memberCluster := &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName, + Labels: labelsForClaim(claim), + }, + Spec: clusterv1beta1.MemberClusterSpec{ + Identity: rbacv1.Subject{ + Kind: "ServiceAccount", + Name: clusterName, + Namespace: "fleet-system", + APIGroup: "", + }, + }, + } + if err := p.Create(ctx, memberCluster); err != nil { + if !errors.IsAlreadyExists(err) { + return err + } + // A cluster already holding the deterministic name is not automatically this claim's + // fulfillment: it may be stale or belong to something else entirely, and reporting + // Completed for a cluster that does not satisfy the claim would hand the contract a + // fulfillment that never happened. Only a cluster the claim's own terms accept counts. + existing := &clusterv1beta1.MemberCluster{} + if getErr := p.Get(ctx, client.ObjectKey{Name: clusterName}, existing); getErr != nil { + return getErr + } + if !clusterMatchesTerms(existing, claim.Spec.ClusterSelectorTerms) { + return fmt.Errorf("member cluster %s exists but does not satisfy claim %s; refusing to report completion", clusterName, claim.Name) + } + } + + claim.Status.ProvisionedClusterName = &clusterName + meta.SetStatusCondition(&claim.Status.Conditions, metav1.Condition{ + Type: placementv1alpha1.ClusterClaimCondTypeCompleted, + Status: metav1.ConditionTrue, + Reason: "Provisioned", + Message: fmt.Sprintf("spike: member cluster %s provisioned", clusterName), + ObservedGeneration: claim.Generation, + }) + return p.Status().Update(ctx, claim) +} + +// labelsForClaim derives the provisioned cluster's labels from the claim, the +// way a real provisioner would feed selector terms into a cluster blueprint. +func labelsForClaim(claim *placementv1alpha1.ClusterClaim) map[string]string { + if len(claim.Spec.ClusterSelectorTerms) == 0 { + return nil + } + out := map[string]string{} + maps.Copy(out, claim.Spec.ClusterSelectorTerms[0].MatchLabels) + return out +} + +func (p *FakeProvisioner) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + Named("spike-fake-provisioner"). + For(&placementv1alpha1.ClusterClaim{}). + Complete(p) +} diff --git a/test/spike/clusterclaim/lifecycle_test.go b/test/spike/clusterclaim/lifecycle_test.go new file mode 100644 index 000000000..a57283356 --- /dev/null +++ b/test/spike/clusterclaim/lifecycle_test.go @@ -0,0 +1,213 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" +) + +const spikeProvisionerFinalizer = "spike.kubefleet.dev/provisioner" + +// Lifecycle gaps in the claim contract: what happens around deletion, +// ownership, and naming is unspecified by FEP-0001. These specs demonstrate +// the current behavior empirically so the contract discussion has concrete +// anchors. +var _ = Describe("claim lifecycle gaps", Ordered, func() { + var counter int + + newClaimFor := func(policyName, policyNamespace, region string) *placementv1alpha1.ClusterClaim { + counter++ + return &placementv1alpha1.ClusterClaim{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("lc-claim-%d", counter)}, + Spec: placementv1alpha1.ClusterClaimSpec{ + PlacementPolicyRef: &placementv1alpha1.ObjectReference{ + Name: policyName, + Namespace: policyNamespace, + APIGroup: "placement.kubefleet.dev", + APIVersion: "v1alpha1", + Kind: "PlacementPolicy", + }, + ClusterSelectorTerms: []placementv1alpha1.ClusterLabelAndPropertySelectorTerm{ + {MatchLabels: map[string]string{"topology.kubernetes.io/region": region}}, + }, + }, + } + } + + newPolicy := func(name, namespace string) *placementv1alpha1.PlacementPolicy { + return &placementv1alpha1.PlacementPolicy{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: placementv1alpha1.PlacementPolicySpec{ + ResourceSelectors: []placementv1alpha1.ResourceSelector{ + {APIVersion: "v1", Kind: "ConfigMap", Name: "demo"}, + }, + }, + } + } + + BeforeAll(func() { + for _, ns := range []string{"tenant-a", "tenant-b"} { + Expect(k8sClient.Create(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: ns}})).Should(Succeed()) + } + }) + + AfterEach(func() { + mcList := &clusterv1beta1.MemberClusterList{} + Expect(k8sClient.List(ctx, mcList)).Should(Succeed()) + for i := range mcList.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &mcList.Items[i]))).Should(Succeed()) + } + Eventually(func() (int, error) { + if err := k8sClient.List(ctx, mcList); err != nil { + return -1, err + } + return len(mcList.Items), nil + }, eventuallyTimeout, eventuallyInterval).Should(Equal(0)) + }) + + // FINDING (spike round 2): nothing in the contract forbids a provisioner + // finalizer, and with one held, "withdrawal" only marks the claim + // Terminating. The FEP never says whether a Terminating claim still counts + // toward the concurrency budget (starvation) or is replaced + // (double-provisioning) — one of the two must be chosen by #786. + It("withdrawal against a provisioner finalizer only marks the claim Terminating", func() { + claim := newClaimFor("app", "tenant-a", "eastus") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("the provisioner protects its in-flight work with a finalizer") + Expect(k8sClient.Get(ctx, clientKey(claim.Name), claim)).Should(Succeed()) + claim.Finalizers = append(claim.Finalizers, spikeProvisionerFinalizer) + Expect(k8sClient.Update(ctx, claim)).Should(Succeed()) + + By("a matching cluster appears and the withdrawer deletes the claim") + mc := &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "manual-eastus", Labels: map[string]string{"topology.kubernetes.io/region": "eastus"}}, + Spec: clusterv1beta1.MemberClusterSpec{ + Identity: rbacv1.Subject{Kind: "ServiceAccount", Name: "manual-eastus", Namespace: "fleet-system"}, + }, + } + Expect(k8sClient.Create(ctx, mc)).Should(Succeed()) + + By("the claim lingers in Terminating instead of disappearing") + Eventually(func() bool { + fetched := &placementv1alpha1.ClusterClaim{} + if err := k8sClient.Get(ctx, clientKey(claim.Name), fetched); err != nil { + return false + } + return !fetched.DeletionTimestamp.IsZero() + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + Consistently(func() error { + return k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + }, time.Second*2, eventuallyInterval).Should(Succeed()) + + By("releasing the finalizer completes the withdrawal") + fetched := &placementv1alpha1.ClusterClaim{} + Expect(k8sClient.Get(ctx, clientKey(claim.Name), fetched)).Should(Succeed()) + fetched.Finalizers = nil + Expect(k8sClient.Update(ctx, fetched)).Should(Succeed()) + Eventually(func() bool { + err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + }) + + // FINDING (spike round 2): deleting the owning PlacementPolicy leaves the + // claim behind with nothing to ever clean it up — cross-scope + // ownerReferences (namespaced owner, cluster-scoped dependent) are invalid + // in Kubernetes GC, and the FEP specifies no controller-side cleanup. + // placementPolicyRef also records no UID, so a recreated same-named policy + // silently "inherits" the orphan. + It("KNOWN GAP: deleting the referenced PlacementPolicy orphans the claim", func() { + policy := newPolicy("app", "tenant-a") + Expect(k8sClient.Create(ctx, policy)).Should(Succeed()) + claim := newClaimFor(policy.Name, policy.Namespace, "northeurope") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, claim))).Should(Succeed()) }) + + By("the policy is deleted") + Expect(k8sClient.Delete(ctx, policy)).Should(Succeed()) + Eventually(func() bool { + err := k8sClient.Get(ctx, types.NamespacedName{Name: policy.Name, Namespace: policy.Namespace}, &placementv1alpha1.PlacementPolicy{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + + By("the claim survives indefinitely — nothing reconciles it away") + Consistently(func() error { + return k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + }, time.Second*2, eventuallyInterval).Should(Succeed()) + }) + + // The API server accepts a cross-scope ownerReference without complaint; + // the invalidity only surfaces later, in the garbage collector (which + // reports OwnerRefInvalidNamespace and skips the object — envtest runs no + // GC, so that half is documented rather than asserted). The takeaway for + // #786: ownerReferences are not a usable cleanup mechanism here, however + // tempting the API server makes it look. + It("cross-scope ownerReference to a namespaced policy is accepted by the API server (footgun)", func() { + policy := newPolicy("owner-demo", "tenant-a") + Expect(k8sClient.Create(ctx, policy)).Should(Succeed()) + DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, policy))).Should(Succeed()) }) + + claim := newClaimFor(policy.Name, policy.Namespace, "uksouth") + claim.OwnerReferences = []metav1.OwnerReference{{ + APIVersion: "placement.kubefleet.dev/v1alpha1", + Kind: "PlacementPolicy", + Name: policy.Name, + UID: policy.UID, + }} + Expect(k8sClient.Create(ctx, claim)).Should(Succeed(), "accepted at admission; invalid only at GC time") + Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) + }) + + // FINDING (spike round 2): claims are cluster-scoped, policies are + // namespaced, and the FEP defines no naming scheme. Any deterministic + // name derived from the policy name alone collides across namespaces. + It("KNOWN GAP: deterministic claim names derived from policy names collide across namespaces", func() { + policyA := newPolicy("app", "tenant-b") + Expect(k8sClient.Create(ctx, policyA)).Should(Succeed()) + DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, policyA))).Should(Succeed()) }) + // tenant-a's policy "app" may already exist from earlier specs; ensure it does. + policyB := newPolicy("app", "tenant-a") + if err := k8sClient.Create(ctx, policyB); err != nil { + Expect(errors.IsAlreadyExists(err)).Should(BeTrue()) + } + + claimName := "claim-for-app" + claimA := newClaimFor("app", "tenant-b", "eastasia") + claimA.Name = claimName + Expect(k8sClient.Create(ctx, claimA)).Should(Succeed()) + DeferCleanup(func() { Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, claimA))).Should(Succeed()) }) + + claimB := newClaimFor("app", "tenant-a", "westeurope") + claimB.Name = claimName + err := k8sClient.Create(ctx, claimB) + Expect(errors.IsAlreadyExists(err)).Should(BeTrue(), "same policy name in another namespace collides on the flat cluster-scoped claim namespace") + }) +}) diff --git a/test/spike/clusterclaim/matcher.go b/test/spike/clusterclaim/matcher.go new file mode 100644 index 000000000..b387bcaf6 --- /dev/null +++ b/test/spike/clusterclaim/matcher.go @@ -0,0 +1,86 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" +) + +// clusterMatchesTerms reports whether a member cluster satisfies any of the +// given selector terms (terms are ORed; requirements within a term are ANDed), +// mirroring the semantics FEP-0001 assigns to ClusterClaim +// spec.clusterSelectorTerms. Per the FEP, no terms means any cluster matches. +// +// Spike scope: label matching only (MatchLabels + MatchLabelExpressions). +// MatchClusterPropertyExpressions needs the cluster-property provider, which +// is out of scope here; a term carrying property expressions matches nothing. +func clusterMatchesTerms(cluster *clusterv1beta1.MemberCluster, terms []placementv1alpha1.ClusterLabelAndPropertySelectorTerm) bool { + if len(terms) == 0 { + return true + } + for i := range terms { + if clusterMatchesTerm(cluster, &terms[i]) { + return true + } + } + return false +} + +func clusterMatchesTerm(cluster *clusterv1beta1.MemberCluster, term *placementv1alpha1.ClusterLabelAndPropertySelectorTerm) bool { + if len(term.MatchClusterPropertyExpressions) > 0 { + return false + } + + sel := &metav1.LabelSelector{MatchLabels: term.MatchLabels} + for _, expr := range term.MatchLabelExpressions { + op, ok := labelSelectorOp(expr.Operator) + if !ok { + // Numeric operators (Gt, Lt, ...) apply to cluster properties only. + return false + } + sel.MatchExpressions = append(sel.MatchExpressions, metav1.LabelSelectorRequirement{ + Key: expr.Key, + Operator: op, + Values: expr.Values, + }) + } + + s, err := metav1.LabelSelectorAsSelector(sel) + if err != nil { + return false + } + return s.Matches(labels.Set(cluster.Labels)) +} + +func labelSelectorOp(op placementv1alpha1.LabelClusterPropertyExpressionOperator) (metav1.LabelSelectorOperator, bool) { + switch op { + case placementv1alpha1.LabelClusterPropertyExpressionOperatorIn: + return metav1.LabelSelectorOpIn, true + case placementv1alpha1.LabelClusterPropertyExpressionOperatorNotIn: + return metav1.LabelSelectorOpNotIn, true + case placementv1alpha1.LabelClusterPropertyExpressionOperatorExists: + return metav1.LabelSelectorOpExists, true + case placementv1alpha1.LabelClusterPropertyExpressionOperatorDoesNotExist: + return metav1.LabelSelectorOpDoesNotExist, true + default: + return "", false + } +} diff --git a/test/spike/clusterclaim/suite_test.go b/test/spike/clusterclaim/suite_test.go new file mode 100644 index 000000000..df73d064f --- /dev/null +++ b/test/spike/clusterclaim/suite_test.go @@ -0,0 +1,97 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "context" + "path/filepath" + "testing" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + "sigs.k8s.io/controller-runtime/pkg/manager" + "sigs.k8s.io/controller-runtime/pkg/metrics/server" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" +) + +var ( + cfg *rest.Config + mgr manager.Manager + k8sClient client.Client + testEnv *envtest.Environment + ctx context.Context + cancel context.CancelFunc + fakeProvisioner *FakeProvisioner + withdrawer *Withdrawer +) + +func TestClusterClaimSpike(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "ClusterClaim Workflow Spike Suite") +} + +var _ = BeforeSuite(func() { + ctx, cancel = context.WithCancel(context.TODO()) + + By("bootstrapping test environment") + testEnv = &envtest.Environment{ + CRDDirectoryPaths: []string{filepath.Join("../../../", "config", "crd", "bases")}, + ErrorIfCRDPathMissing: true, + } + + var err error + cfg, err = testEnv.Start() + Expect(err).Should(Succeed()) + Expect(cfg).NotTo(BeNil()) + + Expect(placementv1alpha1.AddToScheme(scheme.Scheme)).Should(Succeed()) + Expect(clusterv1beta1.AddToScheme(scheme.Scheme)).Should(Succeed()) + + k8sClient, err = client.New(cfg, client.Options{Scheme: scheme.Scheme}) + Expect(err).Should(Succeed()) + Expect(k8sClient).NotTo(BeNil()) + + By("starting the manager with the fake provisioner and the withdrawer prototype") + mgr, err = ctrl.NewManager(cfg, ctrl.Options{ + Scheme: scheme.Scheme, + Metrics: server.Options{BindAddress: "0"}, + }) + Expect(err).Should(Succeed()) + + fakeProvisioner = NewFakeProvisioner(mgr.GetClient()) + Expect(fakeProvisioner.SetupWithManager(mgr)).Should(Succeed()) + withdrawer = NewWithdrawer(mgr.GetClient()) + Expect(withdrawer.SetupWithManager(mgr)).Should(Succeed()) + + go func() { + defer GinkgoRecover() + Expect(mgr.Start(ctx)).Should(Succeed()) + }() +}) + +var _ = AfterSuite(func() { + cancel() + By("tearing down the test environment") + Expect(testEnv.Stop()).Should(Succeed()) +}) diff --git a/test/spike/clusterclaim/validation_test.go b/test/spike/clusterclaim/validation_test.go new file mode 100644 index 000000000..f004e958e --- /dev/null +++ b/test/spike/clusterclaim/validation_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "fmt" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" +) + +// These specs pin the CEL/schema contract of the claim API (issue #791, +// coordinating with #793 for the broader CEL test scope). +var _ = Describe("ClusterClaim CEL and schema validation", func() { + var counter int + + newClaim := func(terms []placementv1alpha1.ClusterLabelAndPropertySelectorTerm) *placementv1alpha1.ClusterClaim { + counter++ + return &placementv1alpha1.ClusterClaim{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("cel-claim-%d", counter)}, + Spec: placementv1alpha1.ClusterClaimSpec{ + PlacementPolicyRef: &placementv1alpha1.ObjectReference{ + Name: "app", + Namespace: "work", + APIGroup: "placement.kubefleet.dev", + APIVersion: "v1alpha1", + Kind: "PlacementPolicy", + }, + ClusterSelectorTerms: terms, + }, + } + } + + regionTerm := func(region string) placementv1alpha1.ClusterLabelAndPropertySelectorTerm { + return placementv1alpha1.ClusterLabelAndPropertySelectorTerm{ + MatchLabels: map[string]string{"topology.kubernetes.io/region": region}, + } + } + + It("rejects creation without placementPolicyRef", func() { + claim := newClaim(nil) + claim.Spec.PlacementPolicyRef = nil + Expect(k8sClient.Create(ctx, claim)).ShouldNot(Succeed()) + }) + + It("rejects mutation of placementPolicyRef (CEL immutability)", func() { + claim := newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("eastus")}) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + + claim.Spec.PlacementPolicyRef.Name = "other" + err := k8sClient.Update(ctx, claim) + Expect(err).Should(HaveOccurred()) + Expect(err.Error()).Should(ContainSubstring("immutable")) + }) + + It("rejects mutation of clusterSelectorTerms, including reordering (CEL immutability)", func() { + claim := newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("eastus"), regionTerm("westus")}) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + + By("appending a term") + mutated := claim.DeepCopy() + mutated.Spec.ClusterSelectorTerms = append(mutated.Spec.ClusterSelectorTerms, regionTerm("centralus")) + Expect(k8sClient.Update(ctx, mutated)).ShouldNot(Succeed()) + + By("reordering terms — CEL list equality is order-sensitive") + mutated = claim.DeepCopy() + mutated.Spec.ClusterSelectorTerms = []placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("westus"), regionTerm("eastus")} + Expect(k8sClient.Update(ctx, mutated)).ShouldNot(Succeed()) + }) + + // FINDING (spike, 2026-08-12): the field-level CEL rule `self == oldSelf` + // on spec.clusterSelectorTerms does NOT fire when the field is removed — + // Kubernetes skips field-scoped transition rules when the field is absent + // from the new object. Unsetting the terms flips the claim's meaning to + // "any cluster satisfies this claim", silently widening it. The guard must + // be duplicated at the spec level, e.g. + // `has(self.clusterSelectorTerms) == has(oldSelf.clusterSelectorTerms)`. + // This spec pins the CURRENT (buggy) behavior so the suite stays green; + // flip the assertion once the API is fixed (candidate: PR #803). + It("KNOWN GAP: dropping clusterSelectorTerms entirely bypasses CEL immutability", func() { + claim := newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("eastus")}) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + + claim.Spec.ClusterSelectorTerms = nil + Expect(k8sClient.Update(ctx, claim)).Should(Succeed(), "currently accepted — should be rejected once the spec-level guard exists") + }) + + // FINDING (spike round 2): the removal bypass has a mirror image — a claim + // created WITHOUT terms ("any cluster satisfies") can have terms ADDED + // later, silently narrowing it. Same root cause as the removal case: + // field-scoped transition rules are skipped when the field is absent on + // either side of the update. Note `[]` serializes as absent via omitempty, + // so an empty-list create also leaves the claim mutable. + It("KNOWN GAP: adding clusterSelectorTerms after creation bypasses CEL immutability", func() { + claim := newClaim(nil) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + + claim.Spec.ClusterSelectorTerms = []placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("eastus")} + Expect(k8sClient.Update(ctx, claim)).Should(Succeed(), "currently accepted — should be rejected once the spec-level guard exists") + }) + + // FINDING (spike round 2): nothing ties the Completed condition to its + // evidence. A provisioner can report Completed=True with no + // provisionedClusterName, and can later downgrade or repoint a completed + // claim — the status subresource accepts all of it. Status atomicity and + // terminal-state pinning need CEL rules (Completed=True requires + // provisionedClusterName; no downgrade out of a terminal state; + // provisionedClusterName immutable once set). + It("KNOWN GAP: status accepts Completed=True without provisionedClusterName, and downgrades", func() { + claim := newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("australiaeast")}) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + + By("Completed=True lands with no provisioned cluster recorded") + claim.Status.Conditions = []metav1.Condition{{ + Type: placementv1alpha1.ClusterClaimCondTypeCompleted, + Status: metav1.ConditionTrue, + Reason: "Provisioned", + Message: "spike: no provisionedClusterName set", + LastTransitionTime: metav1.Now(), + }} + Expect(k8sClient.Status().Update(ctx, claim)).Should(Succeed(), "currently accepted — atomicity CEL would reject this") + + By("the terminal state downgrades back to in-progress") + claim.Status.Conditions[0].Status = metav1.ConditionFalse + claim.Status.Conditions[0].Reason = "Provisioning" + Expect(k8sClient.Status().Update(ctx, claim)).Should(Succeed(), "currently accepted — terminal-state pinning would reject this") + }) + + It("accepts a claim with no selector terms (any cluster satisfies)", func() { + claim := newClaim(nil) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + }) + + It("enforces expression operator/values arity rules", func() { + By("In with empty values is rejected") + claim := newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{{ + MatchLabelExpressions: []placementv1alpha1.LabelClusterPropertyExpression{{ + Key: "env", + Operator: placementv1alpha1.LabelClusterPropertyExpressionOperatorIn, + }}, + }}) + Expect(k8sClient.Create(ctx, claim)).ShouldNot(Succeed()) + + By("Exists with values is rejected") + claim = newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{{ + MatchLabelExpressions: []placementv1alpha1.LabelClusterPropertyExpression{{ + Key: "env", + Operator: placementv1alpha1.LabelClusterPropertyExpressionOperatorExists, + Values: []string{"staging"}, + }}, + }}) + Expect(k8sClient.Create(ctx, claim)).ShouldNot(Succeed()) + }) + + It("keeps spec.generation at 1 for the object's whole lifetime (immutable spec invariant)", func() { + claim := newClaim([]placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("eastus")}) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + DeferCleanup(func() { Expect(k8sClient.Delete(ctx, claim)).Should(Succeed()) }) + + mutated := claim.DeepCopy() + mutated.Spec.ClusterSelectorTerms = []placementv1alpha1.ClusterLabelAndPropertySelectorTerm{regionTerm("westus")} + Expect(k8sClient.Update(ctx, mutated)).ShouldNot(Succeed()) + + fetched := &placementv1alpha1.ClusterClaim{} + Expect(k8sClient.Get(ctx, clientKey(claim.Name), fetched)).Should(Succeed()) + Expect(fetched.Generation).Should(Equal(int64(1))) + }) +}) diff --git a/test/spike/clusterclaim/withdrawer.go b/test/spike/clusterclaim/withdrawer.go new file mode 100644 index 000000000..6fbc92361 --- /dev/null +++ b/test/spike/clusterclaim/withdrawer.go @@ -0,0 +1,140 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "context" + "sync" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" + "github.com/kubefleet-dev/kubefleet/pkg/scheduler/clustereligibilitychecker" +) + +// Withdrawer prototypes the claim-management slice of the FEP-0001 placement +// policy controller (#786): per the FEP, KubeFleet evaluates outstanding +// claims whenever the member cluster set changes, withdraws (deletes) a claim +// as soon as its selector terms are satisfied by ANY member cluster — +// regardless of the claim's own Completed status — and otherwise refreshes +// status.lastObservedMostRecentClusterCreationTimestamp so provisioners can +// tell the claim has been re-evaluated and is still wanted. +// +// The FEP says a claim is withdrawn once a matching cluster "is joined to the +// fleet" without saying whether "joined" means the MemberCluster object exists +// or the cluster is actually usable by the scheduler. The two readings are +// prototyped side by side, toggled by SetEligibilityGate: +// +// - gate off (default, FEP-as-written reading): withdraw on raw selector +// match against the MemberCluster object; +// - gate on (recommended reading): additionally require the cluster to pass +// the scheduler's real eligibility gate +// (clustereligibilitychecker.IsEligible: member agent online, recent +// heartbeat, Joined=True, healthy) before the match counts. +type Withdrawer struct { + client.Client + + checker *clustereligibilitychecker.ClusterEligibilityChecker + + mu sync.Mutex + eligibilityGate bool +} + +func NewWithdrawer(c client.Client) *Withdrawer { + return &Withdrawer{Client: c, checker: clustereligibilitychecker.New()} +} + +// SetEligibilityGate toggles between the FEP-as-written withdrawal predicate +// (raw selector match, gate off) and the eligibility-gated variant (gate on). +func (w *Withdrawer) SetEligibilityGate(on bool) { + w.mu.Lock() + defer w.mu.Unlock() + w.eligibilityGate = on +} + +func (w *Withdrawer) gated() bool { + w.mu.Lock() + defer w.mu.Unlock() + return w.eligibilityGate +} + +func (w *Withdrawer) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + claim := &placementv1alpha1.ClusterClaim{} + if err := w.Get(ctx, req.NamespacedName, claim); err != nil { + return ctrl.Result{}, client.IgnoreNotFound(err) + } + + memberClusters := &clusterv1beta1.MemberClusterList{} + if err := w.List(ctx, memberClusters); err != nil { + return ctrl.Result{}, err + } + + var mostRecent metav1.Time + for i := range memberClusters.Items { + mc := &memberClusters.Items[i] + if clusterMatchesTerms(mc, claim.Spec.ClusterSelectorTerms) { + eligible := true + if w.gated() { + eligible, _ = w.checker.IsEligible(mc) + } + if eligible { + // Withdraw: the need is met, whether by the provisioned cluster or + // any other cluster that joined or was relabeled. + return ctrl.Result{}, client.IgnoreNotFound(w.Delete(ctx, claim)) + } + } + if mc.CreationTimestamp.After(mostRecent.Time) { + mostRecent = mc.CreationTimestamp + } + } + + // Still unfulfilled: refresh the freshness marker if the fleet has moved on. + ts := claim.Status.LastObservedMostRecentClusterCreationTimestamp + if !mostRecent.IsZero() && (ts == nil || mostRecent.After(ts.Time)) { + claim.Status.LastObservedMostRecentClusterCreationTimestamp = &mostRecent + return ctrl.Result{}, w.Status().Update(ctx, claim) + } + return ctrl.Result{}, nil +} + +func (w *Withdrawer) SetupWithManager(mgr ctrl.Manager) error { + // Re-evaluate every claim when any MemberCluster changes; this is the + // "evaluates unfulfilled cluster selectors as soon as a new cluster is + // joined ... or an existing cluster has been relabeled" behavior. + mapAllClaims := handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, _ client.Object) []reconcile.Request { + claims := &placementv1alpha1.ClusterClaimList{} + if err := w.List(ctx, claims); err != nil { + return nil + } + reqs := make([]reconcile.Request, 0, len(claims.Items)) + for i := range claims.Items { + reqs = append(reqs, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(&claims.Items[i])}) + } + return reqs + }) + + return ctrl.NewControllerManagedBy(mgr). + Named("spike-claim-withdrawer"). + For(&placementv1alpha1.ClusterClaim{}). + Watches(&clusterv1beta1.MemberCluster{}, mapAllClaims). + Complete(w) +} diff --git a/test/spike/clusterclaim/workflow_test.go b/test/spike/clusterclaim/workflow_test.go new file mode 100644 index 000000000..77bd59830 --- /dev/null +++ b/test/spike/clusterclaim/workflow_test.go @@ -0,0 +1,254 @@ +/* +Copyright 2026 The KubeFleet Authors. + +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. +*/ + +package clusterclaim + +import ( + "fmt" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + rbacv1 "k8s.io/api/rbac/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + + clusterv1beta1 "github.com/kubefleet-dev/kubefleet/apis/cluster/v1beta1" + placementv1alpha1 "github.com/kubefleet-dev/kubefleet/apis/kubefleet.dev/placement/v1alpha1" +) + +const ( + eventuallyTimeout = time.Second * 10 + eventuallyInterval = time.Millisecond * 250 + // consistentlyDuration is how long a negative assertion watches for the thing that must not + // happen; long enough for several reconcile rounds, short enough not to dominate the suite. + consistentlyDuration = time.Second * 2 +) + +func clientKey(name string) types.NamespacedName { + return types.NamespacedName{Name: name} +} + +// The behavioral contract of the claim workflow, per FEP-0001 "Cluster +// requests". Each spec here corresponds to a named scenario in the #791 +// verification matrix. +var _ = Describe("cluster claim workflow", Ordered, func() { + var counter int + + newClaim := func(region string) *placementv1alpha1.ClusterClaim { + counter++ + return &placementv1alpha1.ClusterClaim{ + ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("wf-claim-%d", counter)}, + Spec: placementv1alpha1.ClusterClaimSpec{ + PlacementPolicyRef: &placementv1alpha1.ObjectReference{ + Name: "app", + Namespace: "work", + APIGroup: "placement.kubefleet.dev", + APIVersion: "v1alpha1", + Kind: "PlacementPolicy", + }, + ClusterSelectorTerms: []placementv1alpha1.ClusterLabelAndPropertySelectorTerm{ + {MatchLabels: map[string]string{"topology.kubernetes.io/region": region}}, + }, + }, + } + } + + newMemberCluster := func(name string, labels map[string]string) *clusterv1beta1.MemberCluster { + return &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{Name: name, Labels: labels}, + Spec: clusterv1beta1.MemberClusterSpec{ + Identity: rbacv1.Subject{ + Kind: "ServiceAccount", + Name: name, + Namespace: "fleet-system", + }, + }, + } + } + + AfterEach(func() { + // The gate is spike-process state, not cluster state: reset it whatever the spec did, or + // a failure between set and reset would silently change every later spec's semantics. + withdrawer.SetEligibilityGate(false) + claimList := &placementv1alpha1.ClusterClaimList{} + Expect(k8sClient.List(ctx, claimList)).Should(Succeed()) + for i := range claimList.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &claimList.Items[i]))).Should(Succeed()) + } + fakeProvisioner.SetPolicy(PolicyIgnore) + // Scrub member clusters between scenarios so fulfillment state does not leak. + mcList := &clusterv1beta1.MemberClusterList{} + Expect(k8sClient.List(ctx, mcList)).Should(Succeed()) + for i := range mcList.Items { + Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, &mcList.Items[i]))).Should(Succeed()) + } + Eventually(func() (int, error) { + if err := k8sClient.List(ctx, mcList); err != nil { + return -1, err + } + return len(mcList.Items), nil + }, eventuallyTimeout, eventuallyInterval).Should(Equal(0)) + }) + + It("happy path: provisioner fulfills the claim, then the claim is withdrawn", func() { + fakeProvisioner.SetPolicy(PolicyFulfill) + // The eligibility gate holds withdrawal open: the provisioned cluster has no agent + // heartbeat, so a gated withdrawer does not yet count it. Without the hold, withdrawal + // can race the provisioner's status update, and the test would pass without ever + // proving the provisioner's half of the contract was honored. + withdrawer.SetEligibilityGate(true) + claim := newClaim("eastus") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("the provisioner completes the claim while withdrawal is held") + Eventually(func(g Gomega) { + fulfilled := &placementv1alpha1.ClusterClaim{} + g.Expect(k8sClient.Get(ctx, clientKey(claim.Name), fulfilled)).Should(Succeed()) + g.Expect(fulfilled.Status.ProvisionedClusterName).ShouldNot(BeNil()) + g.Expect(*fulfilled.Status.ProvisionedClusterName).Should(Equal("provisioned-" + claim.Name)) + completed := meta.FindStatusCondition(fulfilled.Status.Conditions, placementv1alpha1.ClusterClaimCondTypeCompleted) + g.Expect(completed).ShouldNot(BeNil()) + g.Expect(completed.Status).Should(Equal(metav1.ConditionTrue)) + }, eventuallyTimeout, eventuallyInterval).Should(Succeed(), "the provisioner must report completion before withdrawal") + + By("the provisioned cluster's member agent reporting in is what withdraws the claim") + // The gate stays on: withdrawal happens because the cluster genuinely becomes eligible, + // the same transition eligibility_test drives, not because the test flips the harness's + // bypass switch. The status update is itself the member cluster event that re-runs the + // withdrawer's evaluation. + mc := &clusterv1beta1.MemberCluster{} + Expect(k8sClient.Get(ctx, clientKey("provisioned-"+claim.Name), mc)).Should(Succeed()) + now := metav1.Now() + mc.Status.AgentStatus = []clusterv1beta1.AgentStatus{{ + Type: clusterv1beta1.MemberAgent, + Conditions: []metav1.Condition{ + {Type: string(clusterv1beta1.AgentJoined), Status: metav1.ConditionTrue, Reason: "AgentJoined", Message: "spike: simulated join", LastTransitionTime: now}, + {Type: string(clusterv1beta1.AgentHealthy), Status: metav1.ConditionTrue, Reason: "AgentHealthy", Message: "spike: simulated health", LastTransitionTime: now}, + }, + LastReceivedHeartbeat: now, + }} + Expect(k8sClient.Status().Update(ctx, mc)).Should(Succeed()) + Eventually(func() bool { + err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue(), "claim should be withdrawn (deleted)") + + By("the provisioned member cluster remains") + Expect(k8sClient.Get(ctx, clientKey("provisioned-"+claim.Name), mc)).Should(Succeed()) + Expect(mc.Labels).Should(HaveKeyWithValue("topology.kubernetes.io/region", "eastus")) + }) + + It("a name collision with a cluster that does not satisfy the claim is not reported as completion", func() { + fakeProvisioner.SetPolicy(PolicyFulfill) + claim := newClaim("northeurope") + // Squat the deterministic name with a cluster whose labels satisfy nothing. + squatter := &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "provisioned-" + claim.Name, + Labels: map[string]string{"unrelated": "squatter"}, + }, + Spec: clusterv1beta1.MemberClusterSpec{ + Identity: rbacv1.Subject{Kind: "ServiceAccount", Name: "squatter", Namespace: "fleet-system"}, + }, + } + Expect(k8sClient.Create(ctx, squatter)).Should(Succeed()) + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("the claim stays uncompleted rather than claiming the squatter as fulfillment") + Consistently(func() bool { + current := &placementv1alpha1.ClusterClaim{} + if err := k8sClient.Get(ctx, clientKey(claim.Name), current); err != nil { + return false + } + return meta.FindStatusCondition(current.Status.Conditions, placementv1alpha1.ClusterClaimCondTypeCompleted) == nil + }, consistentlyDuration, eventuallyInterval).Should(BeTrue(), "a squatted name must not become a completion report") + }) + + It("withdraw-by-other-cluster: an unrelated matching cluster joins while the provisioner never completes", func() { + fakeProvisioner.SetPolicy(PolicyIgnore) + claim := newClaim("westus") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("the claim persists while unfulfilled and unprovisioned") + Consistently(func() error { + return k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + }, consistentlyDuration, eventuallyInterval).Should(Succeed()) + + By("a manually joined cluster satisfies the selector") + Expect(k8sClient.Create(ctx, newMemberCluster("manual-westus", map[string]string{"topology.kubernetes.io/region": "westus"}))).Should(Succeed()) + + By("the claim is withdrawn even though Completed was never set") + Eventually(func() bool { + err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + }) + + It("failure path: Completed=False/Failed leaves the claim outstanding until a matching cluster appears", func() { + fakeProvisioner.SetPolicy(PolicyFail) + claim := newClaim("centralus") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("the provisioner reports terminal failure") + Eventually(func() bool { + fetched := &placementv1alpha1.ClusterClaim{} + if err := k8sClient.Get(ctx, clientKey(claim.Name), fetched); err != nil { + return false + } + cond := meta.FindStatusCondition(fetched.Status.Conditions, placementv1alpha1.ClusterClaimCondTypeCompleted) + return cond != nil && cond.Status == metav1.ConditionFalse && cond.Reason == "Failed" + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + + By("the failed claim is NOT withdrawn — the selector is still unfulfilled (open design question: retry policy)") + Consistently(func() error { + return k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + }, consistentlyDuration, eventuallyInterval).Should(Succeed()) + + By("fulfillment by any cluster still withdraws the failed claim") + Expect(k8sClient.Create(ctx, newMemberCluster("late-centralus", map[string]string{"topology.kubernetes.io/region": "centralus"}))).Should(Succeed()) + Eventually(func() bool { + err := k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + return err != nil && client.IgnoreNotFound(err) == nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + }) + + It("staleness: a non-matching cluster join refreshes the freshness marker instead of withdrawing", func() { + fakeProvisioner.SetPolicy(PolicyIgnore) + claim := newClaim("northeurope") + Expect(k8sClient.Create(ctx, claim)).Should(Succeed()) + + By("a cluster joins that does not satisfy the selector") + Expect(k8sClient.Create(ctx, newMemberCluster("wrong-region", map[string]string{"topology.kubernetes.io/region": "eastus"}))).Should(Succeed()) + + By("the claim survives and lastObservedMostRecentClusterCreationTimestamp is refreshed") + Eventually(func() bool { + fetched := &placementv1alpha1.ClusterClaim{} + if err := k8sClient.Get(ctx, clientKey(claim.Name), fetched); err != nil { + return false + } + return fetched.Status.LastObservedMostRecentClusterCreationTimestamp != nil + }, eventuallyTimeout, eventuallyInterval).Should(BeTrue()) + + Consistently(func() error { + return k8sClient.Get(ctx, clientKey(claim.Name), &placementv1alpha1.ClusterClaim{}) + }, consistentlyDuration, eventuallyInterval).Should(Succeed()) + + Expect(k8sClient.Delete(ctx, &placementv1alpha1.ClusterClaim{ObjectMeta: metav1.ObjectMeta{Name: claim.Name}})).Should(Succeed()) + }) +})