-
Notifications
You must be signed in to change notification settings - Fork 334
Fix issue where pod template override pod spec was missing #3270
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
Conversation
…uded Signed-off-by: Jason Parraga <[email protected]>
Signed-off-by: Jason Parraga <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3270 +/- ##
==========================================
- Coverage 83.35% 75.86% -7.50%
==========================================
Files 347 261 -86
Lines 28791 24574 -4217
Branches 2960 2962 +2
==========================================
- Hits 23999 18643 -5356
- Misses 3956 5106 +1150
+ Partials 836 825 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Jason Parraga <[email protected]>
fg91
left a comment
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.
Thanks for fixing, LG 🙏
…3270) * Fix issue where pod template override pod spec was conditionally included Signed-off-by: Jason Parraga <[email protected]> * Add unit test Signed-off-by: Jason Parraga <[email protected]> * Make unit test parameterized Signed-off-by: Jason Parraga <[email protected]> --------- Signed-off-by: Jason Parraga <[email protected]> Signed-off-by: Atharva <[email protected]>
Tracking issue
Fixes flyteorg/flyte#6463
Why are the changes needed?
The pod_spec was not included in the pod template when not using fast registration. This would lead to bad task specification errors on the backend since there would be no pod spec but a primary container name was configured.
Although this comes from our custom Armada plugin, the plugin is just using common pod_helpers code.
After debugging I found that this was logged from propeller pod_helpers.go code.
What changes were proposed in this pull request?
Updates the translation code to render the pod template pod spec unconditionally.
How was this patch tested?
Tested in a production environment.
Setup process
Screenshots
Check all the applicable boxes
Summary by Bito
This pull request resolves an issue with the pod template override and the pod specification missing during non-fast registration, ensuring the pod spec is rendered unconditionally and enhancing task execution reliability. The changes were validated in a production environment.