Skip to content

Commit

Permalink
control-service: Add support for vdk version (#2943)
Browse files Browse the repository at this point in the history
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]>
Co-authored-by: github-actions <>
  • Loading branch information
doks5 committed Dec 14, 2023
1 parent 356879d commit dbb2e00
Show file tree
Hide file tree
Showing 14 changed files with 98 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private void setVdkVersionForDeployment() throws Exception {
.with(user("user"))
.content(getDataJobDeploymentVdkVersionRequestBody("new_vdk_version_tag"))
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isBadRequest());
.andExpect(status().isAccepted());
}

private void disableDeployment() throws Exception {
Expand Down Expand Up @@ -80,7 +80,7 @@ private void resetVdkDeploymentVersion() throws Exception {
.with(user("user"))
.content(getDataJobDeploymentVdkVersionRequestBody(""))
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isBadRequest());
.andExpect(status().isAccepted());
}

private MvcResult getDeployment() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@ public void testDataJobDeploymentDynamicVdkVersion() throws Exception {
.with(user("user"))
.content(getDataJobDeploymentVdkVersionRequestBody("new_vdk_version_tag"))
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isBadRequest());
.andExpect(status().isAccepted());

// verify vdk version is not changed
// verify vdk version is changed
mockMvc
.perform(
get(String.format(
Expand All @@ -223,7 +223,7 @@ public void testDataJobDeploymentDynamicVdkVersion() throws Exception {
.with(user("user"))
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$.vdk_version", is("release")));
.andExpect(jsonPath("$.vdk_version", is("new_vdk_version_tag")));

// Execute change python version and set corresponding vdk version for deployment
mockMvc
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public static DesiredDataJobDeployment toDesiredDataJobDeployment(JobDeployment

deployment.setGitCommitSha(jobDeployment.getGitCommitSha());
deployment.setPythonVersion(jobDeployment.getPythonVersion());
deployment.setVdkVersion(jobDeployment.getVdkVersion());

return deployment;
}
Expand Down Expand Up @@ -222,6 +223,10 @@ public static DesiredDataJobDeployment mergeDeployments(
newDeployment.getPythonVersion() != null
? newDeployment.getPythonVersion()
: oldDeployment.getPythonVersion());
mergedDeployment.setVdkVersion(
newDeployment.getVdkVersion() != null
? newDeployment.getVdkVersion()
: oldDeployment.getVdkVersion());
mergedDeployment.setLastDeployedBy(
userDeployer != null ? userDeployer : oldDeployment.getLastDeployedBy());
mergedDeployment.setSchedule(
Expand All @@ -236,6 +241,8 @@ public static DesiredDataJobDeployment mergeDeployments(
? newDeployment.getEnabled()
: oldDeployment.getEnabled());

resetVdkVersionIfPythonVersionChange(oldDeployment, mergedDeployment);

return mergedDeployment;
}

Expand All @@ -260,6 +267,15 @@ private static void checkDeploymentsCanBeMerged(
}
}

private static void resetVdkVersionIfPythonVersionChange(
DesiredDataJobDeployment oldDeployment, DesiredDataJobDeployment newDeployment) {
if (newDeployment.getPythonVersion() != null
&& oldDeployment.getPythonVersion() != null
&& !oldDeployment.getPythonVersion().equals(newDeployment.getPythonVersion())) {
newDeployment.setVdkVersion(null);
}
}

private static void mergeDeploymentResources(
DesiredDataJobDeployment mergedDeployment,
JobDeployment newDeployment,
Expand Down Expand Up @@ -311,6 +327,7 @@ public static DataJobDeploymentStatus toJobDeploymentStatus(
var deploymentStatus = new DataJobDeploymentStatus();
deploymentStatus.setJobVersion(actualDataJobDeployment.getGitCommitSha());
deploymentStatus.setPythonVersion(actualDataJobDeployment.getPythonVersion());
deploymentStatus.setVdkVersion(actualDataJobDeployment.getVdkVersion());
deploymentStatus.setId(actualDataJobDeployment.getDataJobName());
deploymentStatus.setEnabled(actualDataJobDeployment.getEnabled());
deploymentStatus.setContacts(getContactsFromJob(job));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ public void patchDeployment(DataJob dataJob, JobDeployment jobDeployment) {
dataJob.getJobConfig().getTeam(), dataJob.getName(), deploymentStatus.get());
var mergedDeployment =
DeploymentModelConverter.mergeDeployments(oldDeployment, jobDeployment);
resetVdkVersionIfPythonVersionChange(oldDeployment, mergedDeployment);
validateFieldsCanBePatched(oldDeployment, mergedDeployment);

// we are setting sendNotification to false since it's not necessary. If something fails we'd
Expand Down Expand Up @@ -122,17 +123,6 @@ private void validateFieldsCanBePatched(
mergedDeployment.getPythonVersion(),
"Use POST HTTP request to change python version.");
}

if (mergedDeployment.getVdkVersion() != null
&& !mergedDeployment.getVdkVersion().equals(oldDeployment.getVdkVersion())) {
throw new ApiConstraintError(
"vdk_version",
String.format(
"same as the current vdk version -- %s -- when using PATCH request.",
oldDeployment.getVdkVersion()),
mergedDeployment.getPythonVersion(),
"Use POST HTTP request to change vdk version.");
}
}

/**
Expand Down Expand Up @@ -172,6 +162,7 @@ public void updateDeployment(
dataJob.getJobConfig().getTeam(), dataJob.getName(), deploymentStatus.get());
setPythonVersionIfNull(oldDeployment, jobDeployment);
jobDeployment = DeploymentModelConverter.mergeDeployments(oldDeployment, jobDeployment);
resetVdkVersionIfPythonVersionChange(oldDeployment, jobDeployment);
}

if (jobDeployment.getPythonVersion() == null) {
Expand Down Expand Up @@ -280,4 +271,13 @@ private void setPythonVersionIfNull(JobDeployment oldDeployment, JobDeployment n
newDeployment.setPythonVersion(supportedPythonVersions.getDefaultPythonVersion());
}
}

private void resetVdkVersionIfPythonVersionChange(
JobDeployment oldDeployment, JobDeployment newDeployment) {
if (newDeployment.getPythonVersion() != null
&& oldDeployment.getPythonVersion() != null
&& !oldDeployment.getPythonVersion().equals(newDeployment.getPythonVersion())) {
newDeployment.setVdkVersion(null);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,10 @@ private void updateCronJob(DataJob dataJob, JobDeployment jobDeployment, String
"-c",
"cp -r $(python -c \"from distutils.sysconfig import get_python_lib;"
+ " print(get_python_lib())\") /vdk/. && cp /usr/local/bin/vdk /vdk/.");
var jobVdkImage = supportedPythonVersions.getVdkImage(jobDeployment.getPythonVersion());
var jobVdkImage =
isVdkVersionPassedDifferentFromOneSetByPythonVersion(jobDeployment)
? supportedPythonVersions.replaceVdkVersionInImage(jobDeployment.getVdkVersion())
: supportedPythonVersions.getVdkImage(jobDeployment.getPythonVersion());
var jobInitContainer =
KubernetesService.container(
"vdk",
Expand Down Expand Up @@ -372,4 +375,16 @@ private Map<String, String> getJobAnnotations(
public static String getCronJobName(String jobName) {
return jobName;
}

private boolean isVdkVersionPassedDifferentFromOneSetByPythonVersion(
JobDeployment jobDeployment) {
var passedVdkVersion = jobDeployment.getVdkVersion();
var vdkVersionSetByPythonVersion =
DockerImageName.getTag(
supportedPythonVersions.getVdkImage(jobDeployment.getPythonVersion()));

return passedVdkVersion != null
&& !passedVdkVersion.isEmpty()
&& !passedVdkVersion.equals(vdkVersionSetByPythonVersion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,10 @@ private KubernetesService.CronJob getCronJob(
"-c",
"cp -r $(python -c \"from distutils.sysconfig import get_python_lib;"
+ " print(get_python_lib())\") /vdk/. && cp /usr/local/bin/vdk /vdk/.");
var jobVdkImage = supportedPythonVersions.getVdkImage(jobDeployment.getPythonVersion());
var jobVdkImage =
isVdkVersionPassedDifferentFromOneSetByPythonVersion(jobDeployment)
? supportedPythonVersions.replaceVdkVersionInImage(jobDeployment.getVdkVersion())
: supportedPythonVersions.getVdkImage(jobDeployment.getPythonVersion());
var jobInitContainer =
KubernetesService.container(
"vdk",
Expand Down Expand Up @@ -481,4 +484,16 @@ private void handleResourcesException(ParseException e) {
"Verify the string can be parsed to a number");
log.error(errorMessage.toString(), e);
}

private boolean isVdkVersionPassedDifferentFromOneSetByPythonVersion(
DesiredDataJobDeployment jobDeployment) {
var passedVdkVersion = jobDeployment.getVdkVersion();
var vdkVersionSetByPythonVersion =
DockerImageName.getTag(
supportedPythonVersions.getVdkImage(jobDeployment.getPythonVersion()));

return passedVdkVersion != null
&& !passedVdkVersion.isEmpty()
&& !passedVdkVersion.equals(vdkVersionSetByPythonVersion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,9 @@ public String getDefaultPythonVersion() {
private String getBuilderImage(Map<String, String> supportedPythonVersion) {
return supportedPythonVersion.getOrDefault(BUILDER_IMAGE, dockerRegistryService.builderImage());
}

public String replaceVdkVersionInImage(String vdkVersion) {
String defaultVdkImage = getDefaultVdkImage();
return defaultVdkImage.replace(DockerImageName.getTag(defaultVdkImage), vdkVersion);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public abstract class BaseDataJobDeployment {

private String pythonVersion;

private String vdkVersion;

private String gitCommitSha;

private String schedule;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
alter table if exists desired_data_job_deployment
add column if not exists vdk_version varchar;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
alter table if exists actual_data_job_deployment
add column if not exists vdk_version varchar;
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public void testScheduleLabelsAnnotations() {
.thenReturn(new KubernetesService.Resources("1.5", "200"));
Mockito.when(kubernetesResources.dataJobInitContainerRequests())
.thenReturn(new KubernetesService.Resources("1.5", "200"));
Mockito.when(supportedPythonVersions.getVdkImage("3.9")).thenReturn("release");

var annotationCaptor = ArgumentCaptor.forClass(Map.class);
var labelCaptor = ArgumentCaptor.forClass(Map.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ public void setUp() {

when(kubernetesService.readCronJob(TEST_CRONJOB_NAME))
.thenReturn(Optional.of(TestUtils.getJobDeploymentStatus()));
when(supportedPythonVersions.getVdkImage(any())).thenReturn("release");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,8 @@ public void setUp() {
when(defaultConfigurations.dataJobLimits())
.thenReturn(new KubernetesService.Resources("2000m", "1G"));

when(supportedPythonVersions.getDefaultVdkImage()).thenReturn("release");
when(supportedPythonVersions.getDefaultVdkImage()).thenReturn("domain:5000/name:release");
when(supportedPythonVersions.getVdkImage(any())).thenReturn("domain:5000/name:release");

JobConfig jobConfig = new JobConfig();
jobConfig.setSchedule(TEST_JOB_SCHEDULE);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public void getVdkImage_defaultImage() {
supportedPythonVersions, SUPPORTED_PYTHON_VERSIONS, supportedVersions);
ReflectionTestUtils.setField(supportedPythonVersions, DEFAULT_PYTHON_VERSION, "3.7");

final String defaultVdkImage = "test_vdk_image_3.7";
final String defaultVdkImage = "domain:5000/name:test_vdk_image_3.7";

Assertions.assertEquals(defaultVdkImage, supportedPythonVersions.getVdkImage("3.11"));
}
Expand All @@ -240,17 +240,31 @@ public void getVdkImage_defaultImage() {
public void getVdkImage_multipleSupportedVersions() {
var supportedVersions = generateSupportedPythonVersionsConf();

final String resultVdkImg = "test_vdk_image_3.8";
final String resultVdkImg = "domain:5000/name:test_vdk_image_3.8";
ReflectionTestUtils.setField(
supportedPythonVersions, SUPPORTED_PYTHON_VERSIONS, supportedVersions);

Assertions.assertEquals(resultVdkImg, supportedPythonVersions.getVdkImage("3.8"));
}

@Test
public void replaceVdkVersionInImage_replaceTheImage() {
var supportedVersions = generateSupportedPythonVersionsConf();
ReflectionTestUtils.setField(
supportedPythonVersions, SUPPORTED_PYTHON_VERSIONS, supportedVersions);
ReflectionTestUtils.setField(supportedPythonVersions, DEFAULT_PYTHON_VERSION, "3.8");
String resImage = "domain:5000/name:replaced_vdk_image";

Assertions.assertEquals(
resImage, supportedPythonVersions.replaceVdkVersionInImage("replaced_vdk_image"));
}

private static Map<String, Map<String, String>> generateSupportedPythonVersionsConf() {
return Map.of(
"3.7", Map.of(BASE_IMAGE, "python:3.7-slim", VDK_IMAGE, "test_vdk_image_3.7"),
"3.8", Map.of(BASE_IMAGE, "python:3.8-slim", VDK_IMAGE, "test_vdk_image_3.8"),
"3.7",
Map.of(BASE_IMAGE, "python:3.7-slim", VDK_IMAGE, "domain:5000/name:test_vdk_image_3.7"),
"3.8",
Map.of(BASE_IMAGE, "python:3.8-slim", VDK_IMAGE, "domain:5000/name:test_vdk_image_3.8"),
"3.9", Map.of(BASE_IMAGE, "python:3.9-slim", VDK_IMAGE, "test_vdk_image_3.9"));
}
}

0 comments on commit dbb2e00

Please sign in to comment.