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

control-service: protect against v1 cron batch not being supported #1777

Conversation

murphp15
Copy link
Collaborator

@murphp15 murphp15 commented Mar 23, 2023

Why

When we merged this PR https://github.com/vmware/versatile-data-kit/pull/1580/files we unintentionally broke compatibility with version 1.19 and 1.20.

We always check for v1 versions of batch jobs. checking for that version on clusters <1.21 will result in errors.

What

Within this PR we add a guard around that statement and swallow the error if it happens.

How has this been tested

installed old version of k8s locally and ran integration tests against it.
Within the class DataJobDeploymentCrudIT I also changed the property atajobs.control.k8s.k8sSupportsV1CronJob to false. As this is the config that should be set for 1.20.
All tests are green.

I also ran the integration tests on main against the old cluster to make sure it catches the bug and it does result in many failed tests.

Not handled in this PR

How do we prevent issues like this in the future?
Maybe integration tests run against many versions of k8s?

Copy link
Contributor

@ivakoleva ivakoleva left a comment

Choose a reason for hiding this comment

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

What happens when another version pops up? May need to spend some time on designing a solution for addressing those compatibility issues better.

May you create and link a ticket for the mentioned in Testing done section?

@murphp15
Copy link
Collaborator Author

@ivakoleva
ticket describing testing problem #1778.

@murphp15 murphp15 enabled auto-merge (squash) March 23, 2023 17:00
@murphp15 murphp15 merged commit a9b832d into main Mar 29, 2023
@murphp15 murphp15 deleted the person/murphp15/protect_against_v1_cron_batch_not_being_supported branch March 29, 2023 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants