ITADN

[Design] EventAction* namespace for events.k8s.io/v1 migration

#7527OpenTej-Katika 创建于 2026-05-20
T
Tej-Katikacommented
Part of #7251. ## Background `controller-runtime` v0.23.1 deprecated `Manager.GetEventRecorderFor` in favor of `Manager.GetEventRecorder`, which records `events.k8s.io/v1` Events instead of the legacy `core/v1` Events. Issue #7251 tracks the karmada-wide migration. Unlike `core/v1`, the `events.k8s.io/v1` API treats `action` as a **required, machine-readable** field on every emitted Event ([upstream type definition](https://github.com/kubernetes/kubernetes/blob/9169d916a0ae6e32118b780e9055f2d928037737/staging/src/k8s.io/api/events/v1/types.go#L57-L59)). Karmada therefore needs a deliberate, project-wide action namespace before the per-controller migration PRs can land. This issue proposes that namespace and asks for design review. The constants are implemented and validated in PoC PR #7465 (cronfederatedhpa slice). ## Design decisions in #7251 thread 1. **Per-operation scope, not per-controller.** `reportingController` already identifies the source, so repeating the controller name in `action` is redundant. Patterned on upstream Kubernetes — e.g. `staging/src/k8s.io/apiserver/pkg/server/dynamiccertificates/` uses `SNICertificateReload`, `CACertificateReload`. 2. **Reason strings preserved byte-for-byte.** No external-consumer-visible wire change (dashboards, monitoring, alerts that watch event reasons keep working). The action namespace is purely additive. 3. **`related` argument may be `nil`** when there is no meaningful secondary object. 4. **No RBAC changes in the migration PRs.** karmada-controller-manager's kubeconfig (`CN=system:admin, O=system:masters`) already implies cluster-admin on the karmada-apiserver, covering both core Events and events.k8s.io Events. RBAC cleanup gets revisited after all controllers are migrated. ## Proposed action namespace (**34** constants) ### Paired actions (cover Failed/Succeed reason pairs) | Action constant | Value | Paired reason(s) | Emitting packages | |---|---|---|---| | `EventActionCreateExecutionSpace` | `CreateExecutionSpace` | `CreateExecutionSpaceFailed/Succeed` | cluster | | `EventActionRemoveExecutionSpace` | `RemoveExecutionSpace` | `RemoveExecutionSpaceFailed/Succeed` | cluster | | `EventActionTaintCluster` | `TaintCluster` | `TaintClusterFailed/Succeed` | cluster, taint | | `EventActionSyncImpersonationConfig` | `SyncImpersonationConfig` | `SyncImpersonationConfigFailed/Succeed` | unifiedauth | | `EventActionReflectStatus` | `ReflectStatus` | `ReflectStatusFailed/Succeed` | status/work | | `EventActionInterpretHealth` | `InterpretHealth` | `InterpretHealthFailed/Succeed` | status/work | | `EventActionSyncWorkload` | `SyncWorkload` | `SyncWorkloadFailed/Succeed` | execution | | `EventActionCleanupWork` | `CleanupWork` | `CleanupWorkFailed` *(no paired success)* | binding | | `EventActionSyncScheduleResultToDependencies` | `SyncScheduleResultToDependencies` | `SyncScheduleResultToDependenciesFailed/Succeed` | dependenciesdistributor | | `EventActionSyncWork` | `SyncWork` | `SyncWorkFailed/Succeed` | binding (RB+CRB) | | `EventActionAggregateStatus` | `AggregateStatus` | `AggregateStatusFailed/Succeed` | status, helper/workstatus | | `EventActionScheduleBinding` | `ScheduleBinding` | `ScheduleBindingFailed/Succeed` | scheduler | | `EventActionDescheduleBinding` | `DescheduleBinding` | `DescheduleBindingFailed/Succeed` | descheduler | | `EventActionEvictWorkloadFromCluster` | `EvictWorkloadFromCluster` | `EvictWorkloadFromClusterFailed/Succeed` | helper/binding | | `EventActionSyncFederatedResourceQuota` | `SyncFederatedResourceQuota` | `SyncFederatedResourceQuotaFailed/Succeed` | federatedresourcequota/sync | | `EventActionCollectFederatedResourceQuotaStatus` | `CollectFederatedResourceQuotaStatus` | `CollectFederatedResourceQuotaStatusFailed/Succeed` | federatedresourcequota/status | | `EventActionCollectFederatedResourceQuotaOverallStatus` | `CollectFederatedResourceQuotaOverallStatus` | `CollectFederatedResourceQuotaOverallStatusFailed/Succeed` | federatedresourcequota/enforcement | | `EventActionApplyPolicy` | `ApplyPolicy` | `ApplyPolicyFailed/Succeed` | detector | | `EventActionApplyOverridePolicy` | `ApplyOverridePolicy` | `ApplyOverridePolicyFailed/Succeed` | overridemanager | | `EventActionPreemptPolicy` | `PreemptPolicy` | `PreemptPolicyFailed/Succeed` | detector/preemption | | `EventActionGetDependencies` | `GetDependencies` | `GetDependenciesFailed/Succeed` | dependenciesdistributor | | `EventActionGetComponents` | `GetComponents` | `GetComponentsFailed/Succeed` | detector | | `EventActionGetReplicas` | `GetReplicas` | `GetReplicasFailed/Succeed` | detector | | `EventActionSyncDerivedService` | `SyncDerivedService` | `SyncDerivedServiceFailed/Succeed` | mcs/service_import | | `EventActionSyncService` | `SyncService` | `SyncServiceFailed/Succeed` | multiclusterservice | | `EventActionDispatchEndpointSlice` | `DispatchEndpointSlice` | `DispatchEndpointSliceFailed/Succeed` | endpointslice_dispatch | ### Singleton actions (reasons with no paired success/failure variant) | Action constant | Value | Singleton reason | Emitting package | |---|---|---|---| | `EventActionDispatchWork` | `DispatchWork` | `WorkDispatching` | execution | | `EventActionResolveDependencyPolicy` | `ResolveDependencyPolicy` | `DependencyPolicyConflict` | dependenciesdistributor/policy_utils | | `EventActionResolveCluster` | `ResolveCluster` | `ClusterNotFound` | multiclusterservice | | `EventActionValidateClusterAPI` | `ValidateClusterAPI` | `APIIncompatible` | multiclusterservice | ### CronFederatedHPA (PoC slice in #7465) | Action constant | Value | Paired reason | Emitting package | |---|---|---|---| | `EventActionStartCronFederatedHPARule` | `StartCronFederatedHPARule` | `StartRuleFailed` | cronfederatedhpa | | `EventActionUpdateCronFederatedHPA` | `UpdateCronFederatedHPA` | `UpdateCronFederatedHPAFailed` | cronfederatedhpa | | `EventActionScaleCronFederatedHPA` | `ScaleCronFederatedHPA` | `ScaleFailed` | cronfederatedhpa | | `EventActionUpdateCronFederatedHPAStatus` | `UpdateCronFederatedHPAStatus` | `UpdateStatusFailed` | cronfederatedhpa | ## Naming convention - **Paired reasons (`<Op>Failed`/`<Op>Succeed`):** action is `<Op>` — drop the outcome suffix. E.g. `EventReasonScheduleBindingFailed/Succeed` → `EventActionScheduleBinding`. - **Singletons:** action is named after the operation that emits the event, not the outcome. E.g. `EventReasonClusterNotFound` is emitted while resolving a cluster, so the action is `ResolveCluster`. - **Operation-scoped, not controller-scoped** (matches upstream — see `dynamiccertificates`). ## Out of scope (flagged for follow-up) **13 literal-string reasons** are currently emitted without `EventReason*` constants. Constants for these will be added inside each per-controller migration PR, not here: - `pkg/controllers/federatedhpa/federatedhpa_controller.go`: `FailedGetScale`, `FailedGetScaleTargetRef`, `FailedGetBindings`, `FailedGetTargetClusters`, `FailedComputeMetricsReplicas`, `FailedRescale`, `SuccessfulRescale`, `SelectorRequired`, `InvalidSelector`, `AmbiguousSelector`, `FailedUpdateStatus` - `pkg/servicenameresolutiondetector/coredns/detector.go`: `LoadCorednsConditionFailed`, `StoreCorednsConditionFailed` - `operator/pkg/controller/karmada/validating.go`: validation-error literal reason **Two pre-existing reason-name vs reason-value mismatches** in `pkg/events/events.go`: - `EventReasonSyncFederatedResourceQuotaFailed = "SyncWorkFailed"` clashes with `EventReasonSyncWorkFailed` - `EventReasonCollectFederatedResourceQuotaStatusFailed = "AggregateStatusFailed"` clashes with `EventReasonAggregateStatusFailed` These emit identical wire strings across different operations. Worth a dedicated cleanup PR; not safe to change inside the events-API migration since the reason string is the externally-visible surface. ## PoC validation PR #7465 implements the cronfederatedhpa slice end-to-end against this namespace: - 1 recorder acquisition in `cmd/controller-manager/app/controllermanager.go`, 1 in `NewCronHandler`, 5 emission sites across the controller and the scaling job. - All 4 cronfederatedhpa `EventAction*` constants exercised. - Reason strings unchanged (verified by a `capturingRecorder` unit test). - The full 34-constant namespace lives in `pkg/events/events.go` so the per-controller follow-ups have somewhere to import from. Once this design lands without objections, #7465 becomes the template for the remaining per-controller migration PRs under #7251. ## Asking for feedback on 1. **Namespace shape** — any actions you'd name differently, split, or collapse? 2. **Naming convention** — does dropping the `Failed/Succeed` suffix from paired reasons read cleanly? 3. **Out-of-scope split** — does deferring the 13 literal-string reasons + the 2 reason-value mismatches to their own PRs match how you'd sequence this?
12 条评论