Repository navigation
chore: backport kubefleet main (2026-10-12) - #1372
Open
Britania Rodriguez Reyes (britaniar) wants to merge 21 commits into
Open
Britania Rodriguez Reyes (britaniar) wants to merge 21 commits into
Britania Rodriguez Reyes (britaniar) wants to merge 21 commits into
Conversation
* feat: [FEP-0001] parse the place-to annotation into cluster selectors Adds the parser behind annotation-based placement: it turns the value of the kubefleet.dev/place-to annotation into the cluster selectors of a placement policy. The controller that keeps a policy object in sync with the annotation follows separately. The grammar is the one FEP-0001 specifies: a semicolon-separated list of selectors, each a comma-separated list of LABEL_KEY=LABEL_VALUE matchers with an optional count=N|All directive that defaults to 1, and with `region` and `alias` accepted as shorthands for the well-known region label and the reserved cluster alias label. Every key and value is validated against the rules Kubernetes itself enforces, and the parser's own limits are held to the ones the placement policy CRD declares, so a mistaken annotation is reported against the annotation rather than surfacing later as a policy object the API server rejects and a reconciler that retries it forever. Where the FEP is silent the parser errs toward rejecting the input: duplicate label keys (including a shorthand colliding with the key it expands to), a repeated count directive, and an empty label value are all errors rather than silently resolved. A selector carrying only a count directive is allowed, so `count=All` on its own selects the whole fleet. The count ceiling is enforced client-side only. The API's pattern marker constrains the string form of the field alone, so the API server today accepts an integer count both above the ceiling and below one; this is noted on the constant and belongs with the other CEL gaps in the placement policy API. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: [FEP-0001] derive the placement policy generated from an annotation Computes what the placement policy for an annotated resource should look like: its name, the labels recording where it came from, its owner reference, and the resource selector that places the annotated resource itself. The reconciler that creates, updates and deletes it follows separately. The policy's scope follows the annotated resource's own, so the owner reference is always same-scope: a cluster-scoped object owned by a namespaced one is accepted on creation and then never collected at all. The reference claims neither controller nor blocking ownership, since setting either requires permission to update the finalizers subresource of the owner, and the owner here can be a resource of any kind. Names and label values are derived through a new pkg/utils/naming, added rather than written twice: the placement policy controller of Azure#786 has its own private copies of the same helpers, with the same semantics, and can move onto this package when it merges. Two bugs have already been fixed once in those copies, and a second copy is how they come back. Every part of an identity that can outgrow where it is put is shortened through that package, including the API group, which is validated as a DNS-1123 subdomain and so runs to four times what a label value holds. The generated name is shortened the same way and finished with a hash of the whole, untruncated identity, so that resources which differ only past the truncation point, or only in the case of their kind, cannot converge on one policy. Fields the API server defaults are set explicitly. Left unset they are stored with the default and then read as a difference on the next pass, which would have the reconciler issue an update that changes nothing for as long as the policy exists. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * refactor: [FEP-0001] name the placement annotation after what it holds The annotation key was introduced as a placeholder. Rename it to kubefleet.dev/cluster-selectors, which names the value rather than the action taken on it and matches the clusterSelectors field of the policy generated from it, so that the annotation and the field it stands in for do not have to be translated between when read together. Renames the constant to ClusterSelectorsAnnotation and the parser entry point to parseClusterSelectors to follow. No behavior changes. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: [FEP-0001] reconcile the placement policy generated from an annotation Adds the reconciler that keeps a generated placement policy in step with the cluster-selectors annotation on a resource: it creates the policy when the annotation appears, updates it when the annotation changes, and deletes it when the annotation is removed. A resource that is itself deleted is deliberately left alone. The generated policy carries an owner reference back to it, so garbage collection removes the policy without this controller racing the collector to delete an object that is already doomed. An annotation the parser rejects is reported to the user as an event on the annotated resource and the key is dropped rather than retried: no number of retries makes a malformed annotation parse. A policy generated from an earlier, valid annotation is left standing, since tearing down a running placement is a worse answer to a typo than keeping the last placement the user did express. Only what the controller generates is overwritten on an existing policy -- the spec, the provenance labels, and its own owner reference. Labels and owner references from elsewhere are preserved so that a generated policy can be labelled by an operator or a GitOps tool without the two taking turns undoing each other. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: [FEP-0001] run annotation-based placement in the hub agent Wires the annotation placement controller into the resource watcher, behind --enable-annotation-based-placement. The flag defaults to off: the feature reads and writes the placement.kubefleet.dev/v1alpha1 policies, whose definitions this chart does not install yet, so the hub agent checks they exist and refuses to start without them rather than retrying against a kind the cluster does not have. The controller shares the resource watcher's informers but is fed only the resources that carry the annotation, which keeps every unrelated object in the cluster out of its queue. An update is enqueued when either the old or the new object carries the annotation: looking at the new one alone would filter out the removal of an annotation, which is precisely the event that has to delete the generated policy. A deleted resource is not enqueued at all, since the policy is garbage collected through its owner reference. The controller gets a rate limiter of its own rather than the one the placement controllers share. An exponential failure limiter keys its backoff on the queued item alone, and this controller queues the same cluster wide keys as the resource change controller, so one limiter between them would let a success here clear the backoff that repeated failures there had earned. Grants the hub agent write access to the generated policies; the broad rule it otherwise relies on is read-only. No status subresource rule: the controller never writes status. Adds an envtest suite covering the round trip against a real API server -- annotation set, changed, made invalid, and removed -- for both a namespaced and a cluster-scoped resource. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] repair generated policy drift and close the cleanup gaps The controller synchronized in one direction only: it acted on events about the annotated resources, so anything that happened to a generated policy directly went unseen. Deleting a policy, or editing its spec, produced no event on the resource it came from, and for a resource that never changes again the policy stayed missing or wrong forever -- a resync does not step in, since updates whose resource version did not move are dropped. The resource watcher now watches the generated policies themselves and, for every policy event, enqueues the resources the policy names as its owners, so the next reconciliation restores whatever the event changed. The owner references are used rather than the parent labels, which are lossy and, being labels, can be stripped -- itself drift this path exists to repair. The placement.kubefleet.dev group joins the resource config's default skip list along the way: everything in it is KubeFleet's own bookkeeping, and treating the generated policies as placeable resources would let a placement that selects a namespace propagate them. A deleted resource now has its generated policy deleted explicitly rather than left to garbage collection. The collector removes a dependent only once every owner reference on it is gone, and the merge deliberately preserves owner references that other parties add -- any live one of which would have kept the policy standing indefinitely. A resource that stops passing the resource watcher's filter without being deleted -- a ReplicaSet adopted by a Deployment, for instance -- is reported by the watcher as a deletion and then never seen again. The reconciler therefore applies the same eligibility test itself and deletes the generated policy of a live but ineligible resource, that being its last chance to clean up. A key whose kind the API server does not know, which the policy watch can produce by enqueuing whatever a policy names as its owner, is dropped rather than retried: no retry makes the kind exist, and the key would otherwise back off on the queue forever. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] do not report a deletion another pass already performed Deleting a generated policy re-enters the reconciler through the generated policy watch, and the second pass's cached read can still see the object the first pass just deleted. The delete it then issues comes back not found, which was being swallowed into a success: one real deletion produced two "deleted" log lines, and on the annotation-removal path would have produced two events. Surfaced by exercising the controller on a kind cluster rather than by any test. A delete that finds nothing now reports that this pass deleted nothing, matching what the read-side already did when the policy was gone before the pass began. Also settles the whenUnfulfilled question the policy derivation had flagged as open: generated selectors keep the API's RequestCluster default deliberately, since asking the platform for a cluster that does not exist yet is the point of these APIs and the annotation is meant to be the lowest-friction way to do so. Guarding against a selector that nothing can ever satisfy belongs to the claim fulfillment layer. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] bound the integer form of a selector count The count of a cluster selector is an int-or-string whose only bound was a pattern, and a pattern constrains the string form alone: the API server rejected count: "1000" while accepting count: 1000, along with zero and negative integers the field's own documentation rules out. A CEL rule now bounds the integer form to the same 1-999 range the pattern holds the string form to. The parser had been holding this line by itself for annotation-derived counts; hand-authored policies had no line at all. The bounds are pinned twice: envtest submits both forms of both verdicts to a real API server, and the parser's limits test asserts the CRD's rule and message name the same ceiling the parser enforces, without binding either to an exact spelling. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] keep the minCount rule out of problems that are not its own The minCount<=count rule guarded its string comparison with an unbounded digit match, so a count too long for an int64 reached the int() conversion and turned a plain validation failure into an opaque evaluation error beside it. The guard now recognizes only the numeric strings of at most the three digits the count field itself permits; anything else -- "All", or junk the field's own validation rejects -- passes the rule vacuously, so the real problem is reported alone, and the message drops a qualifier about a case the rule no longer treats specially. Count additionally gains a MaxLength matching its longest legal values, which rejects an over-long string outright. The digit guard is spelled [0-9]{1,3} rather than mirroring the count pattern's own numeric branch verbatim: the CEL cost estimator prices a regex by its length against an unbounded string, an int-or-string being opaque to the sibling MaxLength, and the longer spelling does not fit the budget. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: bound the integer form of the rolling update settings The maxUnavailable and maxSurge fields of a rolling update strategy are int-or-strings whose only bound was a pattern, and a pattern constrains the string form alone: the API server rejected maxUnavailable: "-1" while accepting maxUnavailable: -1, a value the field's documentation rules out and the rollout controller cannot act on. A CEL rule now keeps the integer form non-negative in both the v1 and v1beta1 APIs. For ClusterResourcePlacement the validating webhook already rejected negative values, so this closes a gap only where the webhook is not running. ResourcePlacement has no working webhook behind it at all, so there the schema is the only line of defense; the new tests cover both kinds for that reason. Note that these are served API versions: a manifest carrying a negative integer bound, previously accepted and stored, is rejected on its next update that changes the value. Unchanged stored values are ratcheted through, since the field sits on a plain struct path that validation correlates across updates. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: judge the object a tombstone wraps rather than the tombstone A deletion the watch missed arrives as a tombstone wrapping the object's final state, and the resource watcher's filter derived its key from the tombstone itself, which is not a runtime object. Every relist-detected deletion was therefore dropped before any handler saw it, even though the delete handler has always known how to unwrap one. The filter now unwraps the tombstone before judging the object, and the test that pinned the dropped-deletion behavior as intended now pins the opposite. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: [FEP-0001] seed the cluster alias label on member cluster join FEP-0001 reserves the kubefleet.dev/cluster-alias label for selecting member clusters by name, and its own examples select by alias -- yet nothing has ever set the label, so every alias selector matched no clusters at all. With annotation-based placement enabled, the member cluster controller now seeds the alias from the cluster name on join, and only when the label is absent entirely. Unlike the member name label beside it, which states a fact the controller owns and reasserts, the alias exists to be renamed: it is the indirection that lets a selector follow the cluster playing a role rather than a fixed name, so a value an admin changed is never reverted. Fleets that do not run the feature are left untouched. The guard rail that optionally denies member cluster label changes exempted only the kubernetes-fleet.io prefix; it now exempts the kubefleet.dev prefix as well. The hub agent is not in system:masters, and denying it this label would have failed the same update that maintains the member name label, wedging the cluster's reconciliation. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: bound the digit form of the rolling update settings The digit branch of the rolling update pattern accepted a string of any length, which passed validation only to fail in whatever consumed it later. It is now bounded to nine digits, all of which fit an int32 comfortably, in both the v1 and v1beta1 APIs; the same ratcheting considerations apply as for the sign bound before it. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] name the generated kind in the controller's events An event about a ClusterPlacementPolicy called it a placement policy, so pasting the name from the event into kubectl against that kind found nothing. The created, updated, and deleted events now name the kind verbatim. The kind is asked of the same function that decides the scope, keeping that decision in the one place its contract promises, and the deleted event's doc comment now covers both of its causes. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: [FEP-0001] warn on a duplicate cluster alias The cluster alias label is seeded on member cluster join and selects a cluster by a name of the admin's choosing, so it is meant to identify one cluster; two clusters sharing an alias makes an alias-based selector match both. The member cluster validating webhook now warns when the alias being set is already held by another cluster. It is a warning, never a denial: labelling a replacement cluster with the outgoing one's alias before removing it from the outgoing one is exactly the handoff the alias exists to allow, and that state must stay legal while transient. For the same reason a failure to list the member clusters admits the request without a warning rather than blocking it -- the webhook fails closed, so an advisory check that errored on a listing hiccup would wedge every member cluster write. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] follow only the generating owner and skip excluded namespaces Two correctness gaps found in review of the generated-policy watch and the placement eligibility check. The watch re-enqueued every owner reference a policy carried, so a foreign owner -- one the merge preserves, or any owner on a policy that merely shares these informers -- was treated as a generating source. If its kind was one the resource watcher does not track, reading it lazily created an informer outside the resource configuration. Only the owner whose identity reproduces the policy's own generated name is followed now; matching a 64-bit identity hash admits nothing else. The eligibility check read metadata.namespace to decide whether a resource's namespace is skipped, but a Namespace object carries none and is itself the namespace, so an annotated "default" or "kube-system" generated a policy for a namespace KubeFleet excludes. The namespace is now taken from the object's own name for a Namespace, matching how the resource change controller already special-cases the kind. Also clarifies, per review, that the event reasons are the same for both generated kinds, that eligibility can change over a resource's life and is re-checked on the events that change it, and where the placement CRDs come from until a chart installs them. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] restrict the kubefleet.dev/ label exemption to fleet controllers The guard rail that optionally denies member cluster label changes exempted the whole kubefleet.dev/ prefix for every user. Unlike the member-name label the controller reasserts each reconcile, the cluster alias is left as it is set, so a non-admin's edit to it would persist and redirect alias-based placements to another cluster. The prefix is now exempt only for a whitelisted identity (the hub agent, which seeds the alias) or a cluster admin; ordinary users are denied. The kubernetes-fleet.io/ prefix stays exempt for everyone as before, and admins can still rename an alias for a cluster handoff, since they are exempted before this check runs. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * refactor: [FEP-0001] compute the generated policy name once on the delete path deletePolicy derived the generated policy name and deleteGeneratedPolicy derived it again for the same policy -- two SHA-256 hashes of the same identity per deletion. deleteGeneratedPolicy now returns the name it computed, and deletePolicy uses it. No behavior change. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] harden the generated placement policy against review feedback Address review findings on the annotation-based placement controller: - Sanitize characters legal in a resource's own name but not in a Kubernetes object name (an RBAC name like system:aggregate-to-admin, or a dot beside another separator) before they reach a generated policy's name, which the API server would otherwise reject and hot-loop the reconciler. The uniqueness hash is still taken over the full, unsanitized identity. - Leave a policy this controller did not generate untouched: before updating or deleting the policy at a resource's deterministic name, verify it carries this controller's provenance labels, so a hand-authored policy colliding with that name is neither overwritten nor deleted. The conflict is surfaced to the user through an event. - Clean up a stale generated policy when its source's API is gone: a source of a config-excluded API stops being placed, and a key whose kind the API server no longer knows has its generated policy deleted rather than left behind. A key naming a version retired while the kind lives on falls back to the served version instead of being treated as a gone kind. - Converge the owner reference across a source deleted and recreated under the same name: match the existing reference on the source's group, kind, and name rather than its UID, so the one reference is updated in place instead of a stale one accumulating every cycle. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] keep a generated policy repairable and correctly scoped Two further review findings on the annotation-based placement controller: - A generated policy was recognized as this controller's own only when all of its provenance labels matched, and that check gates both the repair path, which restores drifted labels, and the delete path. Editing one label on a generated policy therefore made it neither repairable nor removable, so its placement could keep running after the annotation was removed. Recognize ownership from either marker this controller stamps -- the owner reference to the source or the provenance labels -- so a single drifted marker no longer strands the policy, while a foreign policy that merely collides with the name still carries neither and is left untouched. - The source's scope was read from the queued key's group-version-kind, which the informer manager keys on the exact version. When a served version is retired, or the version-agnostic mapping fallback resolves a different one than the key named, a cluster-scoped source was read through a namespaced lookup, missed, and taken for deleted, deleting and recreating its policy. Read the scope from the resolved REST mapping instead. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] read generated policies uncached and requeue on name conflict Two further review findings on the annotation-based placement controller: - Generated policies are watched through a different informer than the controller's cache read from, so an edit or deletion the watch delivered first could be met with a stale object: the pass saw no change and returned, leaving drift until the next resync and, for a deletion -- which a later event or resync never re-delivers, the object being gone from the watch's own cache -- leaving the placement removed indefinitely. Read the policy through an uncached reader, current as of the moment the watch fired; writes still go through the cached client. The queue holds only annotated resources and generated-policy owners, so the read is not on a hot path. - When a policy occupies a resource's generated name but was authored by someone else, the reconciler left it untouched and returned without asking to be run again. Removing that policy fires no event that reaches the source, since it carries no owner reference to it, and the annotation does not change, so the requested policy would never be created once the conflict was cleared. Requeue the source while the conflict stands so the placement is created as soon as the name is free. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] guard the generated policy delete against a replacement race deleteGeneratedPolicy read the policy through the uncached reader and confirmed it was one this controller generated, then deleted it by name with no precondition. A delete targeting the name alone removes whatever occupies it, so a policy replaced between the read and the delete -- deleted and a hand-authored one created at the same name, or overwritten in place with its provenance stripped -- would be deleted even though it was no longer this controller's. Carry the resource version the read returned as a delete precondition, so the delete removes only the exact object this pass verified. A replacement moves the resource version, turning the delete into a conflict that requeues; the next pass reads the current object and, finding it foreign, declines it. The resource version guards both a delete-and-recreate and an in-place overwrite, where a UID precondition would miss the latter. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * chore: [FEP-0001] adopt the ClusterClaim rename from Azure#803 The placement policy API renamed cluster requests to cluster claims; the generated policy's default WhenUnfulfilled option follows suit (RequestCluster -> AddClusterClaim). Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * chore: [FEP-0001] leave annotation-based placement unwired in the hub agent Land the annotation-based placement controller as dormant code, per the deferred-integration ask on the review: the experience is not usable until the rest of FEP-0001 is in place, and the flag, chart values, and hub agent setup that connected the controller would reach every existing installation through backports and release cuts for no benefit. Removed, to return with the integration change once the chain works end to end: the --enable-annotation-based-placement flag and its chart value, RBAC and README entries; the controller construction and CRD check in the hub agent's workload setup; the scheme registration and alias-seeding option in main; and the resource watcher hooks that fed annotated resources and generated-policy events into the controller's queue, with their test. Kept: the controller package with its unit and envtest coverage, the API constants and validation, the naming helpers, and the tombstone unwrap fix in the resource watcher's filter, which stands on its own. Doc comments that described the removed wiring as present now describe the contract the integration change has to meet instead. The kind e2e suite that drove the feature through a live hub agent needs the flag to run and is parked on parked/fep0001-annotation-placement-e2e. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * refactor: [FEP-0001] port annotation-based placement to controller-runtime Move the controller to pkg/v1/controllers, where the rest of the new placement experience lives, and rebuild it on controller-runtime rather than the in-house queue framework, per review. The controller reconciles resources of any kind the hub agent watches, so it runs as a generic controller over a request that carries the kind alongside the namespaced name. The annotated resources reach it through a channel source that the resource watcher will feed once the feature is wired in; the generated policies are watched through the manager's cache, with an event on one mapped back to the resource it came from, so that an edit or a deletion of a generated policy is repaired even when the resource never changes again. Annotated resources are read straight from the API server: they can be of a kind no cache has seen, and a cached read would start an informer for it -- for a kind that cannot be listed or watched, one that never syncs. The generated policies are read through the manager's cache instead, which the policy watch is served from as well. The create-or-update path, the owner reference, and the ownership check use controllerutil, and errors are categorised with the errors package. A policy that occupies a resource's generated name but was authored elsewhere is now reported as an error so that the queue backs off exponentially, instead of being re-examined at a fixed interval forever. The explicit, resource-version-guarded delete stays: garbage collection cannot cover a policy whose annotation was removed or whose resource stopped being eligible, and cannot override an owner reference another party added that the merge preserves. The envtest suite now also runs the controller under a manager, fed through the channel, to prove the watch on the generated policies. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * refactor: [FEP-0001] tidy the annotation placement reconciler Merge the not-found and no-match branches of Reconcile: both delete the generated policy and neither requeues, so one branch does. Replace the hand-rolled digit check plus ParseInt in parseCount with ParseUint, which already rejects signs, whitespace, and exponents; the out-of-range and not-an-integer error families are preserved through strconv.ErrRange. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: seed the cluster alias label on member clusters Add the kubefleet.dev/cluster-alias label, seeded from the cluster name by the member cluster controller when it is absent and never reasserted, so an admin can move an alias between clusters and a selector can follow a role rather than a fixed name. The member cluster webhook warns, but does not deny, when another cluster already holds the alias: two holders is the normal middle state of a handoff. The member cluster label guard treats the kubefleet.dev/ prefix like kubernetes-fleet.io/: the hub agent writes both and is not in system:masters, so without this its own seeding update would be denied wherever denyModifyMemberClusterLabels is on. Placement policies and future APIs select on this label, so it lands on its own rather than behind the annotation-based placement work. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * chore: [FEP-0001] move the rolling update bounds and tombstone fixes to their own PRs Both are v1beta1 fixes for existing users and do not depend on this PR: the rolling update CEL and pattern bounds are Azure#898, the tombstone unwrap in the resource watcher filter is Azure#899. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * refactor: [FEP-0001] read generated policy specs through the API accessor Azure#827 gives both policy kinds a GetSpec accessor, so the package's private policySpec switch, its nil branch, and the test that exercised the unreachable non-policy case go away. A generatedPolicy interface names what emptyPolicyForScope and desiredPolicy return: a client.Object with the accessor, so callers reach the spec without a type switch. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * feat: seed the cluster alias label on member clusters Add the kubefleet.dev/cluster-alias label, seeded from the cluster name by the member cluster controller when it is absent and never reasserted, so an admin can move an alias between clusters and a selector can follow a role rather than a fixed name. The member cluster webhook warns, but does not deny, when another cluster already holds the alias: two holders is the normal middle state of a handoff. The member cluster label guard treats the kubefleet.dev/ prefix like kubernetes-fleet.io/: the hub agent writes both and is not in system:masters, so without this its own seeding update would be denied wherever denyModifyMemberClusterLabels is on. Placement policies and future APIs select on this label, so it lands on its own rather than behind the annotation-based placement work. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] let only repairable drift through the generated policy watch The policy watches admitted every resourceVersion change, and a status write bumps it, so once the scheduling controllers write policy status each of their updates would have cost an uncached read of the annotated resource to learn that nothing drifted. The watch now admits creations, deletions, and updates to the generation, labels, or owner references, which is exactly what applyDesiredPolicy repairs. Also from review: the generatedPolicy interface duplicated Azure#827's accessor, which already embeds client.Object, so the accessor is used directly; the defaults test reads the spec through it instead of a type assertion; the manager suite waits for Start to return so its assertion lands inside the spec tree; the unparsable owner apiVersion branch gains a case; and the unused ClusterScoped field on the shared fake lister, left over from the split-out watcher change, is restored to main. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * fix: [FEP-0001] feed annotated resources through a queue, not a channel Addresses review on the annotation placement controller. - Replace the channel source with a SourceQueue whose Add writes straight into the controller's own workqueue. The resource watcher has a fleet's events to deliver and no business blocking on this controller, which a full channel would have made it do; the queue also collapses repeated reports of one resource into a single reconcile. Resources reported before the manager starts are held, deduplicated, and flushed on Start. - Stop reading a resource under a version it was not reported under. A no-match now distinguishes a retired version from a gone kind: the kind still places, so its generated policy is kept and the request dropped, since API discovery re-reports the resource under the served version as a fresh request. Only a gone kind takes its policy with it. Falling back would have retargeted placement at a version the member clusters may not serve. - Categorize the parser's errors as user errors, keeping the message text intact because it is what the event on the annotated resource shows. - Record events through k8s.io/client-go/tools/events, which every other controller in the repo now uses; this was the last tools/record holdout, and its events would have landed in a different API group from the rest of the hub agent's. - Note at the resource selector that dependent resources are not selected, tracked in Azure#927. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> --------- Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com>
…#939) ExtractIndex and ParsePolicyIndexFromLabel rejected a non-integer and a negative index in one branch, formatting both with ": %w" and the error from strconv.Atoi. For a negative index that error is nil, so the message ended in a formatting artifact: invalid policy index "-1", error: %!w(<nil>) Handle the two cases separately. A non-integer still wraps the strconv error; a negative index now says: invalid policy index "-1": must be a non-negative integer The existing tests covered a negative index but only checked that an error came back. The new ones assert the message, and fail without the change on the negative-index cases. ParsePolicyIndexFromLabel had no tests; they cover valid indexes on cluster-scoped and namespaced snapshots, missing labels, a missing index label, a non-integer and a negative index. pkg/utils/labels goes from 46.7% to 100% coverage. Signed-off-by: Akshita kumari <110122283+akshita317@users.noreply.github.com>
* feat: [FEP-0001] add work applier (1/) (Azure#911) Signed-off-by: Simon Waight <simon.waight@microsoft.com> * Revise security vulnerability reporting instructions Updated the reporting process for security vulnerabilities to direct users to the KubeFleet Security Advisories feature. Signed-off-by: Simon Waight <simon.waight@microsoft.com> --------- Signed-off-by: Simon Waight <simon.waight@microsoft.com> Co-authored-by: Chen Yu <chenyu1@microsoft.com>
* fix: surface unknown eviction conditions in draincluster The draincluster command waits for each eviction to reach a terminal state but then only handled True/False Valid and Executed conditions. A missing or Unknown condition fell through and was reported as a successful drain, masking an uncertain eviction (addressing the pre-existing TODO in draincluster.go). Add safeguards so a missing or Unknown Valid or Executed condition marks the drain unsuccessful and logs that the drain could not be confirmed, instead of silently proceeding. Fixes Azure#659 Signed-off-by: Akshita <110122283+akshita317@users.noreply.github.com> * test: extract evaluateEviction helper and add table-driven tests Address review feedback by extracting the eviction Valid/Executed condition evaluation into a pure evaluateEviction helper, and cover the True/False/Unknown/missing combinations (including the invalid-but-drained reasons) with table-driven unit tests. No behavior change. Signed-off-by: Akshita <110122283+akshita317@users.noreply.github.com> * fix: handle unexpected and non-True eviction statuses defensively Address review feedback: - add an explicit evictionResultExecuted case and a default to the drain switch, so an unexpected classification is treated as unconfirmed rather than a successful drain - evaluateEviction now treats only an explicit True/False status as definitive; Unknown, empty, or unexpected statuses map to indeterminate - add a test case for an empty/invalid Executed status Signed-off-by: Akshita <110122283+akshita317@users.noreply.github.com> * fix: match invalid eviction detail against condition Message markEvictionInvalid sets Reason to the generic ClusterResourcePlacementEvictionInvalidReason and stores the specific detail in Message, so comparing Reason against the EvictionInvalid*Message constants never matched and the invalid-but-drained branch was unreachable. Compare against Message instead, and log Message rather than the generic Reason when reporting an invalid eviction that still drained. Signed-off-by: Akshita <110122283+akshita317@users.noreply.github.com> --------- Signed-off-by: Akshita <110122283+akshita317@users.noreply.github.com> Co-authored-by: Yetkin Timocin <ytimocin@microsoft.com> Co-authored-by: Simon Waight <simon.waight@gmail.com>
* fix: preserve update run transition timestamp Use the selected condition's LastTransitionTime for the UpdateRun status metric so reconciles and hub-agent restarts do not restamp stale statuses. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> * test: align update run metric expectations by gauge Keep the shared metrics test helper sorted by gauge value. For UpdateRun status metrics, normalize expected gauge values from the gathered metrics before comparing so the existing gauge-based ordering remains authoritative after the gauge changed from emission time to persisted LastTransitionTime. Add a label tie-breaker only for equal gauge values in this UpdateRun-specific comparison. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> * test: derive expected UpdateRun gauge values from persisted condition time Remove the alignExpectedMetricGaugeValues workaround and its local label-key/gauge-primary comparator introduced on this branch. The generate*Metric test helpers now derive their expected gauge value directly from updateRun.Status.Conditions[...].LastTransitionTime via a new conditionTransitionTimeSeconds helper (re-fetching updateRun and waiting for the condition to be persisted), matching the exact formula production code uses (float64(cond.LastTransitionTime.UnixNano()) / float64(time.Second)) instead of a time.Now() placeholder. validateUpdateRunMetricsEmitted goes back to using the shared metricsutils.MetricsCmpOptions directly, with no per-package workaround. Since gauges now reflect second-precision persisted timestamps, two distinct condition transitions can legitimately tie on gauge value, so pairing metrics by sorting on gauge value is not reliable. The shared comparator in test/utils/metrics/metrics.go now sorts by a stable label-identity key instead, which guarantees correct got/want pairing regardless of gauge value ties. The Gauge comparer itself keeps its existing permissive (value > 0) check, so this does not change behavior for other consumers of the shared file (e.g. the placement package), which still uses time.Now()-based expected values. Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * test: compare update run metrics with transition times Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> * test: use new expressions for UpdateRun metric pointers Adopt Go 1.26 pointer initialization for the new status metric gauge expectations.\n\nCo-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> --------- Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
* fix: preserve placement status metric timestamps Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> * fix: address placement metric review comments Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> * test: stabilize placement metric comparisons Clarify that synthetic Completed derives its timestamp from the final real condition and compare placement metric series by stable labels when timestamps tie. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> --------- Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…zure#813) * feat: gate delete stage with configured tasks Co-authored-by: ytimocin <5220939+ytimocin@users.noreply.github.com> * fix: avoid reevaluating delete gates after start Co-authored-by: ytimocin <5220939+ytimocin@users.noreply.github.com> * fix: stop safely while delete stage is gated Co-authored-by: ytimocin <5220939+ytimocin@users.noreply.github.com> * chore: complete delete stage task breadcrumb Co-authored-by: ytimocin <5220939+ytimocin@users.noreply.github.com> * feat: support before-stage tasks on the staged update delete stage Let an update strategy hold the delete stage of a staged update run until a wait time passes, an approval request is approved, or both: deleteStage: beforeStageTasks: - type: TimedWait waitTime: 30m - type: Approval The delete stage is configured as a stage, so that its tasks, their statuses and the approval request use the same names as the ones of the update stages. The approval request is named <updateRun>-before-delete-stage. The tasks run concurrently and all of them must complete before any binding is deleted. They only gate the start of the deletion, and they are skipped when there is nothing to delete. An update run that is stopped while it waits for the tasks stops without deleting a binding. A TimedWait task that has elapsed stays elapsed, in the update stages too; stopping and resuming an update run no longer restarts the wait. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> * refactor: align the delete stage tasks with the update stages - Limit the before-stage tasks of the delete stage to a single Approval task, the same as the update stages. A wait before the removal is already possible with a TimedWait task in the after-stage tasks of the last update stage, as the delete stage only starts once those complete. The delete stage now reuses checkBeforeStageTasksStatus, and the TimedWait code path of the delete stage is removed. - Keep a TimedWait after-stage task elapsed once its wait time has passed, as a stop and a resume of the update run move the latest transition of the stage that the wait time is measured from. - Share the code that checks the clusters of the delete stage against the bindings between the execution and the stopping of the delete stage. All the bindings are now checked before one is deleted. - Return early in handleStageApprovalTask instead of nesting. - Build the statuses of the stage tasks with one helper. - Read the conditions of the delete stage with the stage condition types. - Say BeforeStageTaskType, not AfterStageTaskType, in the validation message of an Approval task with a wait time in the before-stage tasks of an update stage. - Fix the kubebuilder marker typo on ResourceSnapshotIndexUsed, and typos in the comments, the messages and the test names. Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> --------- Signed-off-by: Yetkin Timocin <ytimocin@microsoft.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ytimocin <5220939+ytimocin@users.noreply.github.com> Co-authored-by: Yetkin Timocin <ytimocin@microsoft.com>
* Initial plan * fix: update distroless base image for OpenSSL CVEs Co-authored-by: britaniar <145056127+britaniar@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: britaniar <145056127+britaniar@users.noreply.github.com>
…inor adjustments) (Azure#943)
* Initial plan * fix: remediate Go vulnerabilities Co-authored-by: britaniar <145056127+britaniar@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: britaniar <145056127+britaniar@users.noreply.github.com>
…-cncf Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 task done
Yetkin Timocin (ytimocin)
approved these changes
Oct 13, 2026
This branch has not been deployed
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
Backport latest changes from CNCF kubefleet-dev/kubefleet:main into Azure/fleet:main.
Key updates and resolutions:
I have:
How has this code been tested
Special notes for your reviewer