Skip to content

Conversation

@ardakuyumcu
Copy link

What is the purpose of the change

This pull request simplifies Blue/Green deployment preparation by removing unnecessary name reference adjustments. Previously, the operator was performing string-based replacement of the parent deployment name with child deployment names throughout the entire spec JSON, including fields such as checkpoint and savepoint directories, metrics scope, and even configmap names from the pod template spec. This change removes that logic, limiting the name change to only the deployment metadata where it's actually needed.

Brief change log

  • Removed adjustNameReferences utility method that performed string-based spec manipulation
  • Simplified prepareFlinkDeployment to use the original spec directly without name adjustments
  • Child deployment name is now only set on the FlinkDeployment metadata, not propagated through the entire spec via string replacement

Verifying this change

This change is already covered by existing tests, such as:

  • Existing Blue/Green deployment integration tests in e2e-tests/test_bluegreen_*.sh
  • Unit tests for Blue/Green deployment preparation and transitions
  • The change maintains the same functional behavior but simplifies the implementation

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: no

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@bowenli86
Copy link
Member

bowenli86 commented Oct 24, 2025

approved workflow to run

@ardakuyumcu
Copy link
Author

ardakuyumcu commented Oct 24, 2025

It seems like since adjustNameReferences was removed from BlueGreenUtils.java, we are no longer creating a deep copy of the spec through JSON serialization/deserialization. Without this deep copy, the code in prepareFlinkDeployment is directly mutating the original BlueGreenDeployment spec, and configuration changes are being overwritten during reconciliation. The tests pass if I add back the deep copy approach without modifying the spec fields.

Although I'm not sure if this is a test side effect, or a real behavior problem with the utils code.

@schongloo
Copy link
Contributor

schongloo commented Oct 24, 2025

Thanks for the details @ardakuyumcu, indeed if we mutate the original spec during prepareFlinkDeployment subsequent calls to the reconcile loop presume the spec was altered externally (more specifically the handleSpecChangesDuringTransition function). LGTM as it currently stands with your changes.

@ardakuyumcu
Copy link
Author

Thanks @schongloo, I think next steps are on the contributors side? I'll wait for those, cheers!

Copy link
Contributor

@schongloo schongloo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants