diff --git a/cmd/metricscollector/v1beta1/file-metricscollector/main.go b/cmd/metricscollector/v1beta1/file-metricscollector/main.go index 9b9f1b481b6..49476bd4db7 100644 --- a/cmd/metricscollector/v1beta1/file-metricscollector/main.go +++ b/cmd/metricscollector/v1beta1/file-metricscollector/main.go @@ -159,7 +159,7 @@ func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string) { // Check that metric file exists. checkMetricFile(mFile) - // Get Main proccess. + // Get Main process. _, mainProcPid, err := common.GetMainProcesses(mFile) if err != nil { klog.Fatalf("GetMainProcesses failed: %v", err) @@ -271,7 +271,7 @@ func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string) { klog.Fatalf("Write to file %v error: %v", markFile, err) } - // Get child proccess from main PID. + // Get child process from main PID. childProc, err := mainProc.Children() if err != nil { klog.Fatalf("Get children proceses for main PID: %v failed: %v", mainProcPid, err) @@ -291,7 +291,7 @@ func watchMetricsFile(mFile string, stopRules stopRulesFlag, filters []string) { // Report metrics to DB. reportMetrics(filters) - // Wait until main proccess is completed. + // Wait until main process is completed. timeout := 60 * time.Second endTime := time.Now().Add(timeout) isProcRunning := true diff --git a/pkg/controller.v1beta1/experiment/manifest/generator_test.go b/pkg/controller.v1beta1/experiment/manifest/generator_test.go index 17dd2adffea..7bfcff4eb30 100644 --- a/pkg/controller.v1beta1/experiment/manifest/generator_test.go +++ b/pkg/controller.v1beta1/experiment/manifest/generator_test.go @@ -126,7 +126,7 @@ func TestGetRunSpecWithHP(t *testing.T) { Err: true, testDescription: "Number of parameter assignments is not equal to number of Trial parameters", }, - // Parameter from assignments not found in Trial paramters + // Parameter from assignments not found in Trial parameters { Instance: newFakeInstance(), ParameterAssignments: func() []commonapiv1beta1.ParameterAssignment { diff --git a/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go b/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go index 732c2bfc3a5..4e29636e5ef 100644 --- a/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go +++ b/pkg/controller.v1beta1/suggestion/suggestion_controller_test.go @@ -144,13 +144,12 @@ func TestReconcile(t *testing.T) { suggestionDeploy := &appsv1.Deployment{} // Expect that deployment with appropriate name and image is created - g.Eventually(func() bool { + g.Eventually(func() string { if err = c.Get(context.TODO(), types.NamespacedName{Namespace: namespace, Name: resourceName}, suggestionDeploy); err != nil { - return false + return "" } - return len(suggestionDeploy.Spec.Template.Spec.Containers) > 0 && - suggestionDeploy.Spec.Template.Spec.Containers[0].Image == suggestionImage - }, timeout).Should(gomega.BeTrue()) + return suggestionDeploy.Spec.Template.Spec.Containers[0].Image + }, timeout).Should(gomega.Equal(suggestionImage)) // Expect that service with appropriate name is created g.Eventually(func() error { @@ -300,8 +299,10 @@ func newFakeInstance() *suggestionsv1beta1.Suggestion { func newKatibConfigMapInstance() *corev1.ConfigMap { // Create suggestion config - suggestionConfig := map[string]map[string]string{ - "random": {"image": suggestionImage}, + suggestionConfig := map[string]katibconfig.SuggestionConfig{ + "random": { + Image: suggestionImage, + }, } bSuggestionConfig, _ := json.Marshal(suggestionConfig) diff --git a/pkg/new-ui/v1beta1/util.go b/pkg/new-ui/v1beta1/util.go index 7d49a15cd01..632bb249b8e 100644 --- a/pkg/new-ui/v1beta1/util.go +++ b/pkg/new-ui/v1beta1/util.go @@ -255,7 +255,7 @@ func generateNNImage(architecture string, decoder string) string { Embeding is a map: int to Parameter Parameter has id, type, Option - Beforehand substite all ' to " and wrap the string in ` + Beforehand substitute all ' to " and wrap the string in ` */ replacedDecoder := strings.Replace(decoder, `'`, `"`, -1) diff --git a/pkg/ui/v1beta1/util.go b/pkg/ui/v1beta1/util.go index a0f09b91f54..900660432bf 100644 --- a/pkg/ui/v1beta1/util.go +++ b/pkg/ui/v1beta1/util.go @@ -242,7 +242,7 @@ func generateNNImage(architecture string, decoder string) string { Embeding is a map: int to Parameter Parameter has id, type, Option - Beforehand substite all ' to " and wrap the string in ` + Beforehand substitute all ' to " and wrap the string in ` */ replacedDecoder := strings.Replace(decoder, `'`, `"`, -1) diff --git a/pkg/util/v1beta1/katibconfig/config.go b/pkg/util/v1beta1/katibconfig/config.go index 36602d812fa..8eed895ceee 100644 --- a/pkg/util/v1beta1/katibconfig/config.go +++ b/pkg/util/v1beta1/katibconfig/config.go @@ -19,7 +19,7 @@ package katibconfig import ( "context" "encoding/json" - "errors" + "fmt" "strings" corev1 "k8s.io/api/core/v1" @@ -44,6 +44,12 @@ type SuggestionConfig struct { PersistentVolumeLabels map[string]string `json:"persistentVolumeLabels,omitempty"` } +// EarlyStoppingConfig is the JSON early stopping structure in Katib config. +type EarlyStoppingConfig struct { + Image string `json:"image"` + ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` +} + // MetricsCollectorConfig is the JSON metrics collector structure in Katib config. type MetricsCollectorConfig struct { Image string `json:"image"` @@ -52,12 +58,6 @@ type MetricsCollectorConfig struct { WaitAllProcesses *bool `json:"waitAllProcesses,omitempty"` } -// EarlyStoppingConfig is the JSON early stopping structure in Katib config. -type EarlyStoppingConfig struct { - Image string `json:"image"` - ImagePullPolicy corev1.PullPolicy `json:"imagePullPolicy,omitempty"` -} - // GetSuggestionConfigData gets the config data for the given suggestion algorithm name. func GetSuggestionConfigData(algorithmName string, client client.Client) (SuggestionConfig, error) { configMap := &corev1.ConfigMap{} @@ -73,7 +73,7 @@ func GetSuggestionConfigData(algorithmName string, client client.Client) (Sugges // Try to find suggestion data in config map config, ok := configMap.Data[consts.LabelSuggestionTag] if !ok { - return SuggestionConfig{}, errors.New("failed to find suggestions config in ConfigMap: " + consts.KatibConfigMapName) + return SuggestionConfig{}, fmt.Errorf("failed to find suggestions config in ConfigMap: %s", consts.KatibConfigMapName) } // Parse suggestion data to map where key = algorithm name, value = SuggestionConfig @@ -85,20 +85,17 @@ func GetSuggestionConfigData(algorithmName string, client client.Client) (Sugges // Try to find SuggestionConfig for the algorithm suggestionConfigData, ok = suggestionsConfig[algorithmName] if !ok { - return SuggestionConfig{}, errors.New("failed to find suggestion config for algorithm: " + algorithmName + " in ConfigMap: " + consts.KatibConfigMapName) + return SuggestionConfig{}, fmt.Errorf("failed to find suggestion config for algorithm: %s in ConfigMap: %s", algorithmName, consts.KatibConfigMapName) } // Get image from config image := suggestionConfigData.Image if strings.TrimSpace(image) == "" { - return SuggestionConfig{}, errors.New("required value for image configuration of algorithm name: " + algorithmName) + return SuggestionConfig{}, fmt.Errorf("required value for image configuration of algorithm name: %s", algorithmName) } - // Get Image Pull Policy - imagePullPolicy := suggestionConfigData.ImagePullPolicy - if imagePullPolicy != corev1.PullAlways && imagePullPolicy != corev1.PullIfNotPresent && imagePullPolicy != corev1.PullNever { - suggestionConfigData.ImagePullPolicy = consts.DefaultImagePullPolicy - } + // Set Image Pull Policy + suggestionConfigData.ImagePullPolicy = setImagePullPolicy(suggestionConfigData.ImagePullPolicy) // Set resource requirements for suggestion suggestionConfigData.Resource = setResourceRequirements(suggestionConfigData.Resource) @@ -119,9 +116,8 @@ func GetSuggestionConfigData(algorithmName string, client client.Client) (Sugges } // Set default resources - defaultVolumeStorage, _ := resource.ParseQuantity(consts.DefaultSuggestionVolumeStorage) if len(pvcSpec.Resources.Requests) == 0 { - + defaultVolumeStorage, _ := resource.ParseQuantity(consts.DefaultSuggestionVolumeStorage) pvcSpec.Resources.Requests = make(map[corev1.ResourceName]resource.Quantity) pvcSpec.Resources.Requests[corev1.ResourceStorage] = defaultVolumeStorage } @@ -157,7 +153,7 @@ func GetEarlyStoppingConfigData(algorithmName string, client client.Client) (Ear // Try to find early stopping data in config map. config, ok := configMap.Data[consts.LabelEarlyStoppingTag] if !ok { - return EarlyStoppingConfig{}, errors.New("failed to find early stopping config in ConfigMap: " + consts.KatibConfigMapName) + return EarlyStoppingConfig{}, fmt.Errorf("failed to find early stopping config in ConfigMap: %s", consts.KatibConfigMapName) } // Parse early stopping data to map where key = algorithm name, value = EarlyStoppingConfig. @@ -169,20 +165,17 @@ func GetEarlyStoppingConfigData(algorithmName string, client client.Client) (Ear // Try to find EarlyStoppingConfig for the algorithm. earlyStoppingConfigData, ok = earlyStoppingsConfig[algorithmName] if !ok { - return EarlyStoppingConfig{}, errors.New("failed to find early stopping config for algorithm: " + algorithmName + " in ConfigMap: " + consts.KatibConfigMapName) + return EarlyStoppingConfig{}, fmt.Errorf("failed to find early stopping config for algorithm: %s in ConfigMap: %s", algorithmName, consts.KatibConfigMapName) } // Get image from config. image := earlyStoppingConfigData.Image if strings.TrimSpace(image) == "" { - return EarlyStoppingConfig{}, errors.New("required value for image configuration of algorithm name: " + algorithmName) + return EarlyStoppingConfig{}, fmt.Errorf("required value for image configuration of algorithm name: %s", algorithmName) } - // Get Image Pull Policy. - imagePullPolicy := earlyStoppingConfigData.ImagePullPolicy - if imagePullPolicy != corev1.PullAlways && imagePullPolicy != corev1.PullIfNotPresent && imagePullPolicy != corev1.PullNever { - earlyStoppingConfigData.ImagePullPolicy = consts.DefaultImagePullPolicy - } + // Set Image Pull Policy. + earlyStoppingConfigData.ImagePullPolicy = setImagePullPolicy(earlyStoppingConfigData.ImagePullPolicy) return earlyStoppingConfigData, nil } @@ -202,7 +195,7 @@ func GetMetricsCollectorConfigData(cKind common.CollectorKind, client client.Cli // Try to find metrics collector data in config map config, ok := configMap.Data[consts.LabelMetricsCollectorSidecar] if !ok { - return MetricsCollectorConfig{}, errors.New("failed to find metrics collector config in ConfigMap: " + consts.KatibConfigMapName) + return MetricsCollectorConfig{}, fmt.Errorf("failed to find metrics collector config in ConfigMap: %s", consts.KatibConfigMapName) } // Parse metrics collector data to map where key = collector kind, value = MetricsCollectorConfig kind := string(cKind) @@ -214,20 +207,17 @@ func GetMetricsCollectorConfigData(cKind common.CollectorKind, client client.Cli // Try to find MetricsCollectorConfig for the collector kind metricsCollectorConfigData, ok = mcsConfig[kind] if !ok { - return MetricsCollectorConfig{}, errors.New("failed to find metrics collector config for kind: " + kind + " in ConfigMap: " + consts.KatibConfigMapName) + return MetricsCollectorConfig{}, fmt.Errorf("failed to find metrics collector config for kind: %s in ConfigMap: %s", kind, consts.KatibConfigMapName) } // Get image from config image := metricsCollectorConfigData.Image if strings.TrimSpace(image) == "" { - return MetricsCollectorConfig{}, errors.New("required value for image configuration of metrics collector kind: " + kind) + return MetricsCollectorConfig{}, fmt.Errorf("required value for image configuration of metrics collector kind: %s", kind) } - // Get Image Pull Policy - imagePullPolicy := metricsCollectorConfigData.ImagePullPolicy - if imagePullPolicy != corev1.PullAlways && imagePullPolicy != corev1.PullIfNotPresent && imagePullPolicy != corev1.PullNever { - metricsCollectorConfigData.ImagePullPolicy = consts.DefaultImagePullPolicy - } + // Set Image Pull Policy + metricsCollectorConfigData.ImagePullPolicy = setImagePullPolicy(metricsCollectorConfigData.ImagePullPolicy) // Set resource requirements for metrics collector metricsCollectorConfigData.Resource = setResourceRequirements(metricsCollectorConfigData.Resource) @@ -235,6 +225,13 @@ func GetMetricsCollectorConfigData(cKind common.CollectorKind, client client.Cli return metricsCollectorConfigData, nil } +func setImagePullPolicy(imagePullPolicy corev1.PullPolicy) corev1.PullPolicy { + if imagePullPolicy != corev1.PullAlways && imagePullPolicy != corev1.PullIfNotPresent && imagePullPolicy != corev1.PullNever { + return consts.DefaultImagePullPolicy + } + return imagePullPolicy +} + func setResourceRequirements(configResource corev1.ResourceRequirements) corev1.ResourceRequirements { // If requests are empty create new map @@ -298,7 +295,7 @@ func setResourceRequirements(configResource corev1.ResourceRequirements) corev1. } // If user explicitly sets ephemeral-storage value to something negative, nuke it. - // This enables compability with the GKE nodepool autoscalers, which cannot scale + // This enables compatibility with the GKE nodepool autoscalers, which cannot scale // pods which define ephemeral-storage resource constraints. if diskLimit.Sign() == -1 && diskRequest.Sign() == -1 { delete(configResource.Limits, corev1.ResourceEphemeralStorage) diff --git a/pkg/util/v1beta1/katibconfig/config_test.go b/pkg/util/v1beta1/katibconfig/config_test.go new file mode 100644 index 00000000000..58707e1c44e --- /dev/null +++ b/pkg/util/v1beta1/katibconfig/config_test.go @@ -0,0 +1,468 @@ +package katibconfig + +import ( + "encoding/json" + "fmt" + "reflect" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + commonv1beta1 "github.com/kubeflow/katib/pkg/apis/controller/common/v1beta1" + "github.com/kubeflow/katib/pkg/controller.v1beta1/consts" +) + +type katibConfig struct { + suggestion map[string]*SuggestionConfig + earlyStopping map[string]*EarlyStoppingConfig + metricsCollector map[commonv1beta1.CollectorKind]*MetricsCollectorConfig +} + +func TestGetSuggestionConfigData(t *testing.T) { + const testAlgorithmName = "test-suggestion" + + tests := []struct { + testDescription string + katibConfig *katibConfig + expected *SuggestionConfig + inputAlgorithmName string + err bool + }{ + { + testDescription: "All parameters correctly are specified", + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].ImagePullPolicy = corev1.PullAlways + kc.suggestion[testAlgorithmName].Resource = *newFakeCustomResourceRequirements() + return kc + }(), + expected: func() *SuggestionConfig { + c := newFakeSuggestionConfig() + c.ImagePullPolicy = corev1.PullAlways + c.Resource = *newFakeCustomResourceRequirements() + return c + }(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + { + testDescription: "There is not katib-config.", + katibConfig: nil, + err: true, + }, + { + testDescription: fmt.Sprintf("There is not %s field in katib-config configMap", consts.LabelSuggestionTag), + katibConfig: &katibConfig{}, + err: true, + }, + { + testDescription: "There is not the AlgorithmName", + katibConfig: &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}}, + inputAlgorithmName: "invalid-algorithm-name", + err: true, + }, + { + testDescription: "Image filed is empty in katib-config configMap", + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].Image = "" + return kc + }(), + inputAlgorithmName: testAlgorithmName, + err: true, + }, + { + testDescription: fmt.Sprintf("GetSuggestionConfigData sets %s to imagePullPolicy", consts.DefaultImagePullPolicy), + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].ImagePullPolicy = "" + return kc + }(), + expected: newFakeSuggestionConfig(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + { + testDescription: "GetSuggestionConfigData sets resource.requests and resource.limits for the suggestion service", + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].Resource = corev1.ResourceRequirements{} + return kc + }(), + expected: newFakeSuggestionConfig(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + { + testDescription: fmt.Sprintf("GetSuggestionConfigData sets %s to volumeMountPath", consts.DefaultContainerSuggestionVolumeMountPath), + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].VolumeMountPath = "" + return kc + }(), + expected: newFakeSuggestionConfig(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + { + testDescription: "GetSuggestionConfigData sets accessMode and resource.requests for PVC", + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].PersistentVolumeClaimSpec = corev1.PersistentVolumeClaimSpec{} + return kc + }(), + expected: newFakeSuggestionConfig(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + { + testDescription: fmt.Sprintf("GetSuggestionConfigData does not set %s to persistentVolumeReclaimPolicy", corev1.PersistentVolumeReclaimDelete), + katibConfig: func() *katibConfig { + kc := &katibConfig{suggestion: map[string]*SuggestionConfig{testAlgorithmName: newFakeSuggestionConfig()}} + kc.suggestion[testAlgorithmName].PersistentVolumeSpec = corev1.PersistentVolumeSpec{} + return kc + }(), + expected: func() *SuggestionConfig { + c := newFakeSuggestionConfig() + c.PersistentVolumeSpec = corev1.PersistentVolumeSpec{} + return c + }(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + } + + for _, tt := range tests { + t.Run(tt.testDescription, func(t *testing.T) { + fakeKubeClient := newFakeKubeClient(newFakeKatibConfigMap(tt.katibConfig)) + actual, err := GetSuggestionConfigData(tt.inputAlgorithmName, fakeKubeClient) + if (err != nil) != tt.err { + t.Errorf("want error: %v, actual: %v", tt.err, err) + } else if tt.expected != nil { + if !reflect.DeepEqual(actual, *tt.expected) { + t.Errorf("Generated SuggestionConfig is invalid.\n\nactual:\n%v\n\nexpected:\n%v\n\n", actual, *tt.expected) + } + } + }) + } + +} + +func TestGetEarlyStoppingConfigData(t *testing.T) { + const testAlgorithmName = "test-early-stopping" + + tests := []struct { + testDescription string + katibConfig *katibConfig + expected *EarlyStoppingConfig + inputAlgorithmName string + err bool + }{ + { + testDescription: "All parameters correctly are specified", + katibConfig: func() *katibConfig { + kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}} + kc.earlyStopping[testAlgorithmName].ImagePullPolicy = corev1.PullIfNotPresent + return kc + }(), + expected: func() *EarlyStoppingConfig { + c := newFakeEarlyStoppingConfig() + c.ImagePullPolicy = corev1.PullIfNotPresent + return c + }(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + { + testDescription: "There is not katib-config.", + katibConfig: nil, + err: true, + }, + { + testDescription: fmt.Sprintf("There is not %s field in katib-config configMap", consts.LabelEarlyStoppingTag), + katibConfig: &katibConfig{}, + err: true, + }, + { + testDescription: "There is not the AlgorithmName", + katibConfig: &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}}, + inputAlgorithmName: "invalid-algorithm-name", + err: true, + }, + { + testDescription: "Image filed is empty in katib-config configMap", + katibConfig: func() *katibConfig { + kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}} + kc.earlyStopping[testAlgorithmName].Image = "" + return kc + }(), + inputAlgorithmName: testAlgorithmName, + err: true, + }, + { + testDescription: fmt.Sprintf("GetEarlyStoppingConfigData sets %s to imagePullPolicy", consts.DefaultImagePullPolicy), + katibConfig: func() *katibConfig { + kc := &katibConfig{earlyStopping: map[string]*EarlyStoppingConfig{testAlgorithmName: newFakeEarlyStoppingConfig()}} + kc.earlyStopping[testAlgorithmName].ImagePullPolicy = "" + return kc + }(), + expected: newFakeEarlyStoppingConfig(), + inputAlgorithmName: testAlgorithmName, + err: false, + }, + } + + for _, tt := range tests { + t.Run(tt.testDescription, func(t *testing.T) { + fakeKubeClient := newFakeKubeClient(newFakeKatibConfigMap(tt.katibConfig)) + actual, err := GetEarlyStoppingConfigData(tt.inputAlgorithmName, fakeKubeClient) + if (err != nil) != tt.err { + t.Errorf("want error: %v, actual: %v", tt.err, err) + } else if tt.expected != nil { + if !reflect.DeepEqual(actual, *tt.expected) { + t.Errorf("Generated EarlyStoppingConfig is invalid.\n\nactual:\n%v\n\nexpected:\n%v\n\n", actual, *tt.expected) + } + } + }) + } +} + +func TestGetMetricsCollectorConfigData(t *testing.T) { + const ( + invalidCollectorKind commonv1beta1.CollectorKind = "invalidCollector" + testCollectorKind commonv1beta1.CollectorKind = "testCollector" + ) + + nukeResource, _ := resource.ParseQuantity("-1") + nukeResourceRequirements := map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: nukeResource, + corev1.ResourceMemory: nukeResource, + corev1.ResourceEphemeralStorage: nukeResource, + } + + tests := []struct { + testDescription string + katibConfig *katibConfig + expected *MetricsCollectorConfig + inputCollectorKind commonv1beta1.CollectorKind + err bool + }{ + { + testDescription: "All parameters correctly are specified", + katibConfig: func() *katibConfig { + kc := &katibConfig{metricsCollector: map[commonv1beta1.CollectorKind]*MetricsCollectorConfig{testCollectorKind: newFakeMetricsCollectorConfig()}} + kc.metricsCollector[testCollectorKind].ImagePullPolicy = corev1.PullNever + return kc + }(), + expected: func() *MetricsCollectorConfig { + c := newFakeMetricsCollectorConfig() + c.ImagePullPolicy = corev1.PullNever + return c + }(), + inputCollectorKind: testCollectorKind, + err: false, + }, + { + testDescription: "There is not katib-config.", + katibConfig: nil, + err: true, + }, + { + testDescription: fmt.Sprintf("There is not %s field in katib-config configMap", consts.LabelMetricsCollectorSidecar), + katibConfig: &katibConfig{}, + err: true, + }, + { + testDescription: "There is not the cKind", + katibConfig: &katibConfig{metricsCollector: map[commonv1beta1.CollectorKind]*MetricsCollectorConfig{testCollectorKind: newFakeMetricsCollectorConfig()}}, + inputCollectorKind: invalidCollectorKind, + err: true, + }, + { + testDescription: "Image filed is empty in katib-config configMap", + katibConfig: func() *katibConfig { + kc := &katibConfig{metricsCollector: map[commonv1beta1.CollectorKind]*MetricsCollectorConfig{testCollectorKind: newFakeMetricsCollectorConfig()}} + kc.metricsCollector[testCollectorKind].Image = "" + return kc + }(), + inputCollectorKind: testCollectorKind, + err: true, + }, + { + testDescription: fmt.Sprintf("GetMetricsConfigData sets %s to imagePullPolicy", consts.DefaultImagePullPolicy), + katibConfig: func() *katibConfig { + kc := &katibConfig{metricsCollector: map[commonv1beta1.CollectorKind]*MetricsCollectorConfig{testCollectorKind: newFakeMetricsCollectorConfig()}} + kc.metricsCollector[testCollectorKind].ImagePullPolicy = "" + return kc + }(), + expected: newFakeMetricsCollectorConfig(), + inputCollectorKind: testCollectorKind, + err: false, + }, + { + testDescription: "GetMetricsConfigData nukes resource.requests and resource.limits for the metrics collector", + katibConfig: func() *katibConfig { + kc := &katibConfig{metricsCollector: map[commonv1beta1.CollectorKind]*MetricsCollectorConfig{testCollectorKind: newFakeMetricsCollectorConfig()}} + kc.metricsCollector[testCollectorKind].Resource = corev1.ResourceRequirements{ + Requests: nukeResourceRequirements, + Limits: nukeResourceRequirements, + } + return kc + }(), + expected: func() *MetricsCollectorConfig { + c := newFakeMetricsCollectorConfig() + c.Resource = corev1.ResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{}, + Limits: map[corev1.ResourceName]resource.Quantity{}, + } + return c + }(), + inputCollectorKind: testCollectorKind, + err: false, + }, + } + + for _, tt := range tests { + t.Run(tt.testDescription, func(t *testing.T) { + fakeKubeClient := newFakeKubeClient(newFakeKatibConfigMap(tt.katibConfig)) + actual, err := GetMetricsCollectorConfigData(tt.inputCollectorKind, fakeKubeClient) + if (err != nil) != tt.err { + t.Errorf("want error: %v, actual: %v", tt.err, err) + } else if tt.expected != nil { + if !reflect.DeepEqual(actual, *tt.expected) { + t.Errorf("Generated MetricsCollectorConfig is invalid.\n\nactual:\n%v\n\nexpected:\n%v\n\n", actual, *tt.expected) + } + } + }) + } +} + +func newFakeKubeClient(katibConfigMap *corev1.ConfigMap) client.Client { + fakeClientBuilder := fake.NewClientBuilder().WithScheme(scheme.Scheme) + if katibConfigMap != nil { + fakeClientBuilder.WithObjects(katibConfigMap) + } + return fakeClientBuilder.Build() +} + +func newFakeKatibConfigMap(config *katibConfig) *corev1.ConfigMap { + if config == nil { + return nil + } + + data := map[string]string{} + if config.suggestion != nil { + bSuggestionConfig, _ := json.Marshal(config.suggestion) + data[consts.LabelSuggestionTag] = string(bSuggestionConfig) + } + if config.earlyStopping != nil { + bEarlyStoppingConfig, _ := json.Marshal(config.earlyStopping) + data[consts.LabelEarlyStoppingTag] = string(bEarlyStoppingConfig) + } + if config.metricsCollector != nil { + bMetricsCollector, _ := json.Marshal(config.metricsCollector) + data[consts.LabelMetricsCollectorSidecar] = string(bMetricsCollector) + } + + return &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + Kind: "ConfigMap", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: consts.KatibConfigMapName, + Namespace: consts.DefaultKatibNamespace, + }, + Data: data, + } +} + +func newFakeSuggestionConfig() *SuggestionConfig { + defaultVolumeStorage, _ := resource.ParseQuantity(consts.DefaultSuggestionVolumeStorage) + + return &SuggestionConfig{ + Image: "suggestion-image", + ImagePullPolicy: consts.DefaultImagePullPolicy, + Resource: *setFakeResourceRequirements(), + VolumeMountPath: consts.DefaultContainerSuggestionVolumeMountPath, + PersistentVolumeClaimSpec: corev1.PersistentVolumeClaimSpec{ + AccessModes: []corev1.PersistentVolumeAccessMode{ + consts.DefaultSuggestionVolumeAccessMode, + }, + Resources: corev1.ResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceStorage: defaultVolumeStorage, + }, + }, + }, + PersistentVolumeSpec: corev1.PersistentVolumeSpec{ + PersistentVolumeReclaimPolicy: corev1.PersistentVolumeReclaimDelete, + }, + } +} + +func newFakeEarlyStoppingConfig() *EarlyStoppingConfig { + return &EarlyStoppingConfig{ + Image: "early-stopping-image", + ImagePullPolicy: consts.DefaultImagePullPolicy, + } +} + +func newFakeMetricsCollectorConfig() *MetricsCollectorConfig { + return &MetricsCollectorConfig{ + Image: "metrics-collector-image", + ImagePullPolicy: consts.DefaultImagePullPolicy, + Resource: *setFakeResourceRequirements(), + } +} + +func setFakeResourceRequirements() *corev1.ResourceRequirements { + defaultCPURequest, _ := resource.ParseQuantity(consts.DefaultCPURequest) + defaultMemoryRequest, _ := resource.ParseQuantity(consts.DefaultMemRequest) + defaultEphemeralStorageRequest, _ := resource.ParseQuantity(consts.DefaultDiskRequest) + + defaultCPULimit, _ := resource.ParseQuantity(consts.DefaultCPULimit) + defaultMemoryLimit, _ := resource.ParseQuantity(consts.DefaultMemLimit) + defaultEphemeralStorageLimit, _ := resource.ParseQuantity(consts.DefaultDiskLimit) + + return &corev1.ResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: defaultCPURequest, + corev1.ResourceMemory: defaultMemoryRequest, + corev1.ResourceEphemeralStorage: defaultEphemeralStorageRequest, + }, + Limits: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: defaultCPULimit, + corev1.ResourceMemory: defaultMemoryLimit, + corev1.ResourceEphemeralStorage: defaultEphemeralStorageLimit, + }, + } +} + +func newFakeCustomResourceRequirements() *corev1.ResourceRequirements { + customCPURequest, _ := resource.ParseQuantity("25m") + customMemoryRequest, _ := resource.ParseQuantity("200Mi") + customEphemeralStorageRequest, _ := resource.ParseQuantity("550Mi") + + customCPULimit, _ := resource.ParseQuantity("250m") + customMemoryLimit, _ := resource.ParseQuantity("2Gi") + customEphemeralStorageLimit, _ := resource.ParseQuantity("15Gi") + + return &corev1.ResourceRequirements{ + Requests: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: customCPURequest, + corev1.ResourceMemory: customMemoryRequest, + corev1.ResourceEphemeralStorage: customEphemeralStorageRequest, + }, + Limits: map[corev1.ResourceName]resource.Quantity{ + corev1.ResourceCPU: customCPULimit, + corev1.ResourceMemory: customMemoryLimit, + corev1.ResourceEphemeralStorage: customEphemeralStorageLimit, + }, + } +} diff --git a/pkg/webhook/v1beta1/experiment/validator/validator.go b/pkg/webhook/v1beta1/experiment/validator/validator.go index 09a80d6f5d3..a9e01fa4f34 100644 --- a/pkg/webhook/v1beta1/experiment/validator/validator.go +++ b/pkg/webhook/v1beta1/experiment/validator/validator.go @@ -359,7 +359,7 @@ func validatePatchJob(runSpec *unstructured.Unstructured, job interface{}, jobTy // Not necessary to check error job must be valid JSON runSpecAfter, _ := json.Marshal(job) - // Create Patch on tranformed Job (e.g: Job) using unstructured JSON + // Create Patch on transformed Job (e.g: Job) using unstructured JSON runSpecPatchOperations, err := jsonPatch.CreatePatch(runSpecAfter, runSpecBefore) if err != nil { return fmt.Errorf("create patch error: %v", err)