From 7f3cb4ad569587b348c2f60b317bfab1fa7c53e9 Mon Sep 17 00:00:00 2001 From: tariq-hasan Date: Fri, 5 Apr 2024 00:42:04 -0400 Subject: [PATCH] Redefined errors Signed-off-by: tariq-hasan --- .../experiment/manifest/generator.go | 26 +++++++++---------- .../experiment/manifest/generator_test.go | 16 ++++++------ pkg/webhook/v1beta1/pod/inject_webhook.go | 16 ++++++------ .../v1beta1/pod/inject_webhook_test.go | 10 +++---- pkg/webhook/v1beta1/pod/utils.go | 4 +-- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/pkg/controller.v1beta1/experiment/manifest/generator.go b/pkg/controller.v1beta1/experiment/manifest/generator.go index 5507f4388fd..306f6bd9f4a 100644 --- a/pkg/controller.v1beta1/experiment/manifest/generator.go +++ b/pkg/controller.v1beta1/experiment/manifest/generator.go @@ -35,12 +35,12 @@ import ( ) var ( - ErrConfigMapNotFound = errors.New("ConfigMap not found") - ErrConvertStringToUnstructuredFailed = errors.New("ConvertStringToUnstructured failed") - ErrConvertUnstructuredToStringFailed = errors.New("ConvertUnstructuredToString failed") - ErrParamNotFoundInParameterAssignment = errors.New("unable to find non-meta paramater from TrialParameters in ParameterAssignment") - ErrParamNotFoundInTrialParameters = errors.New("unable to find parameter from ParameterAssignment in TrialParameters") - ErrTemplatePathNotFound = errors.New("TemplatePath not found in ConfigMap") + errConfigMapNotFound = errors.New("configMap not found") + errConvertStringToUnstructuredFailed = errors.New("failed to convert string to unstructured") + errConvertUnstructuredToStringFailed = errors.New("failed to convert unstructured to string") + errParamNotFoundInParameterAssignment = errors.New("unable to find non-meta parameter from TrialParameters in ParameterAssignment") + errParamNotFoundInTrialParameters = errors.New("unable to find parameter from ParameterAssignment in TrialParameters") + errTrialTemplateNotFound = errors.New("unable to find trial template in ConfigMap") ) // Generator is the type for manifests Generator. @@ -96,7 +96,7 @@ func (g *DefaultGenerator) GetRunSpecWithHyperParameters(experiment *experiments // Convert Trial template to unstructured runSpec, err := util.ConvertStringToUnstructured(replacedTemplate) if err != nil { - return nil, fmt.Errorf("%w: %s", ErrConvertStringToUnstructuredFailed, err.Error()) + return nil, fmt.Errorf("%w: %s", errConvertStringToUnstructuredFailed, err.Error()) } // Set name and namespace for Run Spec @@ -118,7 +118,7 @@ func (g *DefaultGenerator) applyParameters(experiment *experimentsv1beta1.Experi if trialSpec == nil { trialSpec, err = util.ConvertStringToUnstructured(trialTemplate) if err != nil { - return "", fmt.Errorf("%w: %s", ErrConvertStringToUnstructuredFailed, err.Error()) + return "", fmt.Errorf("%w: %s", errConvertStringToUnstructuredFailed, err.Error()) } } @@ -141,7 +141,7 @@ func (g *DefaultGenerator) applyParameters(experiment *experimentsv1beta1.Experi nonMetaParamCount += 1 continue } else { - return "", fmt.Errorf("%w: parameter: %v, parameter assignment: %v", ErrParamNotFoundInParameterAssignment, param.Reference, assignmentsMap) + return "", fmt.Errorf("%w: parameter: %v, parameter assignment: %v", errParamNotFoundInParameterAssignment, param.Reference, assignmentsMap) } } metaRefKey = sub[1] @@ -185,7 +185,7 @@ func (g *DefaultGenerator) applyParameters(experiment *experimentsv1beta1.Experi // Number of assignment parameters must be equal to the number of non-meta trial parameters // i.e. all parameters in ParameterAssignment must be in TrialParameters if len(assignments) != nonMetaParamCount { - return "", fmt.Errorf("%w: parameter assignments: %v, non-meta trial parameter count: %v", ErrParamNotFoundInTrialParameters, assignments, nonMetaParamCount) + return "", fmt.Errorf("%w: parameter assignments: %v, non-meta trial parameter count: %v", errParamNotFoundInTrialParameters, assignments, nonMetaParamCount) } // Replacing placeholders with parameter values @@ -205,7 +205,7 @@ func (g *DefaultGenerator) GetTrialTemplate(instance *experimentsv1beta1.Experim if trialSource.TrialSpec != nil { trialTemplateString, err = util.ConvertUnstructuredToString(trialSource.TrialSpec) if err != nil { - return "", fmt.Errorf("%w: %s", ErrConvertUnstructuredToStringFailed, err.Error()) + return "", fmt.Errorf("%w: %s", errConvertUnstructuredToStringFailed, err.Error()) } } else { configMapNS := trialSource.ConfigMap.ConfigMapNamespace @@ -213,12 +213,12 @@ func (g *DefaultGenerator) GetTrialTemplate(instance *experimentsv1beta1.Experim templatePath := trialSource.ConfigMap.TemplatePath configMap, err := g.client.GetConfigMap(configMapName, configMapNS) if err != nil { - return "", fmt.Errorf("%w: %v", ErrConfigMapNotFound, err) + return "", fmt.Errorf("%w: %v", errConfigMapNotFound, err) } var ok bool trialTemplateString, ok = configMap[templatePath] if !ok { - return "", fmt.Errorf("%w: TemplatePath: %v, ConfigMap: %v", ErrTemplatePathNotFound, templatePath, configMap) + return "", fmt.Errorf("%w: TemplatePath: %v, ConfigMap: %v", errTrialTemplateNotFound, templatePath, configMap) } } diff --git a/pkg/controller.v1beta1/experiment/manifest/generator_test.go b/pkg/controller.v1beta1/experiment/manifest/generator_test.go index 25f374e6f86..9f89f5ac5f4 100644 --- a/pkg/controller.v1beta1/experiment/manifest/generator_test.go +++ b/pkg/controller.v1beta1/experiment/manifest/generator_test.go @@ -109,7 +109,7 @@ func TestGetRunSpecWithHP(t *testing.T) { return i }(), ParameterAssignments: newFakeParameterAssignment(), - wantError: ErrConvertUnstructuredToStringFailed, + wantError: errConvertUnstructuredToStringFailed, }, "Non-meta parameter from TrialParameters not found in ParameterAssignment": { Instance: newFakeInstance(), @@ -121,7 +121,7 @@ func TestGetRunSpecWithHP(t *testing.T) { } return pa }(), - wantError: ErrParamNotFoundInParameterAssignment, + wantError: errParamNotFoundInParameterAssignment, }, // case in which the lengths of trial parameters and parameter assignments are different "Parameter from ParameterAssignment not found in TrialParameters": { @@ -134,7 +134,7 @@ func TestGetRunSpecWithHP(t *testing.T) { }) return pa }(), - wantError: ErrParamNotFoundInTrialParameters, + wantError: errParamNotFoundInTrialParameters, }, } @@ -200,7 +200,7 @@ spec: map[string]string{templatePath: trialSpec}, nil) invalidConfigMapName := c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return( - nil, ErrConfigMapNotFound) + nil, errConfigMapNotFound) validGetConfigMap3 := c.EXPECT().GetConfigMap(gomock.Any(), gomock.Any()).Return( map[string]string{templatePath: trialSpec}, nil) @@ -237,7 +237,7 @@ spec: expectedRunSpec, err := util.ConvertStringToUnstructured(expectedStr) if err != nil { - t.Errorf("%v: %v", ErrConvertStringToUnstructuredFailed, err) + t.Errorf("%v: %v", errConvertStringToUnstructuredFailed, err) } cases := map[string]struct { @@ -274,7 +274,7 @@ spec: return i }(), ParameterAssignments: newFakeParameterAssignment(), - wantError: ErrConfigMapNotFound, + wantError: errConfigMapNotFound, }, // validGetConfigMap3 case "Invalid template path in ConfigMap name": { @@ -290,7 +290,7 @@ spec: return i }(), ParameterAssignments: newFakeParameterAssignment(), - wantError: ErrTemplatePathNotFound, + wantError: errTrialTemplateNotFound, }, // invalidTemplate case // Trial template is a string in ConfigMap @@ -308,7 +308,7 @@ spec: return i }(), ParameterAssignments: newFakeParameterAssignment(), - wantError: ErrConvertStringToUnstructuredFailed, + wantError: errConvertStringToUnstructuredFailed, }, } diff --git a/pkg/webhook/v1beta1/pod/inject_webhook.go b/pkg/webhook/v1beta1/pod/inject_webhook.go index 2f393dc6763..b7daccd637c 100644 --- a/pkg/webhook/v1beta1/pod/inject_webhook.go +++ b/pkg/webhook/v1beta1/pod/inject_webhook.go @@ -49,10 +49,10 @@ import ( var log = logf.Log.WithName("injector-webhook") var ( - ErrInvalidOwnerAPIVersion = errors.New("invalid owner API version") - ErrInvaidSuggestionName = errors.New("invalid suggestion name") - ErrPodNotBelongToKatibJob = errors.New("pod does not belong to Katib Job") - ErrNestedObjectNotFound = errors.New("nested object not found in the cluster") + errInvalidOwnerAPIVersion = errors.New("invalid owner API version") + errInvalidSuggestionName = errors.New("invalid suggestion name") + errPodNotBelongToKatibJob = errors.New("pod does not belong to Katib Job") + errNestedObjectNotFound = errors.New("nested object not found in the cluster") ) // SidecarInjector injects metrics collect sidecar to the primary pod. @@ -267,7 +267,7 @@ func (s *SidecarInjector) getKatibJob(object *unstructured.Unstructured, namespa // Get group and version from owner API version gv, err := schema.ParseGroupVersion(owners[i].APIVersion) if err != nil { - return "", "", fmt.Errorf("%w: %s", ErrInvalidOwnerAPIVersion, err.Error()) + return "", "", fmt.Errorf("%w: %s", errInvalidOwnerAPIVersion, err.Error()) } gvk := schema.GroupVersionKind{ Group: gv.Group, @@ -280,7 +280,7 @@ func (s *SidecarInjector) getKatibJob(object *unstructured.Unstructured, namespa // Nested object namespace must be equal to object namespace err = s.client.Get(context.TODO(), apitypes.NamespacedName{Name: owners[i].Name, Namespace: namespace}, nestedJob) if err != nil { - return "", "", fmt.Errorf("%w: %s", ErrNestedObjectNotFound, err.Error()) + return "", "", fmt.Errorf("%w: %s", errNestedObjectNotFound, err.Error()) } // Recursively search for Trial ownership in nested object jobKind, jobName, err = s.getKatibJob(nestedJob, namespace) @@ -293,7 +293,7 @@ func (s *SidecarInjector) getKatibJob(object *unstructured.Unstructured, namespa // If jobKind is empty after the loop, Trial doesn't own the object if jobKind == "" { - return "", "", ErrPodNotBelongToKatibJob + return "", "", errPodNotBelongToKatibJob } return jobKind, jobName, nil @@ -330,7 +330,7 @@ func (s *SidecarInjector) getMetricsCollectorArgs(trial *trialsv1beta1.Trial, me suggestion := &suggestionsv1beta1.Suggestion{} err := s.client.Get(context.TODO(), apitypes.NamespacedName{Name: suggestionName, Namespace: trial.Namespace}, suggestion) if err != nil { - return nil, fmt.Errorf("%w: %s", ErrInvaidSuggestionName, err.Error()) + return nil, fmt.Errorf("%w: %s", errInvalidSuggestionName, err.Error()) } args = append(args, "-s-earlystop", util.GetEarlyStoppingEndpoint(suggestion)) } diff --git a/pkg/webhook/v1beta1/pod/inject_webhook_test.go b/pkg/webhook/v1beta1/pod/inject_webhook_test.go index 426b401a793..4eec6a63ac1 100644 --- a/pkg/webhook/v1beta1/pod/inject_webhook_test.go +++ b/pkg/webhook/v1beta1/pod/inject_webhook_test.go @@ -161,7 +161,7 @@ func TestWrapWorkerContainer(t *testing.T) { }, }, PathKind: common.FileKind, - WantError: ErrPrimaryContainerNotFound, + WantError: errPrimaryContainerNotFound, }, "Container with early stopping command": { Trial: func() *trialsv1beta1.Trial { @@ -504,7 +504,7 @@ func TestGetMetricsCollectorArgs(t *testing.T) { }, EarlyStoppingRules: earlyStoppingRules, KatibConfig: configv1beta1.MetricsCollectorConfig{}, - WantError: ErrInvaidSuggestionName, + WantError: errInvalidSuggestionName, }, } @@ -863,7 +863,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - WantError: ErrPodNotBelongToKatibJob, + WantError: errPodNotBelongToKatibJob, }, "Run when Pod owns Job that doesn't exists": { Pod: &v1.Pod{ @@ -879,7 +879,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - WantError: ErrNestedObjectNotFound, + WantError: errNestedObjectNotFound, }, "Run when Pod owns Job with invalid API version": { Pod: &v1.Pod{ @@ -895,7 +895,7 @@ func TestGetKatibJob(t *testing.T) { }, }, }, - WantError: ErrInvalidOwnerAPIVersion, + WantError: errInvalidOwnerAPIVersion, }, } diff --git a/pkg/webhook/v1beta1/pod/utils.go b/pkg/webhook/v1beta1/pod/utils.go index 9349dea8d4c..6a25a2441e7 100644 --- a/pkg/webhook/v1beta1/pod/utils.go +++ b/pkg/webhook/v1beta1/pod/utils.go @@ -36,7 +36,7 @@ import ( mccommon "github.com/kubeflow/katib/pkg/metricscollector/v1beta1/common" ) -var ErrPrimaryContainerNotFound = errors.New("unable to find primary container in mutated pod containers") +var errPrimaryContainerNotFound = errors.New("unable to find primary container in mutated pod containers") func isPrimaryPod(podLabels, primaryLabels map[string]string) bool { @@ -193,7 +193,7 @@ func wrapWorkerContainer(trial *trialsv1beta1.Trial, pod *v1.Pod, namespace, c.Command = command c.Args = []string{argsStr} } else { - return fmt.Errorf("%w: primary container: %v, mutated pod containers: %v", ErrPrimaryContainerNotFound, trial.Spec.PrimaryContainerName, pod.Spec.Containers) + return fmt.Errorf("%w: primary container: %v, mutated pod containers: %v", errPrimaryContainerNotFound, trial.Spec.PrimaryContainerName, pod.Spec.Containers) } return nil }