Skip to content

Conversation

@drossos
Copy link

@drossos drossos commented Oct 27, 2025

What is the purpose of the change

Addresses bug when we include additional podTemplate specs, i.e "Ignored Fields" (using ReflectiveDiffBuilder.clearIgnoredFields()) in the template spec of the FlinkBlueGreenDeployment where it will never come to a conclusion / reconciled state during a Blue-Green deployment.

Brief change log

  • *Reordered where we obtain the DiffResult, that strips ignored fields, to occur before the equality check between target and current spec *

Verifying this change

This change added tests and can be verified as follows:
Newly added tests for checking all 3 cases of podTemplate ignored fields:

  • testIgnoreForRootPodTemplateAdditionalProps
  • testIgnoreForJobManagerPodTemplateAdditionalProps
  • testIgnoreForTaskManagerPodTemplateAdditionalProps

Also tested on remote K8 cluster and locally on MiniKube cluster by using a FlinkBlueGreenDeployment spec that contained valid podTemplate fields at the three previously mentioned levels.

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? n/a

@drossos drossos marked this pull request as ready for review October 28, 2025 01:55
Copy link
Contributor

@ryanvanhuuksloot ryanvanhuuksloot left a comment

Choose a reason for hiding this comment

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

Functionality wise, looks good to me

return BlueGreenDiffType.IGNORE;
}

// Case 2 & 3: Delegate to ReflectiveDiffBuilder for nested spec comparison
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT comment lingering, I think we should keep the comment but it needs to move

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.

2 participants