Update param name and test

opendatahub-io · Jan 7, 2025 · 5e091a2 · 5e091a2
1 parent 7e76713
commit 5e091a2
Show file tree

Hide file tree

Showing 10 changed files with 45 additions and 25 deletions.
diff --git a/api/v1/dspipeline_types.go b/api/v1/dspipeline_types.go
@@ -73,7 +73,7 @@ type APIServer struct {
 	// Include instructlab multi-phase training and LLM generation pipeline with the deployment of this DSP API Server. Default: false
 	// +kubebuilder:default:=false
 	// +kubebuilder:validation:Optional
-	EnableInstructlabPipeline bool   `json:"enableInstructlabPipeline"`
+	EnableInstructLabPipeline bool   `json:"enableInstructLabPipeline"`
 	ArgoLauncherImage         string `json:"argoLauncherImage,omitempty"`
 	ArgoDriverImage           string `json:"argoDriverImage,omitempty"`
 	// Specify custom Pod resource requirements for this component.

diff --git a/api/v1alpha1/dspipeline_types.go b/api/v1alpha1/dspipeline_types.go
@@ -73,7 +73,7 @@ type APIServer struct {
 	// Include instructlab multi-phase training and LLM generation pipeline with the deployment of this DSP API Server. Default: false
 	// +kubebuilder:default:=false
 	// +kubebuilder:validation:Optional
-	EnableInstructlabPipeline bool   `json:"enableInstructlabPipeline"`
+	EnableInstructLabPipeline bool   `json:"enableInstructLabPipeline"`
 	ArgoLauncherImage         string `json:"argoLauncherImage,omitempty"`
 	ArgoDriverImage           string `json:"argoDriverImage,omitempty"`
 	// Specify custom Pod resource requirements for this component.

diff --git a/...ses/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml b/...ses/datasciencepipelinesapplications.opendatahub.io_datasciencepipelinesapplications.yaml
@@ -105,7 +105,7 @@ spec:
                       Server. Setting Deploy to false disables operator reconciliation.
                       Default: true'
                     type: boolean
-                  enableInstructlabPipeline:
+                  enableInstructLabPipeline:
                     default: false
                     description: 'Include instructlab multi-phase training and LLM
                       generation pipeline with the deployment of this DSP API Server.
@@ -996,7 +996,7 @@ spec:
                       Server. Setting Deploy to false disables operator reconciliation.
                       Default: true'
                     type: boolean
-                  enableInstructlabPipeline:
+                  enableInstructLabPipeline:
                     default: false
                     description: 'Include instructlab multi-phase training and LLM
                       generation pipeline with the deployment of this DSP API Server.

diff --git a/config/internal/apiserver/default/deployment.yaml.tmpl b/config/internal/apiserver/default/deployment.yaml.tmpl
@@ -147,7 +147,7 @@ spec:
           args:
             - --config=/config
             - -logtostderr=true
-            {{ if or .APIServer.EnableSamplePipeline .APIServer.EnableInstructlabPipeline }}
+            {{ if or .APIServer.EnableSamplePipeline .APIServer.EnableInstructLabPipeline }}
             - --sampleconfig=/config/sample_config.json
             {{ end }}
             {{ if .PodToPodTLS }}
@@ -206,8 +206,8 @@ spec:
             - mountPath: /etc/tls/private
               name: proxy-tls
           {{ end }}
-          {{ if or .APIServer.EnableSamplePipeline .CustomCABundle .APIServer.EnableInstructlabPipeline}}
-            {{ if or .APIServer.EnableSamplePipeline .APIServer.EnableInstructlabPipeline}}
+          {{ if or .APIServer.EnableSamplePipeline .CustomCABundle .APIServer.EnableInstructLabPipeline}}
+            {{ if or .APIServer.EnableSamplePipeline .APIServer.EnableInstructLabPipeline}}
             - name: sample-config
               mountPath: /config/sample_config.json
               subPath: sample_config.json
@@ -287,7 +287,7 @@ spec:
           configMap:
             name: {{ .CustomCABundle.ConfigMapName }}
         {{ end }}
-        {{ if or .APIServer.EnableSamplePipeline .APIServer.EnableInstructlabPipeline}}
+        {{ if or .APIServer.EnableSamplePipeline .APIServer.EnableInstructLabPipeline}}
         - name: sample-config
           configMap:
             name: sample-config-{{.Name}}

diff --git a/config/internal/apiserver/sample-pipeline/sample-config.yaml.tmpl b/config/internal/apiserver/sample-pipeline/sample-config.yaml.tmpl
@@ -15,9 +15,9 @@ data:
         "description": "[source code](https://github.com/opendatahub-io/data-science-pipelines/tree/master/samples/iris-sklearn) A simple pipeline to demonstrate a basic ML Training workflow",
         "file": "/samples/iris-pipeline-compiled.yaml"
       }
-      {{- if .EnableInstructlabPipeline }},{{ end }}
+      {{- if and .EnableSamplePipeline .EnableInstructLabPipeline }},{{ end }}
       {{- end }}
-      {{- if .EnableInstructlabPipeline }}
+      {{- if .EnableInstructLabPipeline }}
       {
         "name": "[Instructlab] Multi-Phase Training Pipeline",
         "description": "[source code](https://github.com/opendatahub-io/ilab-on-ocp) Instructlab Multi-Phase Training Pipeline",

diff --git a/controllers/apiserver.go b/controllers/apiserver.go
@@ -68,7 +68,7 @@ func (r *DSPAReconciler) ReconcileAPIServer(ctx context.Context, dsp *dspav1.Dat
 	}
 
 	for cmName, template := range samplePipelineTemplates {
-		if dsp.Spec.APIServer.EnableSamplePipeline || dsp.Spec.APIServer.EnableInstructlabPipeline {
+		if dsp.Spec.APIServer.EnableSamplePipeline || dsp.Spec.APIServer.EnableInstructLabPipeline {
 			err := r.Apply(dsp, params, template)
 			if err != nil {
 				return err

diff --git a/controllers/dspipeline_params.go b/controllers/dspipeline_params.go
@@ -94,7 +94,7 @@ type DSPAParams struct {
 	// Use to enable tls communication between component pods.
 	PodToPodTLS               bool
 	EnableSamplePipeline      bool
-	EnableInstructlabPipeline bool
+	EnableInstructLabPipeline bool
 	APIServerServiceDNSName   string
 }
 
@@ -603,7 +603,7 @@ func (p *DSPAParams) ExtractParams(ctx context.Context, dsp *dspa.DataSciencePip
 		setResourcesDefault(config.APIServerResourceRequirements, &p.APIServer.Resources)
 
 		p.EnableSamplePipeline = dsp.Spec.APIServer.EnableSamplePipeline
-		p.EnableInstructlabPipeline = dsp.Spec.APIServer.EnableInstructlabPipeline
+		p.EnableInstructLabPipeline = dsp.Spec.APIServer.EnableInstructLabPipeline
 
 		if p.APIServer.CustomServerConfig == nil {
 			p.APIServer.CustomServerConfig = &dspa.ScriptConfigMap{

diff --git a/tests/pipeline_test.go b/tests/pipeline_test.go
@@ -76,19 +76,28 @@ func (suite *IntegrationTestSuite) TestAPIServerDeployment() {
 		assert.Equal(t, 200, response.StatusCode)
 	})
 
-	suite.T().Run("Should check for pipeline existence based on EnableInstructlabPipeline flag", func(t *testing.T) {
-		response, err := suite.Clientmgr.httpClient.Get(fmt.Sprintf("%s/apis/v2beta1/pipelines", APIServerURL))
-		require.NoError(t, err)
-
-		responseData, err := io.ReadAll(response.Body)
-		require.NoError(t, err)
-		require.Equal(t, 200, response.StatusCode)
-		loggr.Info(string(responseData))
-
+	suite.T().Run("Should check for Instructlab pipeline existence based on EnableInstructLabPipeline flag", func(t *testing.T) {
+		expectedDisplayName := "[Instructlab] Multi-Phase Training Pipeline"
+
+		// Retrieve pipelines
+		pipelines, err := RetrievePipelines(t, httpClient, apiServerURL)
+		require.NoError(t, err, "Failed to retrieve pipelines")
+
+		found := false
+		for _, pipeline := range pipelines.Pipelines {
+			if pipeline.DisplayName == expectedDisplayName {
+				found = true
+				break
+			}
+		}
 		if suite.DSPA.Spec.APIServer.EnableInstructlabPipeline {
-			assert.Contains(t, string(responseData), "[Instructlab] Multi-Phase Training Pipeline")
+			expectedCount := 4
+			assert.Equal(t, expectedCount, len(pipelines.Pipelines), "Pipeline count should match when Instructlab is enabled")
+			assert.True(t, found, "Instructlab pipeline should exist when the flag is enabled")
 		} else {
-			assert.NotContains(t, string(responseData), "[Instructlab] Multi-Phase Training Pipeline")
+			expectedCount := 3
+			assert.Equal(t, expectedCount, len(pipelines.Pipelines), "Pipeline count should match when Instructlab is disabled")
+			assert.False(t, found, "Instructlab pipeline should not exist when the flag is disabled")
 		}
 	})
 }
diff --git a/tests/resources/dspa-lite.yaml b/tests/resources/dspa-lite.yaml
@@ -9,7 +9,7 @@ spec:
     deploy: true
     enableOauth: false
     enableSamplePipeline: true
-    enableInstructlabPipeline: true
+    enableInstructLabPipeline: true
     cABundle:
       configMapName: nginx-tls-config
       configMapKey: rootCA.crt

diff --git a/tests/util/rest.go b/tests/util/rest.go
@@ -94,6 +94,17 @@ func RetrievePipelineId(t *testing.T, httpClient http.Client, APIServerURL strin
 	}
 }
 
+func RetrievePipelines(t *testing.T, httpClient http.Client, APIServerURL string) (Pipeline, error) {
+	response, err := httpClient.Get(fmt.Sprintf("%s/apis/v2beta1/pipelines", APIServerURL))
+	require.NoError(t, err)
+	responseData, err := io.ReadAll(response.Body)
+	require.NoError(t, err)
+	var pipelineData Pipeline
+	err = json.Unmarshal(responseData, &pipelineData)
+	require.NoError(t, err)
+	return pipelineData, nil
+}
+
 func FormatRequestBody(t *testing.T, pipelineID string, PipelineDisplayName string) []byte {
 	requestBody := PipelineRequest{
 		DisplayName: PipelineDisplayName,