james-kan-shopify commented on code in PR #1053:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/1053#discussion_r2710803602
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -109,13 +110,38 @@ public UpdateControl<FlinkBlueGreenDeployment>
initiateDeployment(
*/
public UpdateControl<FlinkBlueGreenDeployment> checkAndInitiateDeployment(
BlueGreenContext context, BlueGreenDeploymentType
currentBlueGreenDeploymentType) {
+
BlueGreenDiffType specDiff = getSpecDiff(context);
if (specDiff != BlueGreenDiffType.IGNORE) {
FlinkDeployment currentFlinkDeployment =
context.getDeploymentByType(currentBlueGreenDeploymentType);
- if (isFlinkDeploymentReady(currentFlinkDeployment)) {
+ if (specDiff == BlueGreenDiffType.SUSPEND &&
currentFlinkDeployment != null) {
+ setLastReconciledSpec(context);
+ LOG.info(
+ "In-place suspension for '{}'",
+ currentFlinkDeployment.getMetadata().getName());
+ return patchFlinkDeployment(context,
currentBlueGreenDeploymentType);
+ }
+
+ if (specDiff == BlueGreenDiffType.RESUME && currentFlinkDeployment
!= null) {
+ setLastReconciledSpec(context);
+ LOG.info(
+ "In-place resume for '{}'",
currentFlinkDeployment.getMetadata().getName());
+ return patchFlinkDeployment(context,
currentBlueGreenDeploymentType);
+ }
+
+ // Check if child is currently suspended - if so, just patch specs
without restart
Review Comment:
Hi! Yes, a RESUME will reconcile all changes made during suspension.
However, without this patch, the FlinkDeployment and FlinkBlueGreenDeployment
will be out of sync during that entire time.
We think keeping them in sync provides a better user experience: users can
inspect the child FlinkDeployment at any time and see the exact spec that will
be executed, rather than having to track changes across resources. We're hoping
to eventually make the FlinkBlueGreenDeployment the single source of truth,
with the child always reflecting the parent's current desired state.
That said, we're open to other perspectives! If you think the extra patch
call isn't worth it, we'd be happy to discuss!
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -397,11 +446,44 @@ public UpdateControl<FlinkBlueGreenDeployment>
monitorTransition(
}
}
+ private boolean isFlinkDeploymentSuspended(FlinkDeployment deployment) {
+ if (deployment == null) {
+ return false;
+ }
+ return deployment.getStatus().getLifecycleState() ==
ResourceLifecycleState.SUSPENDED
+ && isChildSuspended(deployment);
Review Comment:
Thanks for that! Ya, that's a redundant check.
--
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]
