fix: Report accurate health for federated workloads#127
Open
scotwells wants to merge 63 commits into
Open
Conversation
Workloads targeting a city location are now automatically routed to the correct physical site via a Karmada-based federation layer. Each POP cell operates independently, instance health is surfaced back to the control plane in real time, and the platform remains available even when parts of the control plane are temporarily unreachable. Controllers added: - WorkloadDeploymentFederator: replicates WDs into Karmada and manages PropagationPolicies per city code - InstanceProjector: mirrors Instance write-backs from Karmada into the project namespace on the control plane ResourceInterpreterCustomization deployed at config time teaches Karmada how to aggregate replica counts and conditions across POP cells. Operator flags --enable-management-controllers and --enable-cell-controllers allow each deployment to opt into only the controllers it needs. Includes a 6-test Chainsaw e2e suite covering federation, deletion cascade, propagation policy lifecycle, instance projection, instance write-back, and the full end-to-end chain. Resolves #85 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…edge Introduces management-plane and cell overlay paths to the compute OCI artifact so the infra repo can deploy compute-manager in the correct mode for each tier of the federation architecture. The management-plane overlay deploys compute-manager with only WorkloadDeploymentFederator and InstanceProjector enabled, connected to the Karmada downstream control plane via projected ServiceAccount token auth. The cell overlay deploys compute-manager with only WorkloadDeploymentReconciler and InstanceReconciler enabled, with no downstream connection or webhook server. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ts for webhook TLS Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove the hardcoded datum-control-plane ClusterIssuer from the csi-webhook-cert component. DNS names stay since they are fixed by the service name and namespace. Each consuming overlay now supplies the issuer via a strategic merge patch, allowing different environments to use different cert issuers without forking the component. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each WorkloadDeployment is routed to exactly one cell cluster via its PropagationPolicy, so aggregation across multiple members is not needed. Replace the summing logic with a direct pass-through of the single member's status. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The cert issuer name is environment-specific configuration that belongs in the infra repo, not the compute overlay. The infra repo's base manager patch already owns the full webhook-server-tls volume definition including the issuer. Consumers deploying outside infra must patch the issuer in their own overlay. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…moval dev: inline self-signed Issuer + Certificate for host.docker.internal, replace kustomize replacements block with direct annotation patch, remove Certificate-patching from webhook_patch.yaml, and clear webhookServer secretRef from config.yaml. single-cluster: replace cert-manager Certificate approach with the csi-webhook-cert component, matching the main branch overlay. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The WorkloadReconciler watches networkingv1alpha.Network objects, which requires the network-services-operator CRDs to be installed. Cell clusters don't have those CRDs, causing the manager to crash on startup. Gate the WorkloadReconciler behind enableManagementControllers so it only runs where the Network CRDs are present. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracts server config file reading and decoding into a dedicated loadServerConfig helper, reducing main's cyclomatic complexity from 31 to 29 to satisfy the gocyclo linter limit of 30. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Milo's authorization webhook uses Extra claims on the admission request (iam.miloapis.com/parent-name, iam.miloapis.com/parent-type, etc.) to resolve the correct project-scoped policy binding. Dropping them caused the SAR to return Allowed=false even for users with networks.use, because the authorizer couldn't locate the binding without the project context. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
metricRules belongs under spec.quota, not spec.billing. The field is not declared in the ServiceBillingConfig schema, causing Flux dry-run failures in staging with: .spec.billing.metricRules: field not declared in schema
Previously, InstanceReconciler wrote ResourceClaim objects against the local deployment cluster via managementCluster.GetClient(). Those claims were never seen by the Milo quota system, leaving every Instance in QuotaGranted=Unknown indefinitely. This change routes claim creation and deletion to the correct Milo project control plane for each instance using a new ProjectQuotaClientManager that builds per-project REST clients by rewriting the host path — mirroring the URL construction already used by the milomulticluster provider. The management-cluster claim watch is replaced with a multicluster Watches call so that grant/denial status changes in project control planes re-trigger instance reconciles. Claims are stamped with a source-cluster label (discovery.clusterName) so each edge controller only reacts to the claims it created. Co-Authored-By: Claude <claude@anthropic.com>
The admission webhook requires that all metrics referenced in spec.quota.limits[].metric and spec.quota.metricRules[].metricCosts match a name declared in spec.metrics[]. The four quota-tracking metrics (workloads, instances, vcpus, memory) were missing from spec.metrics[], causing the webhook to reject the resource.
…o cell setup Controller flags --enable-management-controllers and --enable-cell-controllers now default to false so kustomize components must explicitly opt in, rather than both groups running by default. This prevented the management-plane deployment from crashing when discovery.clusterName was unset — that field is only required by the InstanceReconciler (a cell controller), so the validation now lives in InstanceReconciler.SetupWithManager instead of initializeClusterDiscovery. Also adds cell-controllers and management-controllers components to the single-cluster overlay, which was silently running with no controllers enabled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…scovery The rebase during cherry-pick propagation introduced a mixed state where cmd/main.go had the edgeClusterName/projectRestConfig return values partially reverted. This cleans up the function signature and call sites to be consistent, while keeping the validation removed from initializeClusterDiscovery (it belongs in InstanceReconciler.SetupWithManager per the original fix intent). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… RBAC The workload-deployment-federator calls ensureDownstreamNamespace before federating WorkloadDeployment resources, but the compute-manager ClusterRole was missing core-group namespace permissions, causing every reconcile to fail with a forbidden error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Workload scheduling and admission now consult LocationBinding objects (project-scoped, created by the service catalog) rather than the global Location list. This ensures consumers only see locations that are both healthy and available to their specific project. Also upgrades network-services-operator and milo dependencies to versions that introduce LocationBinding and address multicluster-runtime v0.23 API changes (ClusterName type, ProviderRunnable Start lifecycle, generic webhook builder). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ources WorkloadDeploymentReconciler creates and owns NetworkBinding and SubnetClaim resources, and watches Location, NetworkContext, and Subnet. InstanceReconciler watches ResourceClaim for quota. Neither was granted the necessary ClusterRole rules, causing watch failures on cell clusters. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
From the cell cluster's perspective, Karmada is upstream (the federation control plane), not downstream. Rename the flag, env var, and related variables throughout to reflect the actual relationship. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…viderRunnable fix Points go.miloapis.com/milo to the feature branch commit that implements multicluster.ProviderRunnable on the Milo provider, enabling the mc manager to auto-call provider.Start() and set p.mcAware so project clusters can be registered. Without this, p.mcAware was always nil and every project reconcile logged "Multicluster manager not yet started" forever. Also removes the & from ResourceRef in ResourceClaimSpec — the feature branch has ResourceRef as a value type, not a pointer. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Remove non-existent QuotaRestConfig() call and fix SetupWithManager argument count; pass nil quota config to skip quota enforcement for now. Single-tenant cell mode uses namespace-as-project-id and the fixed 'single' cluster name. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wires up Milo ResourceClaim-based quota accounting for cells running in single-cell discovery mode (mode: single), where the multicluster ClusterName is always "single" rather than the Milo project name. Key changes: - Add QuotaKubeconfigPath config field and QuotaRestConfig() method so quota REST config can be configured independently of discovery mode. Returns (nil, nil) when neither path is set, disabling quota rather than silently targeting the local apiserver. - Add projectIDForInstance and clusterNameForProject func fields to InstanceReconciler. In single mode, project ID is derived from instance.Namespace; the watch map func always enqueues ClusterName "single" rather than the project namespace, avoiding ErrClusterNotFound on every quota-grant event. - Guard ResourceClaim watch map func against claims with empty ResourceRef to prevent a nil-dereference panic when a label-matching claim from another actor has no ResourceRef set. - Add TestReconcileQuotaSingleMode covering the full single-mode quota flow: project ID from namespace, watch re-enqueue to "single" cluster. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
v2.1.5 was built with Go 1.24 and refuses to lint Go 1.25 modules. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tatus change Write-back was only triggered inside the statusChanged||readyChanged block, so instances stuck in a scheduling gate (no status transitions) were never replicated to Karmada. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nged Use apiequality.Semantic.DeepEqual to avoid unnecessary API calls to Karmada on every reconcile when nothing has actually changed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
From the cell cluster's perspective, Karmada is upstream. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…llers Karmada is upstream from every controller's perspective — cell controllers write instances up to it, management controllers read/write through it. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Client Management controllers (WorkloadDeploymentFederator, InstanceProjector) now refuse to start when --enable-management-controllers is set but --federation-kubeconfig is omitted, logging a clear error and exiting 1. Previously the controllers were silently skipped — the same fail-open-silent class as the quota P1 issue — leaving federation and instance projection broken with no operator-visible signal. Alongside the fail-loud guard, rename the Karmada/federation client identifiers to a neutral "federation" framing (FederationClient, federationRestConfig, --federation-kubeconfig / FEDERATION_KUBECONFIG) across all three controllers, cmd/main.go, and the kustomize base manifests. The previous --upstream-kubeconfig flag is removed; deployments must migrate to --federation-kubeconfig. Update all comments to match. Coordination note: once this artifact is deployed, management-plane and edge/lab deployments must set FEDERATION_KUBECONFIG (infra PRs in parallel). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
fix(mgmt): fail loud on missing federation kubeconfig; rename federation client
…e cells Introduces a --feature-gates=NetworkingIntegration=false flag so operators can run compute on cells where network-services-operator (VPC) is not yet available. When disabled: no NetworkBinding is created, the Network scheduling gate is omitted from new Instances (and removed from existing ones), and the networking step is treated as immediately ready so Instances reach the runtime. Default is true, leaving all production behavior unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat: make the compute networking integration optional
…e manifest Overlays can now enable or disable feature gates (e.g. NetworkingIntegration) with a strategic-merge patch on the FEATURE_GATES env var rather than appending a raw flag string. The base default is empty, which the binary already guards (only calls Set when non-empty), so all existing cells are unaffected. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(config): wire feature gates through FEATURE_GATES env var in base deployment manifest
When the cell plane wrote Instances back to the Karmada control plane, writeBackToUpstream built the downstream object with only the two upstream-owner routing labels, dropping the workload-uid, workload-deployment-uid, and instance-index labels the cell stamps. The management-plane InstanceProjector then had no labels to copy and could not resolve the WorkloadDeployment owner reference, so `datumctl compute instances` rendered WORKLOAD as "orphaned" and CITY as "unknown". Carry the three linking labels onto the downstream object so the projector can propagate them and resolve the owner reference. The update path now merges only the labels this controller owns instead of replacing the whole map, preserving any Karmada-managed labels, and logs a warning when a linking label is missing at write-back. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exposes per-container entrypoint and argument overrides on the
SandboxContainer type, mirroring Kubernetes pod-spec semantics:
- Command []string — overrides the image ENTRYPOINT
- Args []string — overrides the image CMD; combined with
Command when both are set
When neither field is set the image's own ENTRYPOINT/CMD are used
unchanged, which is the correct default for standard OCI images
(e.g. hello-world, nginx). Infrastructure providers that translate
Instance specs (such as unikraft-provider) should map these fields
through to the underlying runtime.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…stant Four occurrences in instance_writeback_test.go triggered goconst because testInstanceType = "d1-standard-2" already exists in the same package. Replacing all four with the constant keeps golangci-lint at 0 issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nd-args feat(api): add Command and Args fields to SandboxContainer
Instances are projected back to the project cluster where the CLI reads them. Previously, correlating an Instance with its workload/deployment required joining on workload-deployment-uid, which differs per Karmada plane (the WD UID in the management cluster does not match the uid assigned in the cell). Add four new label constants and stamp them on every Instance at create and update time: - workload-deployment-name (deployment.Name) - city-code (deployment.Spec.CityCode) - workload-name (deployment.Spec.WorkloadRef.Name) - placement-name (deployment.Spec.PlacementName) These self-describing labels let the CLI resolve WORKLOAD/CITY/placement directly from the projected Instance object, without any cross-plane join. All four labels are included in the writeBackToUpstream allowlist so they propagate through InstanceReconciler → Karmada → InstanceProjector into the user-facing project cluster. Also persist the resolved Location (already discovered during network reconciliation) onto WorkloadDeployment.Status.Location, and propagate it into Instance.Spec.Location best-effort. A nil location never blocks instance creation; the existing scheduling path is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The owner reference on a projected Instance must reference the actual project-control-plane WorkloadDeployment so that GC cascades and deletes projections when the deployment is removed. The previous implementation compared the WorkloadDeploymentUIDLabel value (which carries the edge/Karmada plane WD UID) against project-cluster WD UIDs — a match that never succeeds because each Kubernetes plane mints its own UID for the same object. The result was that ownerWD stayed nil, no ownerReference was set, and Instance projections leaked indefinitely (e.g. my-api/test-workload orphaned after WD deletion). Fix: resolve the owning WD by the federation-stable WorkloadDeployment NAME via a direct projectClient.Get against the project cluster, satisfying the core invariant that the owner reference UID/name/GVK must come from a live project-cluster object. The name is read from the new WorkloadDeploymentNameLabel (already stamped by dd3421a); an ordinal-strip fallback handles Instances created before that label was introduced. If the project WD is NotFound, requeue with RequeueAfter: 5s without creating the projection, so a projection is never created without an owner reference. This handles the transient ordering race where Karmada propagates an Instance back before WorkloadReconciler has created the project WD. Existing ownerless projections self-heal on the next reconcile once the project WD exists. Tests added: - "WD name label present, edge UID differs from project UID": asserts ownerRef.UID == projTestWDUID AND != projTestEdgeWDUID (regression guard against reintroducing cross-plane UID matching). - "WD name label absent, fallback name extraction from instance name": verifies the ordinal-strip path produces a correct owner reference. - "project WD not found — requeue, no ownerless projection created": asserts RequeueAfter > 0 and no projection object exists. - "WD name label absent and instance name yields no resolvable WD": verifies unrecognised instance names are skipped cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Newly-introduced controller labels (city-code, workload-name, workload-deployment-name, placement-name) were only stamped on the create path and the template-hash-mismatch update path. A pre-existing instance that is not-Ready with an unchanged template hash takes the Wait branch and was never re-stamped, so the labels were absent on instances like sre-gate-test-default-dfw-0. Add a dedicated label-backfill pass that runs after the ordered rollout decision and skip-loop. For each existing, non-deleting instance, desiredControllerLabels() computes the full desired label set; if any key differs a NewPatchLabelsAction (ActionTypePatchLabels) is emitted. The action executes via client.MergeFrom patch, which sends only the metadata diff — spec, template, and template-hash are never touched (constraint 1). Backfill actions are appended after the rollout skip-loop so they are never subject to the "skip all but first" rule and never counted as an update in progress (constraint 2). The pass is idempotent: it is a no-op when all labels already match. Fix the misleading comment on addInstanceControllerLabels that overstated coverage by claiming the function ran on "both create and update paths"; the comment now reflects that backfill covers every reconcile pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s update After quota is granted, the Quota scheduling gate was never removed from spec.controller.schedulingGates, leaving instances stuck "Pending (SchedulingGatesPresent)" even though the workload was running. Root cause: Reconcile returned early after writing QuotaGranted=True to status (statusChanged=true path), before reaching removeQuotaSchedulingGate. Because ResourceClaims are immutable after creation and local Instances are not watched (WithEngageWithLocalCluster(false)), no subsequent event would re-enqueue the instance — the gate was stranded forever. Fix: on the success path (quotaErr==nil), fall through to removeQuotaSchedulingGate after persisting the status update rather than returning early. Only return early with quotaErr when it is non-nil, which preserves the transient-failure backoff-requeue behavior. Also updates existing tests that previously required two reconciles to clear the gate (the second of which could never arrive in production), and adds TestQuotaGateRemovedInSingleReconcile as a regression test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WorkloadDeployment reconciliation read instance.Status.Controller.ObservedTemplateHash to count current replicas without checking that Status.Controller (a nilable pointer) was populated. Infra providers set that field independently of the Programmed condition, so an instance could report Programmed=True with Status.Controller still nil. The dereference panicked, and because it ran before the status write, the deployment's status froze at its create-time values and the reconcile hot-looped -- surfacing as a permanently Unavailable workload with 0 ready replicas even though its instances were running. Guard the dereference so reconciliation completes and the deployment reports accurate readiness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Instance projections copied the workload-deployment-uid label verbatim from the cell-plane Instance, where it carries the cell/Karmada WorkloadDeployment UID. That value never matches the project-plane WorkloadDeployment UID, so label-selector lookups in the project cluster (e.g. the CLI CITY column) failed to resolve. Overwrite the label on the projection with the owning project-side WorkloadDeployment's UID, which the projector already resolves for the owner reference. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Federated WorkloadDeployment status was aggregated onto the Karmada hub object but never propagated back to the project-namespace deployment that users and the Workload controller read. The federator only watched the project-side object, so its downstream status sync never fired reactively, leaving the project deployment's status empty and the parent Workload stuck at 0/0 replicas. Add a WorkloadDeploymentStatusSyncer on the management plane that watches Karmada WorkloadDeployments and writes their aggregated status back to the originating project deployment, mirroring how the InstanceProjector resolves the project cluster and namespace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ecv
approved these changes
May 30, 2026
ecv
left a comment
ecv
left a comment
There was a problem hiding this comment.
Do we need these comments? Genuine question, you know what I think but that's not what's important necessarily.
The comments added earlier in this branch narrated the change and the bug rather than the rationale a future reader needs. Trim each to a single why-focused line. Add a "Code comments" convention to CLAUDE.md: explain why not what, stay concise, and describe the code as-is rather than the change that produced it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drop the step-by-step what-the-code-does comments in the status syncer and the redundant comments at its wiring/federator call sites, keeping only the why (label stamping, cluster-name encoding, name as cross-plane key, Update-vs-Patch). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scotwells
force-pushed
the
feat/federated-deployment-scheduling
branch
7 times, most recently
from
b45810f to
73177eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Federated workloads were reported as
Unavailablewith0/0ready replicas even when all of their instances were running —datumctl compute workloadsshowed every workload unhealthy whiledatumctl compute instancesshowed themRunning. This makes federated workload health and readiness reflect reality across the CLI, API, andWorkloadstatus.Three independent defects in the status pipeline (Instance → WorkloadDeployment → Workload, spanning the cell, Karmada, and project planes) combined to hide healthy instances:
Status.Controller, a nilable pointer) while counting replicas. Infra providers set that field independently of theProgrammedcondition, so a programmed-but-not-yet-populated instance crashed the reconcile before it wrote status — freezing the deployment at0ready replicas and hot-looping. Now guarded.WorkloadDeploymentStatusSynceron the management plane watches hub deployments and writes their status back to the originating project deployment.workload-deployment-uidlabel, which never matches the project-plane UID, breaking label-selector lookups (the CLI CITY column). Projections now stamp the project-side UID.With all three fixed, a running federated instance flows readiness up the full chain: the cell deployment counts it, Karmada aggregates it, the syncer carries it to the project deployment, and the
WorkloadreportsAvailable/1/1.Test plan
make test(envtest) passes, including the updated instance-projector test asserting the project-side UID labelreconcileInstanceGatescounts readiness without panicking when an instance isProgrammed=TruewithStatus.Controller == nilAvailable/1/1indatumctl compute workloadsdatumctl compute instancesresolves the CITY column for projected instancesNotes for reviewers
WorkloadDeploymentStatusSyncerruns only under--enable-management-controllersand must be rolled out to the management-plane controller for project-side status to populate.Programmed=Truebefore populatingStatus.Controller(the Unikraft provider today). A complementary provider-side PR reports the template hash socurrentReplicastracks rollouts.This PR is stacked on
feat/federated-deployment-scheduling(#107) and targets that branch, since the changes depend on federation-only code not yet inmain.🤖 Generated with Claude Code