Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test Instascale machine set functionality #322

Merged
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions test/e2e/instascale_machinepool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ func TestInstascaleMachinePool(t *testing.T) {
test := With(t)
test.T().Parallel()

if !IsOsd() {
test.T().Skip("Skipping test as not running on an OSD cluster")
clusterType := GetClusterType(test)
if clusterType != OsdCluster {
test.T().Skipf("Skipping test as not running on an OSD cluster, resolved cluster type: %s", clusterType)
}

namespace := test.NewTestNamespace()
Expand Down
55 changes: 55 additions & 0 deletions test/e2e/instascale_machineset_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package e2e

import (
"testing"

. "github.com/onsi/gomega"
mcadv1beta1 "github.com/project-codeflare/multi-cluster-app-dispatcher/pkg/apis/controller/v1beta1"

. "github.com/project-codeflare/codeflare-operator/test/support"
)

func TestInstascaleMachineSet(t *testing.T) {
test := With(t)
test.T().Parallel()

// skip test if not using machine sets
clusterType := GetClusterType(test)
if clusterType != OcpCluster {
test.T().Skipf("Skipping test as not running on an OCP cluster, resolved cluster type: %s", clusterType)
}

namespace := test.NewTestNamespace()

// Test configuration
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"),
})

// look for machine set with aw name - expect to find it
test.Expect(GetMachineSets(test)).Should(ContainElement(WithTransform(MachineSetId, Equal("test-instascale"))))
Fiona-Waters marked this conversation as resolved.
Show resolved Hide resolved
// look for machine belonging to the machine set, there should be none
test.Expect(GetMachines(test, "test-instascale")).Should(BeEmpty())

// // Setup batch job and AppWrapper
_, aw, err := createInstaScaleJobAppWrapper(test, namespace, cm)
test.Expect(err).NotTo(HaveOccurred())

// assert that AppWrapper goes to "Running" state
test.Eventually(AppWrapper(test, namespace, aw.Name), TestTimeoutGpuProvisioning).
Should(WithTransform(AppWrapperState, Equal(mcadv1beta1.AppWrapperStateActive)))

// look for machine belonging to the machine set - expect to find it
test.Eventually(Machines(test, "test-instascale"), TestTimeoutLong).Should(HaveLen(1))

// assert that the AppWrapper goes to "Completed" state
test.Eventually(AppWrapper(test, namespace, aw.Name), TestTimeoutMedium).
Should(WithTransform(AppWrapperState, Equal(mcadv1beta1.AppWrapperStateCompleted)))

// look for machine belonging to the machine set - there should be none
test.Eventually(Machines(test, "test-instascale"), TestTimeoutLong).Should(BeEmpty())

}
13 changes: 13 additions & 0 deletions test/support/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ import (
"k8s.io/client-go/tools/clientcmd"

imagev1 "github.com/openshift/client-go/image/clientset/versioned"
machinev1 "github.com/openshift/client-go/machine/clientset/versioned"
routev1 "github.com/openshift/client-go/route/clientset/versioned"
// Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.)
// to ensure that exec-entrypoint and run can make use of them.
)

type Client interface {
Core() kubernetes.Interface
Machine() machinev1.Interface
Route() routev1.Interface
Image() imagev1.Interface
MCAD() mcadclient.Interface
Expand All @@ -42,6 +44,7 @@ type Client interface {

type testClient struct {
core kubernetes.Interface
machine machinev1.Interface
route routev1.Interface
image imagev1.Interface
mcad mcadclient.Interface
Expand All @@ -55,6 +58,10 @@ func (t *testClient) Core() kubernetes.Interface {
return t.core
}

func (t *testClient) Machine() machinev1.Interface {
return t.machine
}

func (t *testClient) Route() routev1.Interface {
return t.route
}
Expand Down Expand Up @@ -88,6 +95,11 @@ func newTestClient() (Client, error) {
return nil, err
}

machineClient, err := machinev1.NewForConfig(cfg)
if err != nil {
return nil, err
}

routeClient, err := routev1.NewForConfig(cfg)
if err != nil {
return nil, err
Expand Down Expand Up @@ -115,6 +127,7 @@ func newTestClient() (Client, error) {

return &testClient{
core: kubeClient,
machine: machineClient,
route: routeClient,
image: imageClient,
mcad: mcadClient,
Expand Down
33 changes: 28 additions & 5 deletions test/support/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,18 @@ const (

// Cluster ID for OSD cluster used in tests, used for testing InstaScale
OsdClusterID = "CLUSTERID"

// Type of cluster test is run on
ClusterTypeEnvVar = "CLUSTER_TYPE"
)

type ClusterType string

const (
OsdCluster ClusterType = "OSD"
OcpCluster ClusterType = "OCP"
HypershiftCluster ClusterType = "HYPERSHIFT"
UndefinedCluster ClusterType = "UNDEFINED"
)

func GetCodeFlareSDKVersion() string {
Expand Down Expand Up @@ -65,12 +77,23 @@ func GetOsdClusterId() (string, bool) {
return os.LookupEnv(OsdClusterID)
}

func IsOsd() bool {
osdClusterId, found := GetOsdClusterId()
if found && osdClusterId != "" {
return true
func GetClusterType(t Test) ClusterType {
clusterType, ok := os.LookupEnv(ClusterTypeEnvVar)
if !ok {
t.T().Logf("Expected environment variable %s not found, cluster type is not defined.", ClusterTypeEnvVar)
return UndefinedCluster
}
switch clusterType {
case "OSD":
return OsdCluster
case "OCP":
return OcpCluster
case "HYPERSHIFT":
return HypershiftCluster
default:
t.T().Logf("Expected environment variable %s contains unexpected value: '%s'", ClusterTypeEnvVar, clusterType)
return UndefinedCluster
}
return false
}

func lookupEnvOrDefault(key, value string) string {
Expand Down
32 changes: 32 additions & 0 deletions test/support/machine.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package support

import (
"github.com/onsi/gomega"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

machinev1beta1 "github.com/openshift/api/machine/v1beta1"
)

func GetMachineSets(t Test) ([]machinev1beta1.MachineSet, error) {
ms, err := t.Client().Machine().MachineV1beta1().MachineSets("openshift-machine-api").List(t.Ctx(), metav1.ListOptions{})
t.Expect(err).NotTo(gomega.HaveOccurred())
return ms.Items, err
}

func Machines(t Test, machineSetName string) func(g gomega.Gomega) []machinev1beta1.Machine {
return func(g gomega.Gomega) []machinev1beta1.Machine {
machine, err := t.Client().Machine().MachineV1beta1().Machines("openshift-machine-api").List(t.Ctx(), metav1.ListOptions{LabelSelector: "machine.openshift.io/cluster-api-machineset=" + machineSetName})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it OK to have openshift-machine-api hard-coded?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could passing the label selector as argument instead be useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we can access the machine label easily in the test to pass it in. Could this work, or what would be the benefit of passing it as an argument? maybe it can be one variable instead of 2 also since these shouldn't change.

labelSelectorPrefix = "machine.openshift.io/cluster-api-machineset="
machine, err := t.Client().Machine().MachineV1beta1().Machines(namespaceToList).List(t.Ctx(), metav1.ListOptions{LabelSelector: labelSelectorPrefix + machineSetName})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, probably bette to keep it that way for now.

g.Expect(err).NotTo(gomega.HaveOccurred())
return machine.Items
}
}

func GetMachines(t Test, machineSetName string) []machinev1beta1.Machine {
t.T().Helper()
return Machines(t, machineSetName)(t)
}

func MachineSetId(machineSet machinev1beta1.MachineSet) string {
return machineSet.Name
}