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/clusterpools.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/config_map.go

@@ -18,9 +18,7 @@ package support
 
 import (
 	"os"
-	"strconv"
-
-	"github.com/onsi/gomega"
+	"strings"
 )
 
 const (
@@ -35,14 +33,11 @@ const (
 	// The testing output directory, to write output files into.
 	CodeFlareTestOutputDir = "CODEFLARE_TEST_OUTPUT_DIR"
 
-	// The name of a secret containing InstaScale OCM token.
-	InstaScaleOcmSecretName = "INSTASCALE_OCM_SECRET_NAME"
-	// The namespace where a secret containing InstaScale OCM token is stored.
-	InstaScaleOcmSecretNamespace = "INSTASCALE_OCM_SECRET_NAMESPACE"
+	// The namespace where a secret containing InstaScale OCM token is stored and the secret name.
+	InstaScaleOcmSecret = "INSTASCALE_OCM_SECRET"
+
 	// 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 {
@@ -61,28 +56,21 @@ func GetPyTorchImage() string {
 	return lookupEnvOrDefault(CodeFlareTestPyTorchImage, "pytorch/pytorch:1.11.0-cuda11.3-cudnn8-runtime")
 }
 
-func GetInstaScaleOcmSecretName() string {
-	return lookupEnvOrDefault(InstaScaleOcmSecretName, "instascale-ocm-secret")
-}
-
-func GetInstaScaleOcmSecretNamespace() string {
-	return lookupEnvOrDefault(InstaScaleOcmSecretNamespace, "default")
+func GetInstascaleOcmSecret() (string, string) {
+	res := strings.SplitN(lookupEnvOrDefault(InstaScaleOcmSecret, "default/instascale-com-secret"), "/", 2)
+	return res[0], res[1]
 }
 
 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 {

test/support/core.go

@@ -25,10 +25,12 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	ocmsdk "github.com/openshift-online/ocm-sdk-go"
+	cmv1 "github.com/openshift-online/ocm-sdk-go/clustersmgmt/v1"
 )
 
 func CreateOCMConnection(test Test) *ocmsdk.Connection {
-	instascaleOCMSecret, err := test.Client().Core().CoreV1().Secrets(GetInstaScaleOcmSecretNamespace()).Get(test.Ctx(), GetInstaScaleOcmSecretName(), metav1.GetOptions{})
+	secretNamespace, secretName := GetInstascaleOcmSecret()
+	instascaleOCMSecret, err := test.Client().Core().CoreV1().Secrets(secretNamespace).Get(test.Ctx(), secretName, metav1.GetOptions{})
 	test.Expect(err).NotTo(gomega.HaveOccurred())
 
 	ocmToken := string(instascaleOCMSecret.Data["token"])
@@ -50,3 +52,23 @@ func buildOCMConnection(secret string) (*ocmsdk.Connection, error) {
 
 	return connection, nil
 }
+
+func MachinePools(t Test, connection *ocmsdk.Connection) func(g gomega.Gomega) []*cmv1.MachinePool {
+	osdClusterId, found := GetOsdClusterId()
+	t.Expect(found).To(gomega.BeTrue(), "OSD cluster id not found, please configure environment properly")
+
+	return func(g gomega.Gomega) []*cmv1.MachinePool {
+		machinePoolsListResponse, err := connection.ClustersMgmt().V1().Clusters().Cluster(osdClusterId).MachinePools().List().Send()
+		g.Expect(err).NotTo(gomega.HaveOccurred())
+		return machinePoolsListResponse.Items().Slice()
+	}
+}
+
+func GetMachinePools(t Test, connection *ocmsdk.Connection) []*cmv1.MachinePool {
+	t.T().Helper()
+	return MachinePools(t, connection)(t)
+}
+
+func MachinePoolId(machinePool *cmv1.MachinePool) string {
+	return machinePool.ID()
+}