[DisaggregatedSet][2/N] Add controller implementation with rolling update support (#836)
* [DisaggregatedSet][2/N] Add controller implementation Add the full DisaggregatedSet controller implementation, migrated from the standalone disaggregatedset module into the main LWS repository. Components added: - Reconciler with stateless rolling update detection - Rolling update executor with coordinated drain and newest-first drain ordering for multi-revision scenarios (A→B→C) - Stateless linear interpolation planner for N-role rolling updates - LeaderWorkerSet manager for workload CRUD operations - Service manager for headless portless endpoint discovery - Utility functions for revision computation, labels, and naming Key behaviors: - Coordinated drain prevents orphan workloads across roles - Newest-first drain ordering ensures broken intermediate revisions drain before stable originals - maxSurge=0 correctly uses maxUnavailable for batch sizing - Stability gate blocks progress until new workload is fully ready Signed-off-by: Mathis Felardos <mathis@mistral.ai> * [DisaggregatedSet][2/N] Add unit and integration tests Add comprehensive test coverage for the DisaggregatedSet controller, migrated from the standalone module with import path and API version updates (v1alpha1 -> v1, LeaderWorkerSetTemplateSpec wrapping). Test files: - executor_test.go: drain ordering, coordinated drain, A→B→C scenarios, scaleUp/scaleDown operations, role change detection - planner_test.go: rolling update step computation, linear interpolation, surge/unavailable constraints, orphan prevention, N-role support - workload_manager_test.go: LWS CRUD, initial-replicas annotation, revision computation, metadata merging - service_manager_test.go: headless service lifecycle, cross-role readiness coordination, drained service cleanup - utils_test.go: label/naming utilities, owner reference management - disaggregatedset_controller_test.go: integration tests with envtest Signed-off-by: Mathis Felardos <mathis@mistral.ai> * [DisaggregatedSet][2/N] Add e2e tests Add end-to-end tests for DisaggregatedSet, migrated from the standalone module. Tests run on Kind clusters and cover: - Operator deployment and webhook readiness - Basic deployment with LWS resource creation - Rolling update with coordinated drain - Scale up/down without rolling update - Service creation (headless portless) with EndpointSlice verification - Labels and annotations propagation to LWS and pods - LWS CR metadata (Kueue queue-name, exclusive-topology) - Garbage collection on deletion - Rolling update step tracking with surge/unavailable validation - N-role (3 roles) rolling updates - Role add/remove/rename with progressive rollout - Mid-rollout A→B→C newest-first drain ordering Test utilities migrated to test/testutils/disaggregatedset/: - fixtures: DisaggregatedSet YAML generation - kubectl: fluent kubectl builder pattern with queries and waiters Updated namespace and deployment references to lws-system since the DisaggregatedSet controller is now integrated into the LWS operator. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * [DisaggregatedSet][2/N] Fix CRD validation and e2e test compatibility - Remove +required marker on inline-embedded LeaderWorkerSetTemplateSpec which caused kubebuilder to generate required: [""] in the CRD, making all DisaggregatedSet creation fail validation - Update fixture YAML generator for v1 API structure (spec: wrapper for LeaderWorkerSetSpec fields, v1alpha1 -> v1) - Fix controller pod count assertion to allow >= 1 replicas - Add DisaggregatedSet CRD to kustomize config/crd/kustomization.yaml Signed-off-by: Mathis Felardos <mathis@mistral.ai> * [DisaggregatedSet][2/N] Remove standalone disaggregatedset test targets Remove `make -C disaggregatedset test` and `make -C disaggregatedset test-e2e` from the root Makefile since the tests are now integrated into the main module under pkg/controllers/disaggregatedset/ and test/e2e/disaggregatedset/. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * [DisaggregatedSet][2/N] Remove e2e build tags for ginkgo compatibility Remove //go:build e2e constraint from DisaggregatedSet e2e test files. The LWS hack/e2e-test.sh runs ginkgo without --tags=e2e, so the build constraint caused compilation failure when running test/e2e/... recursively. Matches the convention used by existing LWS e2e tests which have no build tags. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * [DisaggregatedSet][2/N] Fix e2e test integration with make test-e2e - Rewrite disaggregatedset e2e suite to be lightweight: assumes the operator is already deployed by hack/e2e-test.sh (no standalone image build/deploy logic) - Add --keep-going to ginkgo invocation so disaggregatedset suite runs even when existing LWS tests have failures - Add synchronous cleanup waits (ForLWSCount/ForPodCount) between step tracking test cases to prevent resource leaks Signed-off-by: Mathis Felardos <mathis@mistral.ai> * fix: resolve golangci-lint errors in disaggregatedset controller - Fix goimports formatting in e2e_test.go - Remove unused parameters from planner functions (source, currentNew) - Add nolint:unparam for test helpers with constant namespace values - Add nolint:unparam for reconcileSimple (standard controller-runtime pattern) Signed-off-by: Mathis Felardos <mathis@mistral.ai> * revert: remove --keep-going from e2e test script Revert the --keep-going flag addition to hack/e2e-test.sh to maintain original test behavior. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * update with 'make generate' * fix: restore internalCertManagement and correct image path Address review feedback from yankay: - Restore internalCertManagement.enable: true to ensure webhook certs are provisioned - Fix image path to use correct release convention (.../lws/lws) Signed-off-by: Mathis Felardos <mathis@mistral.ai> * fix: handle percentage values in maxSurge/maxUnavailable Use intstr.GetScaledValueFromIntOrPercent instead of IntValue() to properly handle percentage strings like "25%" in rolling update config. IntValue() silently returns 0 for percentage strings, causing the user's rollout configuration to be ignored. This fix follows the same pattern used in leaderworkerset_controller.go. - Fix extractRollingUpdateConfig to use GetScaledValueFromIntOrPercent - Add unit tests for percentage handling (50%, 25%, 100%) - Add e2e test for percentage-based rolling update - Add MaxSurgeStr/MaxUnavailableStr fields to fixtures for percentages Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Rename disaggregatedsetv1 import alias to disaggregatedset Consistent with the leaderworkerset import alias convention. * Rename reconciler receiver to r for consistency with LWS * Simplify LeaderWorkerTemplate construction in workload_manager Copy the whole struct from config.Spec.LeaderWorkerTemplate instead of constructing it field-by-field, then modify only labels/annotations. * Move label and annotation constants to API package Similar to how LWS defines SetNameLabelKey, GroupIndexLabelKey etc in api/leaderworkerset/v1/leaderworkerset_types.go. * Revert import alias from disaggregatedset back to disaggregatedsetv1 * Extract utils to pkg/utils/disaggregatedset/ package Move utility functions, types, and constants out of the controller package into a dedicated utils package to avoid name collisions (e.g., SetInitialReplicas exists on both the util and the manager). * Add DisaggregatedSet test wrappers Builder-pattern wrappers for DisaggregatedSet and LWS objects, shared MakeRoleSpec helper, and DisaggregatedSetTestScheme. * Refactor test files to use shared wrappers Replace duplicated test helpers across 4 test files with shared wrappers from test/wrappers/disaggregatedset_wrappers.go. * Simplify Create() to use config.Spec directly Copy the whole spec struct and override only Replicas, instead of extracting lwsTemplate, lwsNetworkConfig, and StartupPolicy individually. Add comments explaining why mergeLabels and copyAnnotations are still needed. * Fix goimports formatting * Rename Workload types and variables to LWS naming convention Address PR #836 review feedback to use LWS directly in names, matching the LWS codebase convention of using Sts (StatefulSet) directly. Type renames: - GroupedWorkload -> RevisionRoles - GroupedWorkloads -> RevisionRolesList - GroupWorkloadsByRevision -> GroupByRevision File renames: - workload_manager.go -> lws_manager.go - workload_manager_test.go -> lws_manager_test.go Variable and function renames throughout executor, controller, service_manager and their tests. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Remove WorkloadInfo abstraction, store *LeaderWorkerSet directly Delete the WorkloadInfo struct and store *LeaderWorkerSet pointers directly in RevisionRoles.Roles. This removes the unnecessary abstraction layer that was copying fields from LWS objects into a custom struct. LeaderWorkerSetManager.Get() and List() now return *LeaderWorkerSet directly. Consumers access lws.Spec.Replicas, lws.Status.ReadyReplicas etc. InitialReplicas is accessed via GetInitialReplicas() helper. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Rename source to initialOld for consistency Rename the 'source' parameter to 'initialOld' throughout the planner and executor to make it consistent with other variable names (currentOld, targetNew) and clearer about what it represents: the initial replica counts of old revisions at rollout start. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Fix unparam lint for withInitialReplicasAnnotation test helper Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Move LWS wrapper methods to wrappers.go and remove test helpers Move generic LeaderWorkerSetWrapper methods (Labels, StatusReplicas, ReadyReplicas, CreationTimestamp, OwnerReference, Namespace) from disaggregatedset_wrappers.go to wrappers.go. Delete BuildDisaggregatedSetLWS — callers now use BuildBasicLeaderWorkerSet().Labels() instead. Remove createTestLWS, createTestLWSWithAnnotations, createTestLWSWithAnnotation, and createTestDeployment helpers. Call wrappers directly at each test site. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Document ComputeAllSteps as test-only simulation helper Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Fix unparam lint for buildManagerTestLWS namespace arg Signed-off-by: Mathis Felardos <mathis@mistral.ai> * update crds * Rename leaderworkerset import alias to leaderworkersetv1 in disaggregatedset Consistent with the convention used in the LWS codebase for versioned API imports. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Add namespace documentation to DisaggregatedSet label constants Clarify that labels and annotations are applied to resources in the same namespace as the DisaggregatedSet. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Add inline documentation to DisaggregatedSet controller Document the four-step Reconcile flow, the decision tree between rolling update and simple reconcile paths, and both cleanup functions (cleanupOldLWS vs cleanupDrainedLWS). Add warning log when multiple LWS are found for the same role and revision during drain cleanup. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Document LWS manager function semantics Document GetRevisionRolesList: what it calculates, what it returns, and the assumption behind taking index 0 for the target revision. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Document executor and move Create logging into LWSManager Add inline documentation to ReconcileRollingUpdateNew, initRollingUpdate, and ReconcileRollingUpdate explaining the high-level steps, what "stabilize" means, and the purpose of each code block. Move LWS creation logging from the executor into LWSManager.Create so it's always emitted regardless of caller. Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Fix stale workload references in controller comments Signed-off-by: Mathis Felardos <mathis@mistral.ai> * Remove redundant cleanupOldLWS from simple reconcile path cleanupDrainedLWS already runs at the start of every reconcile and handles all old revisions where every role is at 0 replicas. The simple path is only reached when totalOldReplicas == 0, so cleanupDrainedLWS has already cleaned them up. Signed-off-by: Mathis Felardos <mathis@mistral.ai> --------- Signed-off-by: Mathis Felardos <mathis@mistral.ai>
M
Mathis Felardos committed
17ee2304435d9c0a5ed47f7068242a7e1a30db1e
Parent: 3d6f7c3
Committed by GitHub <noreply@github.com>
on 5/31/2026, 1:34:47 AM