-
Notifications
You must be signed in to change notification settings - Fork 231
Improve PodTemplateSpec sanitizer for GKE Autopilot compatibility #3012
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve PodTemplateSpec sanitizer for GKE Autopilot compatibility #3012
Conversation
…herTest for better type safety Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
Signed-off-by: David Sondermann <david.sondermann@hivemq.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the PodTemplateSpec sanitizer to handle GKE Autopilot environments, which automatically inject additional resource requirements like ephemeral-storage that may not be present in the desired state.
Key Changes:
- Removed the resource map size equality check that prevented sanitization when actual and desired resources had different numbers of entries
- Added test coverage for scenarios where additional resources (e.g., ephemeral-storage) are injected by external operators like GKE Autopilot
- Updated test type parameters for better type safety
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
PodTemplateSpecSanitizer.java | Removed size mismatch filter to allow sanitization when actual resources contain additional entries injected by the platform |
PodTemplateSpecSanitizerTest.java | Removed obsolete size mismatch test and added new tests covering GKE Autopilot scenarios with ephemeral-storage injection |
SSABasedGenericKubernetesResourceMatcherTest.java | Improved type safety by adding explicit generic type parameters to test classes and builders |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
} | ||
|
||
private static class ConfigMapDR extends KubernetesDependentResource<ConfigMap, ConfigMap> { | ||
private static class ConfigMapDR extends KubernetesDependentResource<ConfigMap, HasMetadata> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: What is the reason for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make the existing field mockedContext
re-useable without any casts or warnings.
I've changed the context from Context<?>
to Context<HasMetadata>
, which works for all existing tests. And then aligned ConfigMapDR
to use HasMetadata
as primary resource as well.
The whole test class is now warning free in my IDEA.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thank you!!
Fixes #3006