Skip to content

Commit

Permalink
Merge pull request #1057 from khansaad/update-results-api-validation
Browse files Browse the repository at this point in the history 
Add validations for the UpdateResultsAPI objects
  • Loading branch information
@dinogun
dinogun authored Jan 30, 2024
2 parents 6149211 + 0869ce7 commit fc881e5
Show file tree
Hide file tree
Showing 12 changed files with 187 additions and 63 deletions.
61 changes: 59 additions & 2 deletions design/UpdateResults.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ tuned.
* Experiment name not found.
```
{
"message": "Experiment name: <experiment-name> not found",
"message": "Not Found: experiment_name does not exist: <experiment-name>",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
Expand All @@ -40,7 +40,7 @@ tuned.
* Duplicate Experiment name and Timestamp.
```
{
"message": "Experiment name : <experiment-name> already contains result for timestamp : <timestamp>",
"message": "An entry for this record already exists!",
"httpcode": 409,
"documentationLink": "",
"status": "ERROR"
Expand All @@ -54,6 +54,63 @@ tuned.
"documentationLink": "",
"status": "ERROR"
}
```
* Invalid Input JSON Format.
* For example, `interval_start_time` or `interval_end_time` is not following the correct UTC format or it's blank:
```
{
"message": "Failed to parse the JSON. Please check the input payload ",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
}
```
* Parameters Mismatch
* `version` name passed is different from what was passed while creating the corresponding experiment:
```
{
"message": "Version number mismatch found. Expected: <existing-version> , Found: <new-version>",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
}
```
* `namespace` in the `kubernetes_objects` is different from what was passed while creating the corresponding experiment:
```
{
"message": "kubernetes_objects : Kubernetes Object Namespaces MisMatched. Expected Namespace: <existing-ns>, Found: <new-ns> in Results for experiment: <experiment-name> ",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
}
```
* Similar response will be returned for other parameters when there is a mismatch.
* Invalid Metric variable names.
```
{
"message": "Performance profile: [Metric variable name should be among these values: [cpuRequest, cpuLimit, cpuUsage, cpuThrottle, memoryRequest, memoryLimit, memoryUsage, memoryRSS] for container : <container-name> for experiment: <exp-name>]",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
}
```
* Invalid Aggregation_Info Format
```
{
"message": "Performance profile: [ Format value should be among these values: [GiB, Gi, Ei, KiB, E, MiB, G, PiB, K, TiB, M, P, Bytes, cores, T, Ti, MB, KB, Pi, GB, EB, k, m, TB, PB, bytes, kB, Mi, Ki, EiB] for container : <container-name> for experiment: <exp-name>]",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
}
```
* Invalid Aggregation_Info Values
```
{
"message": "Invalid value type for aggregation_info objects. Expected a numeric value (Double).",
"httpcode": 400,
"documentationLink": "",
"status": "ERROR"
}
```
* Any unknown exception on server side
```
Expand Down
39 changes: 33 additions & 6 deletions src/main/java/com/autotune/analyzer/experiment/ExperimentInitiator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,16 +136,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%s", MISSING_EXPERIMENT_NAME, null));
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 +161,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 +191,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 +214,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
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
3 changes: 0 additions & 3 deletions ...autotune/analyzer/serviceObjects/verification/validators/KubernetesElementsValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
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.ExperimentResultData;
import jakarta.validation.ConstraintValidator;
import jakarta.validation.ConstraintValidatorContext;
Expand All @@ -45,7 +43,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
7 changes: 4 additions & 3 deletions ...autotune/analyzer/serviceObjects/verification/validators/PerformanceProfileValidator.java
View file
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

0 comments on commit fc881e5

Please sign in to comment.