Dennis-Mircea opened a new pull request, #1120:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1120
## What is the purpose of the change
`FlinkResourceContextFactory` currently holds a single `EventRecorder` as a
field, while each controller separately creates its own `EventRecorder` in
`FlinkOperator.register*Controller`. This makes `FlinkService`-emitted events
route through the operator-scoped recorder rather than the calling controller's
recorder, producing a silent asymmetry that becomes a real bug the moment any
per-instance state is added to `EventRecorder`. This PR removes the
factory-held recorder and threads the recorder through
`getResourceContext(...)` so each controller owns its recorder end to end.
## Brief change log
- Added an `EventRecorder` field (with getter) to `FlinkResourceContext`,
plumbed through `FlinkDeploymentContext` and `FlinkSessionJobContext`
constructors.
- Removed the `EventRecorder` constructor argument from
`FlinkResourceContextFactory`. Added it as a third parameter to
`getResourceContext(resource, josdkContext, eventRecorder)`. Changed
`getFlinkService(ctx)` to read the recorder via `ctx.getEventRecorder()`.
- Updated `FlinkOperator` so each `register*Controller` method owns its
own `EventRecorder`. `registerBlueGreenController` gains one for the first time
(blue-green previously had none). The shared operator-scoped `EventRecorder`
field was removed.
- Added an `EventRecorder` to `FlinkBlueGreenDeploymentController` and
`BlueGreenContext`. All six `getResourceContext(...)` call sites in
`BlueGreenDeploymentService` now pass `context.getEventRecorder()`.
- Updated `FlinkDeploymentController`, `FlinkSessionJobController`,
`StateSnapshotReconciler`, and `StateSnapshotObserver` to pass their own
recorder on every `getResourceContext(...)` call.
- Updated the test wiring (`OperatorTestBase`,
`TestingFlinkResourceContextFactory`, the four `Testing*Controller` helpers,
and the unit tests that construct contexts directly) to match the new
signatures.
## Verifying this change
This change is already covered by existing tests, such as
`FlinkDeploymentControllerTest`, `FlinkSessionJobControllerTest`,
`FlinkStateSnapshotControllerTest`, `ApplicationReconcilerTest`,
`SessionJobReconcilerTest`, `StateSnapshotReconcilerTest`,
`BlueGreenUtilsTest`, `NativeFlinkServiceTest`, `StandaloneFlinkServiceTest`,
and `FlinkResourceContextTest`.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changes to the `CustomResourceDescriptors`:
no
- Core observer or reconciler logic that is regularly executed: yes
(signature change to `FlinkResourceContextFactory.getResourceContext` and to
the four controller constructors. Behavior is unchanged because `EventRecorder`
is stateless with respect to `client` and `listeners`.)
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
