Skip to content
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

fix(k8s): correct 0.12 => 0.13 service resource conversion #5272

Merged
merged 1 commit into from Oct 19, 2023
Merged

Conversation

vvagaytsev
Copy link
Collaborator

What this PR does / why we need it:
This PR ensures the the module conversion respects the 0.13 schema validation rules, which are now stricter than in 0.12.

Which issue(s) this PR fixes:

Fixes #5269

Special notes for your reviewer:
A new function convertServiceResourceSpec was extracted to take more specific input parameters and to do the spec-to-spec conversion. This also allowed making the unit test data preparation way easier.

twelvemo
twelvemo previously approved these changes Oct 19, 2023
Copy link
Collaborator

@twelvemo twelvemo left a comment

Choose a reason for hiding this comment

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

Looks really good to me!

Copy link
Contributor

@shumailxyz shumailxyz left a comment

Choose a reason for hiding this comment

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

Thanks @vvagaytsev Fix looks great, but the tests are failing. Please check that.

Copy link
Collaborator

@twelvemo twelvemo left a comment

Choose a reason for hiding this comment

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

One test failures around expecting containerName.

@vvagaytsev
Copy link
Collaborator Author

@twelvemo @shumailxyz thank you! I've pushed the test fixes in the most recent commit. Let's wait for the CI completion.

Copy link
Collaborator

@twelvemo twelvemo left a comment

Choose a reason for hiding this comment

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

Tests looking good now too! Thanks so much for the fast fix!

Copy link
Contributor

@shumailxyz shumailxyz left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks 👍

@vvagaytsev vvagaytsev added this pull request to the merge queue Oct 19, 2023
Merged via the queue into main with commit bfdd0af Oct 19, 2023
42 checks passed
@vvagaytsev vvagaytsev deleted the fix/5269 branch October 19, 2023 14:42
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.

0.13: [Bug]: Module task with podSelector does not work with 0.13
3 participants