From 90f25f692946ca5438fc150769cf4635bb2c5d22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Kr=C3=ADdl?= Date: Mon, 14 Apr 2025 15:20:02 +0200 Subject: [PATCH] [NCL-9069]: Propagate adjuster pod memory when creating adjuster job --- .../common/profile/WithStaticBifrostUrl.java | 17 ++ .../rest/openshift/JobDefinitionCreator.java | 84 ++++++--- rest/src/main/resources/application-test.yaml | 7 +- .../openshift/JobDefinitionCreatorTest.java | 175 ++++++++++++++++++ rest/src/test/resources/job.yaml | 63 +++++++ 5 files changed, 322 insertions(+), 24 deletions(-) create mode 100644 core/src/test/java/org/jboss/pnc/reqour/common/profile/WithStaticBifrostUrl.java create mode 100644 rest/src/test/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreatorTest.java create mode 100644 rest/src/test/resources/job.yaml diff --git a/core/src/test/java/org/jboss/pnc/reqour/common/profile/WithStaticBifrostUrl.java b/core/src/test/java/org/jboss/pnc/reqour/common/profile/WithStaticBifrostUrl.java new file mode 100644 index 00000000..d198d11f --- /dev/null +++ b/core/src/test/java/org/jboss/pnc/reqour/common/profile/WithStaticBifrostUrl.java @@ -0,0 +1,17 @@ +/* + * Copyright 2024 Red Hat, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +package org.jboss.pnc.reqour.common.profile; + +import io.quarkus.test.junit.QuarkusTestProfile; + +import java.util.Map; + +public class WithStaticBifrostUrl implements QuarkusTestProfile { + + @Override + public Map getConfigOverrides() { + return Map.of("reqour.log.final-log.bifrost-uploader.base-url", "https://test.bifrost.com"); + } +} diff --git a/rest/src/main/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreator.java b/rest/src/main/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreator.java index 4c645531..d88ae01a 100644 --- a/rest/src/main/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreator.java +++ b/rest/src/main/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreator.java @@ -13,13 +13,17 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.text.StringSubstitutor; import org.eclipse.microprofile.config.inject.ConfigProperty; +import org.jboss.pnc.api.constants.BuildConfigurationParameterKeys; import org.jboss.pnc.api.reqour.dto.AdjustRequest; import org.jboss.pnc.reqour.config.ReqourConfig; import org.jboss.pnc.reqour.rest.config.ReqourRestConfig; +import org.jboss.pnc.reqour.runtime.UserLogger; +import org.slf4j.Logger; import org.slf4j.MDC; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.HashMap; import java.util.Map; @ApplicationScoped @@ -39,30 +43,27 @@ public class JobDefinitionCreator { @ConfigProperty(name = "quarkus.oidc-client.credentials.secret") String saSecret; + @Inject + @UserLogger + Logger userLogger; + + private static final double RESOURCES_MEMORY_DEFAULT = 4d; + public Job getAdjusterJobDefinition(AdjustRequest adjustRequest, String jobName) { - final Map properties; + final Map properties = new HashMap<>(); + try { - properties = Map.of( - "jobName", - jobName, - "buildType", - adjustRequest.getBuildType(), - "adjustRequest", - objectMapper.writeValueAsString(adjustRequest), - "appEnvironment", - config.appEnvironment(), - "reqourSecretKey", - config.reqourSecretKey(), - "indyUrl", - config.indyUrl(), - "bifrostUrl", - reqourCoreConfig.log().finalLog().bifrostUploader().baseUrl(), - "mdc", - objectMapper.writeValueAsString(MDC.getCopyOfContextMap()), - "saSecret", - saSecret, - "saslJaasConf", - config.saslJaasConf()); + properties.put("jobName", jobName); + properties.put("buildType", adjustRequest.getBuildType()); + properties.put("adjustRequest", objectMapper.writeValueAsString(adjustRequest)); + properties.put("appEnvironment", config.appEnvironment()); + properties.put("resourcesMemory", getResourcesMemory(adjustRequest.getBuildConfigParameters())); + properties.put("reqourSecretKey", config.reqourSecretKey()); + properties.put("indyUrl", config.indyUrl()); + properties.put("bifrostUrl", reqourCoreConfig.log().finalLog().bifrostUploader().baseUrl()); + properties.put("mdc", objectMapper.writeValueAsString(MDC.getCopyOfContextMap())); + properties.put("saSecret", saSecret); + properties.put("saslJaasConf", config.saslJaasConf()); } catch (JsonProcessingException e) { throw new RuntimeException(e); } @@ -77,4 +78,43 @@ public Job getAdjusterJobDefinition(AdjustRequest adjustRequest, String jobName) throw new RuntimeException("Unable to parse Job definition", e); } } + + String getResourcesMemory(Map buildConfigParameters) { + String podMemorySizeFromDefault = getPodMemoryString(RESOURCES_MEMORY_DEFAULT); + if (buildConfigParameters == null + || !buildConfigParameters.containsKey(BuildConfigurationParameterKeys.ALIGNMENT_POD_MEMORY)) { + userLogger.info( + "No override for alignment pod memory size provided, hence, using the default: {}", + podMemorySizeFromDefault); + return podMemorySizeFromDefault; + } + + String podMemorySizeOverride = buildConfigParameters.get(BuildConfigurationParameterKeys.ALIGNMENT_POD_MEMORY); + try { + double parsedPodMemory = Double.parseDouble(podMemorySizeOverride); + if (parsedPodMemory > 0) { + String podMemorySize = getPodMemoryString(parsedPodMemory); + userLogger.info( + "Using override '{}' for alignment pod memory size, which will be: {}", + podMemorySizeOverride, + podMemorySize); + return podMemorySize; + } else { + userLogger.info( + "Overridden alignment memory size cannot have negative value, hence, using the default: {}", + podMemorySizeFromDefault); + return podMemorySizeFromDefault; + } + } catch (NumberFormatException ex) { + userLogger.warn( + "Failed to parse memory size '{}', hence, using the default: {}", + podMemorySizeOverride, + podMemorySizeFromDefault); + return podMemorySizeFromDefault; + } + } + + private String getPodMemoryString(double podMemory) { + return ((int) Math.ceil(podMemory * 1024)) + "Mi"; + } } diff --git a/rest/src/main/resources/application-test.yaml b/rest/src/main/resources/application-test.yaml index 8449cd19..a0e582cb 100644 --- a/rest/src/main/resources/application-test.yaml +++ b/rest/src/main/resources/application-test.yaml @@ -3,6 +3,8 @@ quarkus: enabled: false oidc-client: enabled: false + credentials: + secret: oidc-client-secret otel: enabled: false @@ -16,8 +18,9 @@ reqour: reqour-rest: app-environment: test reqour-secret-key: test-secret - indy-url: https://test-indy.example.com - sasl-jaas-conf: config + indy-url: https://test.indy.com + sasl-jaas-conf: sasl-jaas-config + job-definition-file-path: 'src/test/resources/job.yaml' wiremock: base-url: http://localhost:${quarkus.wiremock.devservices.port} diff --git a/rest/src/test/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreatorTest.java b/rest/src/test/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreatorTest.java new file mode 100644 index 00000000..0a073faa --- /dev/null +++ b/rest/src/test/java/org/jboss/pnc/reqour/rest/openshift/JobDefinitionCreatorTest.java @@ -0,0 +1,175 @@ +/* + * Copyright 2024 Red Hat, Inc. + * SPDX-License-Identifier: Apache-2.0 + */ +package org.jboss.pnc.reqour.rest.openshift; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import io.fabric8.kubernetes.api.model.EnvVar; +import io.fabric8.kubernetes.api.model.Quantity; +import io.fabric8.kubernetes.api.model.batch.v1.Job; +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.junit.TestProfile; +import jakarta.inject.Inject; +import org.jboss.pnc.api.constants.BuildConfigurationParameterKeys; +import org.jboss.pnc.api.dto.Request; +import org.jboss.pnc.api.enums.AlignmentPreference; +import org.jboss.pnc.api.enums.BuildType; +import org.jboss.pnc.api.reqour.dto.AdjustRequest; +import org.jboss.pnc.api.reqour.dto.InternalGitRepositoryUrl; +import org.jboss.pnc.reqour.common.TestDataSupplier; +import org.jboss.pnc.reqour.common.profile.WithStaticBifrostUrl; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.slf4j.MDC; + +import java.net.URI; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +@QuarkusTest +@TestProfile(WithStaticBifrostUrl.class) // override bifrost URL just for this test +class JobDefinitionCreatorTest { + + @Inject + JobDefinitionCreator jobDefinitionCreator; + + private static final String ADJUSTER_JOB_NAME = "adjusterJob"; + private static final Map MDC_MAP = Map.of("foo", "bar"); + + @Inject + ObjectMapper objectMapper; + + @BeforeAll + static void setUp() { + MDC.setContextMap(MDC_MAP); + } + + @Test + void getAdjusterJobDefinition() throws JsonProcessingException { + // Arrange + Quantity expectedAdjusterPodMemory = Quantity.parse("12288Mi"); + AdjustRequest adjustRequest = AdjustRequest.builder() + .internalUrl( + InternalGitRepositoryUrl.builder() + .readonlyUrl("https://gitlab.com/org/project.git") + .readonlyUrl("git@gitlab.com:org/project.git") + .build()) + .ref("foo") + .callback( + Request.builder() + .method(Request.Method.POST) + .uri(URI.create("https://callback.example.com")) + .build()) + .originRepoUrl("https://github.com/org/project.git") + .tempBuild(false) + .alignmentPreference(AlignmentPreference.PREFER_PERSISTENT) + .buildConfigParameters(Map.of(BuildConfigurationParameterKeys.ALIGNMENT_POD_MEMORY, "12")) + .buildType(BuildType.MVN) + .brewPullActive(true) + .taskId(TestDataSupplier.TASK_ID) + .build(); + + // Act + Job adjusterJobDefinition = jobDefinitionCreator.getAdjusterJobDefinition(adjustRequest, ADJUSTER_JOB_NAME); + + // Assert + assertThat(adjusterJobDefinition).isNotNull(); + assertThat(adjusterJobDefinition.getMetadata().getName()).isEqualTo(ADJUSTER_JOB_NAME); + + List envVars = adjusterJobDefinition.getSpec() + .getTemplate() + .getSpec() + .getContainers() + .getLast() + .getEnv(); + assertThat(envVars.get(0).getName()).isEqualTo("KAFKA_CLIENT_SECRET_NAME"); + assertThat(envVars.get(0).getValue()).isEqualTo("kafka-client-secret"); + + assertThat(envVars.get(1).getName()).isEqualTo("REQOUR_SECRET_NAME"); + assertThat(envVars.get(1).getValue()).isEqualTo("test-secret"); + + assertThat(envVars.get(2).getName()).isEqualTo("APP_ENV"); + assertThat(envVars.get(2).getValue()).isEqualTo("test"); + + assertThat(envVars.get(3).getName()).isEqualTo("INDY_URL"); + assertThat(envVars.get(3).getValue()).isEqualTo("https://test.indy.com"); + + assertThat(envVars.get(4).getName()).isEqualTo("BIFROST_URL"); + assertThat(envVars.get(4).getValue()).isEqualTo("https://test.bifrost.com"); + + assertThat(envVars.get(5).getName()).isEqualTo("BUILD_TYPE"); + assertThat(envVars.get(5).getValue()).isEqualTo(adjustRequest.getBuildType().toString()); + + assertThat(envVars.get(6).getName()).isEqualTo("ADJUST_REQUEST"); + assertThat(envVars.get(6).getValue()).contains(objectMapper.writeValueAsString(adjustRequest)); + + assertThat(envVars.get(7).getName()).isEqualTo("MDC"); + assertThat(envVars.get(7).getValue()).isEqualTo(objectMapper.writeValueAsString(MDC_MAP)); + + assertThat(envVars.get(8).getName()).isEqualTo("OIDC_CLIENT_CREDENTIALS_SECRET"); + assertThat(envVars.get(8).getValue()).isEqualTo("oidc-client-secret"); + + assertThat(envVars.get(9).getName()).isEqualTo("SASL_JAAS_CONF"); + assertThat(envVars.get(9).getValue()).isEqualTo("sasl-jaas-config"); + + assertThat( + adjusterJobDefinition.getSpec() + .getTemplate() + .getSpec() + .getContainers() + .getLast() + .getResources() + .getLimits() + .get("memory")) + .isEqualTo(expectedAdjusterPodMemory); + assertThat( + adjusterJobDefinition.getSpec() + .getTemplate() + .getSpec() + .getContainers() + .getLast() + .getResources() + .getRequests() + .get("memory")) + .isEqualTo(expectedAdjusterPodMemory); + } + + @Test + void getResourcesMemory_buildConfigsNotPresent_usesDefault() { + Map buildConfigParameters = null; + String expectedAdjusterMemory = "4096Mi"; + + assertThat(jobDefinitionCreator.getResourcesMemory(buildConfigParameters)).isEqualTo(expectedAdjusterMemory); + } + + @Test + void getResourcesMemory_buildConfigsPresentButNoAdjusterMemoryOverride_usesDefault() { + Map buildConfigParameters = Map + .of(BuildConfigurationParameterKeys.BUILDER_POD_MEMORY, "12"); + String expectedAdjusterMemory = "4096Mi"; + + assertThat(jobDefinitionCreator.getResourcesMemory(buildConfigParameters)).isEqualTo(expectedAdjusterMemory); + } + + @Test + void getResourcesMemory_buildConfigsPresentWithInvalidAdjusterMemoryOverride_usesDefault() { + Map buildConfigParameters = Map + .of(BuildConfigurationParameterKeys.ALIGNMENT_POD_MEMORY, "-12"); + String expectedAdjusterMemory = "4096Mi"; + + assertThat(jobDefinitionCreator.getResourcesMemory(buildConfigParameters)).isEqualTo(expectedAdjusterMemory); + } + + @Test + void getResourcesMemory_buildConfigsPresentWithValidAdjusterMemoryOverride_usesOverride() { + Map buildConfigParameters = Map + .of(BuildConfigurationParameterKeys.ALIGNMENT_POD_MEMORY, "12"); + String expectedAdjusterMemory = "12288Mi"; + + assertThat(jobDefinitionCreator.getResourcesMemory(buildConfigParameters)).isEqualTo(expectedAdjusterMemory); + } +} diff --git a/rest/src/test/resources/job.yaml b/rest/src/test/resources/job.yaml new file mode 100644 index 00000000..136ced51 --- /dev/null +++ b/rest/src/test/resources/job.yaml @@ -0,0 +1,63 @@ +apiVersion: batch/v1 +kind: Job +metadata: + name: %{jobName} +spec: + backoffLimit: 1 # After a (potential) failure, do not try any retries + ttlSecondsAfterFinished: 900 # After 15 minutes, delete the job (hence its adjuster pod) + activeDeadlineSeconds: 10800 # Allow the job (hence its adjuster pod) to run for 3 hours max + template: + metadata: + name: %{jobName} # Note: pods of the job will have name: %{jobName}-, these random suffixes cannot be removed, see: https://stackoverflow.com/a/62833249 + # annotations: + # values from Job's metadata.annotations are not deserialized (used within _getAdjusterJobDefinition_ test) properly + # hence, injecting it directly into env through value + # environment: %{appEnvironment} + # kafka-client-secret: kafka-client-secret + # reqour-secret: %{reqourSecretKey} + spec: + containers: + - image: "quay.io/rh-newcastle-devel/reqour-adjuster-main:latest" + name: reqour-adjuster + env: + - name: KAFKA_CLIENT_SECRET_NAME + value: kafka-client-secret + - name: REQOUR_SECRET_NAME + value: '%{reqourSecretKey}' + - name: APP_ENV + value: '%{appEnvironment}' + - name: INDY_URL + value: '%{indyUrl}' + - name: BIFROST_URL + value: '%{bifrostUrl}' + - name: BUILD_TYPE + value: '%{buildType}' + - name: ADJUST_REQUEST + value: > + %{adjustRequest} + - name: MDC + value: '%{mdc}' + - name: OIDC_CLIENT_CREDENTIALS_SECRET + value: '%{saSecret}' + - name: SASL_JAAS_CONF + value: '%{saslJaasConf}' + livenessProbe: + exec: + command: + - '/bin/bash' + - '-c' + - 'true' + readinessProbe: + exec: + command: + - '/bin/bash' + - '-c' + - 'true' + resources: + limits: + cpu: 500m + memory: '%{resourcesMemory}' + requests: + cpu: 500m + memory: '%{resourcesMemory}' + restartPolicy: Never