[Design] EventAction* namespace for events.k8s.io/v1 migration
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 条评论