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.

Sign up for GitHub

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

Add validations for the UpdateResultsAPI objects #1057

Merged
merged 7 commits into from
Jan 30, 2024
+187 −63
Merged

Add validations for the UpdateResultsAPI objects #1057

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ecf8b21
add validations for the UpdateResultsAPI objects
khansaad Nov 23, 2023
31ff3d8
updates added for more validation
khansaad Nov 27, 2023
39d972f
updates wrt to the container and container image validations, bug fixes
khansaad Dec 1, 2023
86a4711
update parsing failure validations
khansaad Dec 5, 2023
d9ae044
fix failures and changes as per review
khansaad Jan 8, 2024
258b0bf
update docs and other changes as per the review
khansaad Jan 30, 2024
0869ce7
fix conflicts
khansaad Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions src/main/java/com/autotune/analyzer/experiment/ExperimentInitiator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;

import static com.autotune.analyzer.utils.AnalyzerErrorConstants.AutotuneObjectErrors.MISSING_EXPERIMENT;
import static com.autotune.analyzer.utils.AnalyzerErrorConstants.AutotuneObjectErrors.MISSING_EXPERIMENT_NAME;

/**
Expand Down Expand Up @@ -136,16 +137,23 @@ public void generateAndAddRecommendations(KruizeObject kruizeObject, List<Experi
}

public void validateAndAddExperimentResults(List<UpdateResultsAPIObject> updateResultsAPIObjects) {
List<UpdateResultsAPIObject> failedDBObjects = new ArrayList<>();
List<UpdateResultsAPIObject> failedDBObjects;
Validator validator = Validation.byProvider(HibernateValidator.class)
.configure()
.messageInterpolator(new ParameterMessageInterpolator())
.failFast(true)
.buildValidatorFactory()
.getValidator();
Map<String, KruizeObject> mainKruizeExperimentMAP = new ConcurrentHashMap<String, KruizeObject>();
Map<String, KruizeObject> mainKruizeExperimentMAP = new ConcurrentHashMap<>();
List<String> errorReasons = new ArrayList<>();
for (UpdateResultsAPIObject object : updateResultsAPIObjects) {
String experimentName = object.getExperimentName();
if (experimentName == null) {
errorReasons.add(String.format("%s", MISSING_EXPERIMENT));
object.setErrors(getErrorMap(errorReasons));
failedUpdateResultsAPIObjects.add(object);
continue;
}
if (!mainKruizeExperimentMAP.containsKey(experimentName)) {
try {
new ExperimentDBService().loadExperimentFromDBByName(mainKruizeExperimentMAP, experimentName); // TODO try to avoid DB
Expand All @@ -154,17 +162,25 @@ public void validateAndAddExperimentResults(List<UpdateResultsAPIObject> updateR
}
}
if (mainKruizeExperimentMAP.containsKey(experimentName)) {

// check version
String errorMsg = checkVersion(object, mainKruizeExperimentMAP);
if (errorMsg != null) {
errorReasons.add(errorMsg);
object.setErrors(getErrorMap(errorReasons));
failedUpdateResultsAPIObjects.add(object);
continue;
}
object.setKruizeObject(mainKruizeExperimentMAP.get(object.getExperimentName()));
Set<ConstraintViolation<UpdateResultsAPIObject>> violations = new HashSet<>();
try {
violations = validator.validate(object, UpdateResultsAPIObject.FullValidationSequence.class);
if (violations.isEmpty()) {
successUpdateResultsAPIObjects.add(object);
} else {
List<String> errorReasons = new ArrayList<>();
for (ConstraintViolation<UpdateResultsAPIObject> violation : violations) {
String propertyPath = violation.getPropertyPath().toString();
if (null != propertyPath && propertyPath.length() != 0) {
if (null != propertyPath && !propertyPath.isEmpty()) {
errorReasons.add(getSerializedName(propertyPath, UpdateResultsAPIObject.class) + ": " + violation.getMessage());
} else {
errorReasons.add(violation.getMessage());
Expand All @@ -176,13 +192,11 @@ public void validateAndAddExperimentResults(List<UpdateResultsAPIObject> updateR
} catch (Exception e) {
LOGGER.debug(e.getMessage());
e.printStackTrace();
List<String> errorReasons = new ArrayList<>();
errorReasons.add(String.format("%s%s", e.getMessage(), experimentName));
object.setErrors(getErrorMap(errorReasons));
failedUpdateResultsAPIObjects.add(object);
}
} else {
List<String> errorReasons = new ArrayList<>();
errorReasons.add(String.format("%s%s", MISSING_EXPERIMENT_NAME, experimentName));
object.setErrors(getErrorMap(errorReasons));
failedUpdateResultsAPIObjects.add(object);
Expand All @@ -201,6 +215,20 @@ public void validateAndAddExperimentResults(List<UpdateResultsAPIObject> updateR
}
}

private String checkVersion(UpdateResultsAPIObject object, Map<String, KruizeObject> mainKruizeExperimentMAP) {
try {
KruizeObject kruizeObject = mainKruizeExperimentMAP.get(object.getExperimentName());
if (!object.getApiVersion().equals(kruizeObject.getApiVersion())) {
return String.format(AnalyzerErrorConstants.AutotuneObjectErrors.VERSION_MISMATCH,
kruizeObject.getApiVersion(), object.getApiVersion());
}
} catch (Exception e) {
LOGGER.error("Exception occurred while checking version: {}", e.getMessage());
return null;
}
return null;
}

public String getSerializedName(String fieldName, Class<?> targetClass) {
Class<?> currentClass = targetClass;
while (currentClass != null) {
Expand Down
50 changes: 29 additions & 21 deletions src/main/java/com/autotune/analyzer/performanceProfiles/utils/PerformanceProfileUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,9 @@ public static ValidationOutputData validateAndAddProfile(Map<String, Performance
* @param updateResultsAPIObject
* @return
*/
public static String validateResults(PerformanceProfile performanceProfile, UpdateResultsAPIObject updateResultsAPIObject) {
public static List<String> validateResults(PerformanceProfile performanceProfile, UpdateResultsAPIObject updateResultsAPIObject) {

List<String> errorReasons = new ArrayList<>();
String errorMsg = "";
List<AnalyzerConstants.MetricName> mandatoryFields = Arrays.asList(
AnalyzerConstants.MetricName.cpuUsage,
Expand All @@ -87,12 +88,12 @@ public static String validateResults(PerformanceProfile performanceProfile, Upda
queryList.add(metric.getQuery());
}

// Get the metrics data from the Kruize Object
// Get the metrics data from the Kruize Object and validate it
for (KubernetesAPIObject kubernetesAPIObject : updateResultsAPIObject.getKubernetesObjects()) {
for (ContainerAPIObject containerAPIObject : kubernetesAPIObject.getContainerAPIObjects()) {
// if the metrics data is not present, set corresponding validation message and skip adding the current container data
if (containerAPIObject.getMetrics() == null) {
errorMsg = errorMsg.concat(String.format(
errorReasons.add(String.format(
AnalyzerErrorConstants.AutotuneObjectErrors.MISSING_METRICS,
containerAPIObject.getContainer_name(),
updateResultsAPIObject.getExperimentName()
Expand All @@ -106,47 +107,54 @@ public static String validateResults(PerformanceProfile performanceProfile, Upda
// validate the metric values
errorMsg = PerformanceProfileUtil.validateMetricsValues(metric.getName(), metric.getMetricResult());
if (!errorMsg.isBlank()) {
errorReasons.add(errorMsg.concat(String.format(
AnalyzerErrorConstants.AutotuneObjectErrors.CONTAINER_AND_EXPERIMENT,
containerAPIObject.getContainer_name(),
updateResultsAPIObject.getExperimentName())));
break;
}
AnalyzerConstants.MetricName metricName = AnalyzerConstants.MetricName.valueOf(metric.getName());
kruizeFunctionVariablesList.add(metricName);
MetricResults metricResults = metric.getMetricResult();
Map<String, Object> aggrInfoClassAsMap;
if (!perfProfileAggrFunctions.isEmpty()) {
try {
aggrInfoClassAsMap = convertObjectToMap(metricResults.getAggregationInfoResult());
errorMsg = validateAggFunction(aggrInfoClassAsMap, perfProfileAggrFunctions);
if (!errorMsg.isBlank()) {
errorMsg = errorMsg.concat(String.format(" for the experiment : %s"
, updateResultsAPIObject.getExperimentName()));
break;
}
} catch(IllegalAccessException | InvocationTargetException e){
throw new RuntimeException(e);
}
} else{
// check if query is also absent
if (queryList.isEmpty()) {
errorMsg = AnalyzerErrorConstants.AutotuneObjectErrors.QUERY_FUNCTION_MISSING;
try {
aggrInfoClassAsMap = convertObjectToMap(metricResults.getAggregationInfoResult());
errorMsg = validateAggFunction(aggrInfoClassAsMap, perfProfileAggrFunctions);
if (!errorMsg.isBlank()) {
errorReasons.add(errorMsg.concat(String.format(
AnalyzerErrorConstants.AutotuneObjectErrors.CONTAINER_AND_EXPERIMENT,
containerAPIObject.getContainer_name(),
updateResultsAPIObject.getExperimentName())));
break;
}
} catch(IllegalAccessException | InvocationTargetException e){
throw new RuntimeException(e);
}
} else{
// check if query is also absent
if (queryList.isEmpty()) {
errorReasons.add(AnalyzerErrorConstants.AutotuneObjectErrors.QUERY_FUNCTION_MISSING);
break;
}
}
} catch (IllegalArgumentException e) {
LOGGER.error("Error occurred in metrics validation: " + errorMsg);
}
}
if (!errorMsg.isBlank())
if (!errorReasons.isEmpty())
break;

LOGGER.debug("perfProfileFunctionVariablesList: {}", perfProfileFunctionVariablesList);
LOGGER.debug("kruizeFunctionVariablesList: {}", kruizeFunctionVariablesList);
if (!new HashSet<>(kruizeFunctionVariablesList).containsAll(mandatoryFields)) {
errorMsg = errorMsg.concat(String.format("Missing one of the following mandatory parameters for experiment - %s : %s", updateResultsAPIObject.getExperimentName(), mandatoryFields));
errorReasons.add(errorMsg.concat(String.format("Missing one of the following mandatory parameters for experiment - %s : %s",
updateResultsAPIObject.getExperimentName(), mandatoryFields)));
break;
}
}
}
return errorMsg;
return errorReasons;
}

public static void addPerformanceProfile(Map<String, PerformanceProfile> performanceProfileMap, PerformanceProfile performanceProfile) {
Expand Down
31 changes: 28 additions & 3 deletions ...autotune/analyzer/serviceObjects/verification/validators/KubernetesElementsValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,22 @@
package com.autotune.analyzer.serviceObjects.verification.validators;

import com.autotune.analyzer.kruizeObject.KruizeObject;
import com.autotune.analyzer.performanceProfiles.PerformanceProfile;
import com.autotune.analyzer.serviceObjects.Converters;
import com.autotune.analyzer.serviceObjects.UpdateResultsAPIObject;
import com.autotune.analyzer.serviceObjects.verification.annotators.KubernetesElementsCheck;
import com.autotune.analyzer.services.UpdateResults;
import com.autotune.common.data.result.ContainerData;
import com.autotune.common.data.result.ExperimentResultData;
import com.autotune.common.k8sObjects.K8sObject;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class KubernetesElementsValidator implements ConstraintValidator<KubernetesElementsCheck, UpdateResultsAPIObject> {
private static final Logger LOGGER = LoggerFactory.getLogger(KubernetesElementsValidator.class);
Expand All @@ -45,7 +48,6 @@ public boolean isValid(UpdateResultsAPIObject updateResultsAPIObject, Constraint
String errorMessage = "";
try {
KruizeObject kruizeObject = updateResultsAPIObject.getKruizeObject();
PerformanceProfile performanceProfile = UpdateResults.performanceProfilesMap.get(kruizeObject.getPerformanceProfile());
ExperimentResultData resultData = Converters.KruizeObjectConverters.convertUpdateResultsAPIObjToExperimentResultData(updateResultsAPIObject);
String expName = kruizeObject.getExperimentName();
String errorMsg = "";
Expand Down Expand Up @@ -96,6 +98,29 @@ public boolean isValid(UpdateResultsAPIObject updateResultsAPIObject, Constraint
));
}

// Validate container names and image names
List<ContainerData> kruizeContainers = kruizeObject.getKubernetes_objects().get(0).getContainerDataMap().values().stream().toList();
List<ContainerData> resultContainers = resultData.getKubernetes_objects().get(0).getContainerDataMap().values().stream().toList();
if (kruizeContainers.size() != resultContainers.size()) {
kubeObjsMisMatch = true;
errorMsg = errorMsg.concat(
String.format("Containers count MisMatched. Expected: %s, Found: %s in Results for experiment: %s \n",
kruizeContainers.size(), resultContainers.size(), expName));
} else {
for (int i = 0; i < kruizeContainers.size(); i++) {
ContainerData kruizeContainer = kruizeContainers.get(i);
ContainerData resultContainer = resultContainers.get(i);
if (!kruizeContainer.getContainer_name().equals(resultContainer.getContainer_name())
|| !kruizeContainer.getContainer_image_name().equals(resultContainer.getContainer_image_name())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest that we skip the image name check for now. If the container image name has changed owing to a application upgrade, our code needs to keep track of that. We cannot penalize the updateResults API for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

kubeObjsMisMatch = true;
errorMsg = errorMsg.concat(
String.format("Container names or image names MisMatched. Expected: %s - %s, Found: %s - %s in Results for experiment: %s \n",
kruizeContainer.getContainer_name(), kruizeContainer.getContainer_image_name(),
resultContainer.getContainer_name(), resultContainer.getContainer_image_name(), expName));
}
}
}

if (kubeObjsMisMatch) {
context.disableDefaultConstraintViolation();
context.buildConstraintViolationWithTemplate(errorMsg)
Expand Down
7 changes: 4 additions & 3 deletions ...autotune/analyzer/serviceObjects/verification/validators/PerformanceProfileValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;

import static com.autotune.analyzer.utils.AnalyzerErrorConstants.AutotuneObjectErrors.MISSING_PERF_PROFILE;
Expand Down Expand Up @@ -63,12 +64,12 @@ public boolean isValid(UpdateResultsAPIObject updateResultsAPIObject, Constraint
}

// validate the results value present in the updateResultsAPIObject
String errorMsg = PerformanceProfileUtil.validateResults(performanceProfile, updateResultsAPIObject);
if (null == errorMsg || errorMsg.isEmpty()) {
List<String> errorMsg = PerformanceProfileUtil.validateResults(performanceProfile, updateResultsAPIObject);
if (errorMsg.isEmpty()) {
success = true;
} else {
context.disableDefaultConstraintViolation();
context.buildConstraintViolationWithTemplate(errorMsg)
context.buildConstraintViolationWithTemplate(errorMsg.toString())
.addPropertyNode("Performance profile")
.addConstraintViolation();
}
Expand Down
Loading
Loading