You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "fqshopify (via GitHub)" <gi...@apache.org> on 2026/07/07 15:40:27 UTC
[PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
fqshopify opened a new pull request, #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154
## What is the purpose of the change
This pull request removes an unnecessary extra reconciliation loop during `FlinkBlueGreenDeployment` cutover.
Previously, after the operator successfully deleted the previous `FlinkDeployment`, it returned `noUpdate().rescheduleAfter(RETRY_DELAY_MS)`. The BlueGreen deployment was only finalized in a later reconciliation after the controller observed that the previous deployment was gone.
Once the previous deployment has been successfully deleted, the transition can be finalized immediately. This change reuses the existing `finalizeBlueGreenDeployment` logic in the same reconciliation that deletes the previous deployment, while preserving the retry behavior when deletion is not acknowledged.
## Brief change log
- Pass the target `FlinkBlueGreenDeploymentState` into `deleteDeployment`.
- Finalize the BlueGreen transition immediately after successful deletion of the previous `FlinkDeployment`.
- Preserve the existing retry behavior when deletion fails or is not acknowledged.
- Add a regression test verifying that the transition commits to `ACTIVE_GREEN` in the same reconciliation that deletes Blue.
## 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
## 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
Posted by "fqshopify (via GitHub)" <gi...@apache.org>.
fqshopify commented on code in PR #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154#discussion_r3579993452
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkBlueGreenDeploymentControllerTest.java:
##########
@@ -558,6 +558,69 @@ public void verifyFailureDuringTransition(FlinkVersion flinkVersion) throws Exce
testTransitionToGreen(rs, customValue, null);
}
+ @ParameterizedTest
+ @MethodSource("org.apache.flink.kubernetes.operator.TestUtils#flinkVersions")
+ public void verifyGreenNotAbortedWhenNotReadyAfterBlueDeleted(FlinkVersion flinkVersion)
+ throws Exception {
+ var blueGreenDeployment =
+ buildSessionCluster(
+ TEST_DEPLOYMENT_NAME,
+ TEST_NAMESPACE,
+ flinkVersion,
+ null,
+ UpgradeMode.STATELESS);
+ // Tiny abort grace so the abort deadline is already expired by the time Blue is deleted.
+ var configuration = blueGreenDeployment.getSpec().getConfiguration();
+ configuration.put(ABORT_GRACE_PERIOD.key(), "1");
+ configuration.put(DEPLOYMENT_DELETION_DELAY.key(), "0");
+
+ var rs = executeBasicDeployment(flinkVersion, blueGreenDeployment, false, null);
+
+ // Trigger the transition to Green
+ simulateChangeInSpec(rs.deployment, UUID.randomUUID().toString(), 0, null);
+ rs = reconcile(rs.deployment);
+ assertEquals(
+ FlinkBlueGreenDeploymentState.TRANSITIONING_TO_GREEN,
+ rs.reconciledStatus.getBlueGreenState());
+ assertEquals(2, getFlinkDeployments().size());
+
+ // Green becomes ready -> marks the ready timestamp and schedules deletion of Blue
+ simulateSuccessfulJobStart(getFlinkDeployments().get(1));
+ rs = reconcile(rs.deployment);
+
+ // Let the (zero) deletion delay elapse
+ Thread.sleep(10);
Review Comment:
Fixed
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkBlueGreenDeploymentControllerTest.java:
##########
@@ -558,6 +558,69 @@ public void verifyFailureDuringTransition(FlinkVersion flinkVersion) throws Exce
testTransitionToGreen(rs, customValue, null);
}
+ @ParameterizedTest
+ @MethodSource("org.apache.flink.kubernetes.operator.TestUtils#flinkVersions")
+ public void verifyGreenNotAbortedWhenNotReadyAfterBlueDeleted(FlinkVersion flinkVersion)
Review Comment:
Fixed
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -627,17 +627,20 @@ private UpdateControl<FlinkBlueGreenDeployment> waitBeforeDeleting(
}
private UpdateControl<FlinkBlueGreenDeployment> deleteDeployment(
- FlinkDeployment currentDeployment, BlueGreenContext context) {
+ FlinkDeployment currentDeployment,
+ BlueGreenContext context,
+ FlinkBlueGreenDeploymentState nextState) {
boolean deleted = deleteFlinkDeployment(currentDeployment, context);
if (!deleted) {
LOG.info("FlinkDeployment '{}' not deleted, will retry", currentDeployment);
+ return UpdateControl.<FlinkBlueGreenDeployment>noUpdate()
+ .rescheduleAfter(RETRY_DELAY_MS);
} else {
LOG.info("FlinkDeployment '{}' deleted!", currentDeployment);
+ return finalizeBlueGreenDeployment(context, nextState);
Review Comment:
> The delete (child) and the finalize status patch (parent) are not atomic.
Very true. I like your suggestion: once the previous deployment is gone mid-transition, treat that as the point of no return and always finalize forward instead of aborting.
I think that's a significant enough change to warrant it's own PR though so let's handle it separately.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
Posted by "Dennis-Mircea (via GitHub)" <gi...@apache.org>.
Dennis-Mircea commented on code in PR #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154#discussion_r3580323370
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -627,17 +627,20 @@ private UpdateControl<FlinkBlueGreenDeployment> waitBeforeDeleting(
}
private UpdateControl<FlinkBlueGreenDeployment> deleteDeployment(
- FlinkDeployment currentDeployment, BlueGreenContext context) {
+ FlinkDeployment currentDeployment,
+ BlueGreenContext context,
+ FlinkBlueGreenDeploymentState nextState) {
boolean deleted = deleteFlinkDeployment(currentDeployment, context);
if (!deleted) {
LOG.info("FlinkDeployment '{}' not deleted, will retry", currentDeployment);
+ return UpdateControl.<FlinkBlueGreenDeployment>noUpdate()
+ .rescheduleAfter(RETRY_DELAY_MS);
} else {
LOG.info("FlinkDeployment '{}' deleted!", currentDeployment);
+ return finalizeBlueGreenDeployment(context, nextState);
Review Comment:
Sounds good!
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
Posted by "Dennis-Mircea (via GitHub)" <gi...@apache.org>.
Dennis-Mircea commented on code in PR #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154#discussion_r3561282587
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkBlueGreenDeploymentControllerTest.java:
##########
@@ -558,6 +558,69 @@ public void verifyFailureDuringTransition(FlinkVersion flinkVersion) throws Exce
testTransitionToGreen(rs, customValue, null);
}
+ @ParameterizedTest
+ @MethodSource("org.apache.flink.kubernetes.operator.TestUtils#flinkVersions")
+ public void verifyGreenNotAbortedWhenNotReadyAfterBlueDeleted(FlinkVersion flinkVersion)
+ throws Exception {
+ var blueGreenDeployment =
+ buildSessionCluster(
+ TEST_DEPLOYMENT_NAME,
+ TEST_NAMESPACE,
+ flinkVersion,
+ null,
+ UpgradeMode.STATELESS);
+ // Tiny abort grace so the abort deadline is already expired by the time Blue is deleted.
+ var configuration = blueGreenDeployment.getSpec().getConfiguration();
+ configuration.put(ABORT_GRACE_PERIOD.key(), "1");
+ configuration.put(DEPLOYMENT_DELETION_DELAY.key(), "0");
+
+ var rs = executeBasicDeployment(flinkVersion, blueGreenDeployment, false, null);
+
+ // Trigger the transition to Green
+ simulateChangeInSpec(rs.deployment, UUID.randomUUID().toString(), 0, null);
+ rs = reconcile(rs.deployment);
+ assertEquals(
+ FlinkBlueGreenDeploymentState.TRANSITIONING_TO_GREEN,
+ rs.reconciledStatus.getBlueGreenState());
+ assertEquals(2, getFlinkDeployments().size());
+
+ // Green becomes ready -> marks the ready timestamp and schedules deletion of Blue
+ simulateSuccessfulJobStart(getFlinkDeployments().get(1));
+ rs = reconcile(rs.deployment);
+
+ // Let the (zero) deletion delay elapse
+ Thread.sleep(10);
Review Comment:
It's best to avoid using `sleep` here. Consider using an `await()` function instead.
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -627,17 +627,20 @@ private UpdateControl<FlinkBlueGreenDeployment> waitBeforeDeleting(
}
private UpdateControl<FlinkBlueGreenDeployment> deleteDeployment(
- FlinkDeployment currentDeployment, BlueGreenContext context) {
+ FlinkDeployment currentDeployment,
+ BlueGreenContext context,
+ FlinkBlueGreenDeploymentState nextState) {
boolean deleted = deleteFlinkDeployment(currentDeployment, context);
if (!deleted) {
LOG.info("FlinkDeployment '{}' not deleted, will retry", currentDeployment);
+ return UpdateControl.<FlinkBlueGreenDeployment>noUpdate()
+ .rescheduleAfter(RETRY_DELAY_MS);
} else {
LOG.info("FlinkDeployment '{}' deleted!", currentDeployment);
+ return finalizeBlueGreenDeployment(context, nextState);
Review Comment:
One concurrency gap to consider: The delete (child) and the finalize status patch (parent) are not atomic. If the reconcile crashes or the patch 409s after the delete is accepted but before finalize persists, recovery lands in Blue-gone + still `TRANSITIONING_TO_*` + abort timer set. The dispatch only checks Green readiness, not whether Blue still exists ([BlueGreenDeploymentService.java#L489](https://github.com/apache/flink-kubernetes-operator/blob/main/flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java#L489)).
That still allows a rollback to an already-deleted Blue, just far more rarely than before. I'd close it structurally rather than rely on timing: once the previous deployment is absent during `TRANSITIONING_TO_*`, treat that as the point of no return and always finalize forward, never abort.
##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkBlueGreenDeploymentControllerTest.java:
##########
@@ -558,6 +558,69 @@ public void verifyFailureDuringTransition(FlinkVersion flinkVersion) throws Exce
testTransitionToGreen(rs, customValue, null);
}
+ @ParameterizedTest
+ @MethodSource("org.apache.flink.kubernetes.operator.TestUtils#flinkVersions")
+ public void verifyGreenNotAbortedWhenNotReadyAfterBlueDeleted(FlinkVersion flinkVersion)
Review Comment:
The test asserts state/abort-timer/count but not the ingress flip to green, which is part of the cutover contract. I'd say it is worth adding.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
Posted by "fqshopify (via GitHub)" <gi...@apache.org>.
fqshopify commented on code in PR #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154#discussion_r3579994047
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -627,17 +627,20 @@ private UpdateControl<FlinkBlueGreenDeployment> waitBeforeDeleting(
}
private UpdateControl<FlinkBlueGreenDeployment> deleteDeployment(
- FlinkDeployment currentDeployment, BlueGreenContext context) {
+ FlinkDeployment currentDeployment,
+ BlueGreenContext context,
+ FlinkBlueGreenDeploymentState nextState) {
boolean deleted = deleteFlinkDeployment(currentDeployment, context);
if (!deleted) {
LOG.info("FlinkDeployment '{}' not deleted, will retry", currentDeployment);
+ return UpdateControl.<FlinkBlueGreenDeployment>noUpdate()
+ .rescheduleAfter(RETRY_DELAY_MS);
} else {
LOG.info("FlinkDeployment '{}' deleted!", currentDeployment);
+ return finalizeBlueGreenDeployment(context, nextState);
Review Comment:
> The delete (child) and the finalize status patch (parent) are not atomic.
Very true. I like your suggestion: once the previous deployment is gone mid-transition, treat that as the point of no return and always finalize forward instead of aborting.
I think that's a significant enough change to warrant it's own PR though so let's handle it separately: https://github.com/apache/flink-kubernetes-operator/pull/1161
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
Posted by "gyfora (via GitHub)" <gi...@apache.org>.
gyfora merged PR #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-40092] Finalize BlueGreen transition after deleting previous deployment [flink-kubernetes-operator]
Posted by "fqshopify (via GitHub)" <gi...@apache.org>.
fqshopify commented on code in PR #1154:
URL: https://github.com/apache/flink-kubernetes-operator/pull/1154#discussion_r3579994047
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/bluegreen/BlueGreenDeploymentService.java:
##########
@@ -627,17 +627,20 @@ private UpdateControl<FlinkBlueGreenDeployment> waitBeforeDeleting(
}
private UpdateControl<FlinkBlueGreenDeployment> deleteDeployment(
- FlinkDeployment currentDeployment, BlueGreenContext context) {
+ FlinkDeployment currentDeployment,
+ BlueGreenContext context,
+ FlinkBlueGreenDeploymentState nextState) {
boolean deleted = deleteFlinkDeployment(currentDeployment, context);
if (!deleted) {
LOG.info("FlinkDeployment '{}' not deleted, will retry", currentDeployment);
+ return UpdateControl.<FlinkBlueGreenDeployment>noUpdate()
+ .rescheduleAfter(RETRY_DELAY_MS);
} else {
LOG.info("FlinkDeployment '{}' deleted!", currentDeployment);
+ return finalizeBlueGreenDeployment(context, nextState);
Review Comment:
> The delete (child) and the finalize status patch (parent) are not atomic.
Very true. I like your suggestion: once the previous deployment is gone mid-transition, treat that as the point of no return and always finalize forward instead of aborting.
I think that's a significant enough change to warrant it's own PR though so let's handle it separately after this one: https://github.com/apache/flink-kubernetes-operator/pull/1161
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org