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: Add support for vdk version #2943

Merged
merged 12 commits into from
Dec 14, 2023

Conversation

doks5
Copy link
Contributor

@doks5 doks5 commented Nov 26, 2023

Currently, the vdk image used when a data job is deployed, is decided based on the python version. This is done in order to avoid possible incompatibility issues between the vdk image and the job base image. However, it also blocks the possibility to use testing vdk images in CI/CD acceptance tests.

This change adds functionality that allows a client to pass a specific vdk image to be used for a data job deployment. This would allow for CI/CD tests to test specific vdk images before they are released.

Testing Done: Unit and Integration tests

@doks5 doks5 changed the title control service: Add support for vdk version control-service: Add support for vdk version Nov 26, 2023
@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch 5 times, most recently from eb75c32 to b2acd7e Compare November 27, 2023 09:30
@mivanov1988
Copy link
Collaborator

Please standardize the name to either vdkVersion or vdkImage.

@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch 3 times, most recently from 33edbaa to 7b269cf Compare November 28, 2023 20:21
@mivanov1988
Copy link
Collaborator

mivanov1988 commented Nov 29, 2023

We still have both vdkVersion and vdkImage. I suggest renaming all variables to vdkImage, including the API. But let's leave the API changes for a separate PR.

@doks5
Copy link
Contributor Author

doks5 commented Nov 29, 2023

We still have both vdkVersion and vdkImage. I suggest renaming all variables to vdkImage, including the API. But let's leave the API changes for a separate PR.

@mivanov1988 Well, we cannot change the instances to vdkImage without changing the API in the same PR (without leaving the Control Service in a broken state). VDK version is passed in multiple APIs (deployment POST/PATCH) as part of DataJobDeployment, https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/model/apidefs/datajob-api/api.yaml#L1133, and in GET as part of the DataJobDeploymentStatus, https://github.com/vmware/versatile-data-kit/blob/main/projects/control-service/projects/model/apidefs/datajob-api/api.yaml#L1170 and is then propagated throughout the different Control Service components.

@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch 6 times, most recently from a1ed5c6 to f3ea160 Compare November 30, 2023 23:02
@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch 5 times, most recently from cc947f9 to b9313e7 Compare December 12, 2023 10:43
@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch 7 times, most recently from e31470e to 25d5464 Compare December 13, 2023 14:15
@mivanov1988
Copy link
Collaborator

JobDeployerV2 should be changed as well.

Copy link
Collaborator

@dakodakov dakodakov left a comment

Choose a reason for hiding this comment

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

Fix @mivanov1988 's comments and merge :)

@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch from 25d5464 to b74123a Compare December 14, 2023 08:47
@doks5
Copy link
Contributor Author

doks5 commented Dec 14, 2023

JobDeployerV2 should be changed as well.

Good point. Fixed, thanks

doks5 and others added 9 commits December 14, 2023 15:55
Currently, the vdk image used when a data job is deployed, is decided based on the
python version. This is done in order to avoid possible incompatibility issues between
the vdk image and the job base image. However, it also blocks the possibility to use
testing vdk images in CI/CD acceptance tests.

This change adds functionality that allows a client to pass a specific vdk image to be
used for a data job deployment. This would allow for CI/CD tests to test specific vdk
images before they are released.

Testing Done: Unit and Integration tests

Signed-off-by: Andon Andonov <[email protected]>
@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch from 71a596a to 88d2b9f Compare December 14, 2023 13:55
@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch from 4bb8dc4 to 1257409 Compare December 14, 2023 22:21
@doks5 doks5 force-pushed the person/andonova/vdk-version-in-sync branch from 7d992d0 to c2f0c28 Compare December 14, 2023 22:55
@doks5 doks5 merged commit dbb2e00 into main Dec 14, 2023
2 of 3 checks passed
@doks5 doks5 deleted the person/andonova/vdk-version-in-sync branch December 14, 2023 23:34
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

4 participants