Skip to content

Commit

Permalink
Redefined errors
Browse files Browse the repository at this point in the history
Signed-off-by: tariq-hasan <[email protected]>
  • Loading branch information
tariq-hasan committed Apr 7, 2024
1 parent e26b967 commit 7f3cb4a
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 36 deletions.
26 changes: 13 additions & 13 deletions pkg/controller.v1beta1/experiment/manifest/generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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())
}
}

Expand All @@ -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]
Expand Down Expand Up @@ -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
Expand All @@ -205,20 +205,20 @@ 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
configMapName := trialSource.ConfigMap.ConfigMapName
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)
}
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/controller.v1beta1/experiment/manifest/generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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": {
Expand All @@ -134,7 +134,7 @@ func TestGetRunSpecWithHP(t *testing.T) {
})
return pa
}(),
wantError: ErrParamNotFoundInTrialParameters,
wantError: errParamNotFoundInTrialParameters,
},
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -274,7 +274,7 @@ spec:
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
wantError: ErrConfigMapNotFound,
wantError: errConfigMapNotFound,
},
// validGetConfigMap3 case
"Invalid template path in ConfigMap name": {
Expand All @@ -290,7 +290,7 @@ spec:
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
wantError: ErrTemplatePathNotFound,
wantError: errTrialTemplateNotFound,
},
// invalidTemplate case
// Trial template is a string in ConfigMap
Expand All @@ -308,7 +308,7 @@ spec:
return i
}(),
ParameterAssignments: newFakeParameterAssignment(),
wantError: ErrConvertStringToUnstructuredFailed,
wantError: errConvertStringToUnstructuredFailed,
},
}

Expand Down
16 changes: 8 additions & 8 deletions pkg/webhook/v1beta1/pod/inject_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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))
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/webhook/v1beta1/pod/inject_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -504,7 +504,7 @@ func TestGetMetricsCollectorArgs(t *testing.T) {
},
EarlyStoppingRules: earlyStoppingRules,
KatibConfig: configv1beta1.MetricsCollectorConfig{},
WantError: ErrInvaidSuggestionName,
WantError: errInvalidSuggestionName,
},
}

Expand Down Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -895,7 +895,7 @@ func TestGetKatibJob(t *testing.T) {
},
},
},
WantError: ErrInvalidOwnerAPIVersion,
WantError: errInvalidOwnerAPIVersion,
},
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/webhook/v1beta1/pod/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 7f3cb4a

Please sign in to comment.