Fix infinite resource updates due to canonical format conversion of resource requirements #2565
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Donnerbart
force-pushed
the
bugfix/2509-ssa-matcher-quantity/next
branch
4 times, most recently
from
eae7969 to
fc2de09
Compare
Donnerbart
commented
Oct 28, 2024
|
|
||
| static void sanitizeResourceRequirements(final Map<String, Object> actualMap, | ||
| final PodTemplateSpec actualTemplate, final PodTemplateSpec desiredTemplate) { | ||
| if (actualTemplate == null || desiredTemplate == null) { |
There was a problem hiding this comment.
I'm not 100% sure if all of these null checks are needed. I've added them for all fields that have no default value in their declaration.
csviri
approved these changes
Oct 31, 2024
There was a problem hiding this comment.
Overall this looks good to me, just made some really minor. comments.
In future maybe we could have sanitizers customizable, like have the ability to pass only few sanitizers if there will be more.
Thank you @Donnerbart !!
...orsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java
Outdated
Show resolved
Hide resolved
...k/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java
Show resolved
Hide resolved
|
I guess this needs a rebase. Sorry I'm on out this and partially next week. @metacosm from my side this PR can go in after rebase, pls review it also. |
Signed-off-by: David Sondermann <[email protected]>
… SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]>
… of resource requirements Signed-off-by: David Sondermann <[email protected]>
…tizerTest Signed-off-by: David Sondermann <[email protected]>
Donnerbart
force-pushed
the
bugfix/2509-ssa-matcher-quantity/next
branch
from
17e02d9 to
012f87b
Compare
|
Rebase is done. |
csviri
approved these changes
Nov 6, 2024
|
Sorry, I should have merged this before rebasing next. Apologies for the extra work that was bad scheduling on my part. 🙍 |
|
No worries, I'll rebase this as often as needed. I appreciate a lot how quickly you've looked into this contribution. I've also created a backport for the |
metacosm
pushed a commit
that referenced
this pull request
Nov 6, 2024
…resource requirements (#2565) * refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]> Signed-off-by: Chris Laprun <[email protected]>
csviri
pushed a commit
that referenced
this pull request
Nov 13, 2024
…resource requirements (#2565) * refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]> Signed-off-by: Chris Laprun <[email protected]>
metacosm
pushed a commit
that referenced
this pull request
Nov 19, 2024
…resource requirements (#2565) * refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]> Signed-off-by: Chris Laprun <[email protected]>
metacosm
pushed a commit
that referenced
this pull request
Nov 20, 2024
…resource requirements (#2565) * refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]> Signed-off-by: Chris Laprun <[email protected]>
metacosm
pushed a commit
that referenced
this pull request
Nov 27, 2024
…resource requirements (#2565) * refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]> Signed-off-by: Chris Laprun <[email protected]>
This was referenced Dec 3, 2024
csviri
pushed a commit
that referenced
this pull request
Dec 6, 2024
…resource requirements (#2565) * refactor: clean up SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * test: add missing tests for StatefulSet with VolumeClaimTemplates for SSABasedGenericKubernetesResourceMatcher Signed-off-by: David Sondermann <[email protected]> * fix: Fix infinite resource updates due to canonical format conversion of resource requirements Signed-off-by: David Sondermann <[email protected]> * test: Add test cases with init containers to ResourceRequirementsSanitizerTest Signed-off-by: David Sondermann <[email protected]> --------- Signed-off-by: David Sondermann <[email protected]> Signed-off-by: Chris Laprun <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2509
Code coverage
Branch
nextBranch
bugfix/2509-ssa-matcher-quantity/next