addressing feedback

Author: Fiona-Waters

test/e2e/instascale_app_wrapper.go

@@ -17,7 +17,6 @@ limitations under the License.
 package e2e
 
 import (
-	. "github.com/onsi/gomega"
 	mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
 
 	batchv1 "k8s.io/api/batch/v1"
@@ -138,11 +137,6 @@ func createInstaScaleJobAppWrapper(test Test, namespace *corev1.Namespace, confi
 	}
 
 	_, err := test.Client().MCAD().WorkloadV1beta1().AppWrappers(namespace.Name).Create(test.Ctx(), aw, metav1.CreateOptions{})
-	test.Expect(err).NotTo(HaveOccurred())
-	test.T().Logf("AppWrapper created successfully %s/%s", aw.Namespace, aw.Name)
-
-	test.Eventually(AppWrapper(test, namespace, aw.Name), TestTimeoutGpuProvisioning).
-		Should(WithTransform(AppWrapperState, Equal(mcadv1beta1.AppWrapperStateActive)))
 
 	return job, aw, err
 }

test/e2e/instascale_machinepool_test.go

@@ -22,35 +22,29 @@ import (
 	. "github.com/onsi/gomega"
 	mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"
 
-	batchv1 "k8s.io/api/batch/v1"
-	corev1 "k8s.io/api/core/v1"
-
 	. "github.com/project-codeflare/codeflare-operator/test/support"
 )
 
 func TestInstascaleMachinePool(t *testing.T) {
-
 	test := With(t)
 	test.T().Parallel()
 
-	if !IsOsd(test) {
+	if !IsOsd() {
 		test.T().Skip("Skipping test as not running on an OSD cluster")
 	}
 
 	namespace := test.NewTestNamespace()
 
 	// Test configuration
-	testConfigData := map[string][]byte{
+	cm := CreateConfigMap(test, namespace.Name, map[string][]byte{
 		// pip requirements
 		"requirements.txt": ReadFile(test, "mnist_pip_requirements.txt"),
 		// MNIST training script
 		"mnist.py": ReadFile(test, "mnist.py"),
-	}
-	cm := CreateConfigMap(test, namespace.Name, testConfigData)
+	})
 
 	//create OCM connection
 	connection := CreateOCMConnection(test)
-
 	defer connection.Close()
 
 	// check existing cluster machine pool resources
@@ -59,25 +53,18 @@ func TestInstascaleMachinePool(t *testing.T) {
 		ShouldNot(ContainElement(WithTransform(MachinePoolId, Equal("test-instascale-g4dn-xlarge"))))
 
 	// Setup batch job and AppWrapper
-	job, aw, err := createInstaScaleJobAppWrapper(test, namespace, cm)
+	_, aw, err := createInstaScaleJobAppWrapper(test, namespace, cm)
 	test.Expect(err).NotTo(HaveOccurred())
+	test.T().Logf("AppWrapper created successfully %s/%s", aw.Namespace, aw.Name)
+
+	// assert that AppWrapper goes to "Running" state
+	test.Eventually(AppWrapper(test, namespace, aw.Name), TestTimeoutGpuProvisioning).
+		Should(WithTransform(AppWrapperState, Equal(mcadv1beta1.AppWrapperStateActive)))
 
 	// look for machine pool with aw name - expect to find it
 	test.Eventually(MachinePools(test, connection), TestTimeoutLong).
 		Should(ContainElement(WithTransform(MachinePoolId, Equal("test-instascale-g4dn-xlarge"))))
 
-	// Assert that the job has completed
-	test.T().Logf("Waiting for Job %s/%s to complete", job.Namespace, job.Name)
-	test.Eventually(Job(test, job.Namespace, job.Name), TestTimeoutLong).Should(
-		Or(
-			WithTransform(ConditionStatus(batchv1.JobComplete), Equal(corev1.ConditionTrue)),
-			WithTransform(ConditionStatus(batchv1.JobFailed), Equal(corev1.ConditionTrue)),
-		))
-
-	// Assert the job has completed successfully
-	test.Expect(GetJob(test, job.Namespace, job.Name)).
-		To(WithTransform(ConditionStatus(batchv1.JobComplete), Equal(corev1.ConditionTrue)))
-
 	test.Eventually(AppWrapper(test, namespace, aw.Name), TestTimeoutShort).
 		Should(WithTransform(AppWrapperState, Equal(mcadv1beta1.AppWrapperStateCompleted)))
 

test/support/config_map.go

@@ -27,6 +27,27 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
+func CreateConfigMap(t Test, namespace string, content map[string][]byte) *corev1.ConfigMap {
+	configMap := &corev1.ConfigMap{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: corev1.SchemeGroupVersion.String(),
+			Kind:       "ConfigMap",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			GenerateName: "config-",
+			Namespace:    namespace,
+		},
+		BinaryData: content,
+		Immutable:  Ptr(true),
+	}
+
+	configMap, err := t.Client().Core().CoreV1().ConfigMaps(namespace).Create(t.Ctx(), configMap, metav1.CreateOptions{})
+	t.Expect(err).NotTo(gomega.HaveOccurred())
+	t.T().Logf("Created ConfigMap %s/%s successfully", configMap.Namespace, configMap.Name)
+
+	return configMap
+}
+
 func Raw(t Test, obj runtime.Object) runtime.RawExtension {
 	t.T().Helper()
 	data, err := json.Marshal(obj)

test/support/core.go

@@ -18,9 +18,6 @@ package support
 
 import (
 	"os"
-	"strconv"
-
-	"github.com/onsi/gomega"
 )
 
 const (
@@ -41,8 +38,6 @@ const (
 	InstaScaleOcmSecretNamespace = "INSTASCALE_OCM_SECRET_NAMESPACE"
 	// Cluster ID for OSD cluster used in tests, used for testing InstaScale
 	OsdClusterID = "CLUSTERID"
-	// Determine if test is being run on an OSD cluster, used for testing InstaScale.
-	IsOSD = "IS_OSD"
 )
 
 func GetCodeFlareSDKVersion() string {
@@ -73,16 +68,12 @@ func GetOsdClusterId() (string, bool) {
 	return os.LookupEnv(OsdClusterID)
 }
 
-func IsOsd(test Test) bool {
-	test.T().Helper()
-	env := lookupEnvOrDefault(IsOSD, "false")
-	osd, err := strconv.ParseBool(env)
-	if err != nil {
-		test.T().Logf("error parsing IS_OSD environment variable, using default 'false' value, error: %v ", err)
-		return false
+func IsOsd() bool {
+	osdClusterId, found := GetOsdClusterId()
+	if found && osdClusterId != "" {
+		return true
 	}
-	test.Expect(err).NotTo(gomega.HaveOccurred())
-	return osd
+	return false
 }
 
 func lookupEnvOrDefault(key, value string) string {