add tests to check that SpaceProvisionerConfig is only ready when the underlying ToolchainCluster is ready

test/e2e/parallel/spaceprovisionerconfig_test.go

@@ -11,8 +11,9 @@ import (
 	. "github.com/codeready-toolchain/toolchain-e2e/testsupport/spaceprovisionerconfig"
 	"github.com/codeready-toolchain/toolchain-e2e/testsupport/util"
 	"github.com/codeready-toolchain/toolchain-e2e/testsupport/wait"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -25,7 +26,7 @@ func TestSpaceProvisionerConfig(t *testing.T) {
 	awaitilities := WaitForDeployments(t)
 	host := awaitilities.Host()
 
-	t.Run("ready with existing cluster", func(t *testing.T) {
+	t.Run("ready with existing ready cluster", func(t *testing.T) {
 		// given
 		// any ToolchainCluster in th host namespace will do. We don't really care...
 		cluster, err := host.WaitForToolchainCluster(t)
@@ -51,52 +52,43 @@ func TestSpaceProvisionerConfig(t *testing.T) {
 			WithNameThat(spc.Name, Is(NotReady()))
 		require.NoError(t, err)
 	})
-
-	t.Run("becomes ready when cluster appears", func(t *testing.T) {
+	t.Run("becomes ready when cluster appears and becomes ready", func(t *testing.T) {
 		// given
+		existingCluster, err := host.WaitForToolchainCluster(t, wait.UntilToolchainClusterHasName(awaitilities.Member1().ClusterName))
+		require.NoError(t, err)
 		clusterName := util.NewObjectNamePrefix(t) + string(uuid.NewUUID()[0:20])
 
 		// when
 		spc := CreateSpaceProvisionerConfig(t, host.Awaitility, ReferencingToolchainCluster(clusterName))
 
 		// then
-		_, err := wait.
+		_, err = wait.
 			For(t, host.Awaitility, &toolchainv1alpha1.SpaceProvisionerConfig{}).
 			WithNameThat(spc.Name, Is(NotReady()))
 		require.NoError(t, err)
 
 		// when
-		cluster := &toolchainv1alpha1.ToolchainCluster{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      clusterName,
-				Namespace: host.Namespace,
-			},
-		}
-		assert.NoError(t, host.CreateWithCleanup(t, cluster))
+		_ = copyClusterWithSecretAndNewName(t, host.Awaitility, clusterName, existingCluster)
 
 		// then
 		_, err = wait.
 			For(t, host.Awaitility, &toolchainv1alpha1.SpaceProvisionerConfig{}).
 			WithNameThat(spc.Name, Is(Ready()))
 		require.NoError(t, err)
 	})
-
 	t.Run("becomes not ready when cluster disappears", func(t *testing.T) {
 		// given
-		clusterName := util.NewObjectNamePrefix(t) + string(uuid.NewUUID()[0:20])
-		cluster := &toolchainv1alpha1.ToolchainCluster{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      clusterName,
-				Namespace: host.Namespace,
-			},
-		}
-		require.NoError(t, host.CreateWithCleanup(t, cluster))
+
+		// we need to create a copy of the cluster and the token secret
+		existingCluster, err := host.WaitForToolchainCluster(t)
+		require.NoError(t, err)
+		cluster := copyClusterWithSecret(t, host.Awaitility, existingCluster)
 
 		// when
-		spc := CreateSpaceProvisionerConfig(t, host.Awaitility, ReferencingToolchainCluster(clusterName))
+		spc := CreateSpaceProvisionerConfig(t, host.Awaitility, ReferencingToolchainCluster(cluster.Name))
 
 		// then
-		_, err := wait.
+		_, err = wait.
 			For(t, host.Awaitility, &toolchainv1alpha1.SpaceProvisionerConfig{}).
 			WithNameThat(spc.Name, Is(Ready()))
 		require.NoError(t, err)
@@ -110,4 +102,73 @@ func TestSpaceProvisionerConfig(t *testing.T) {
 			WithNameThat(spc.Name, Is(NotReady()))
 		require.NoError(t, err)
 	})
+	t.Run("becomes not ready when cluster becomes not ready", func(t *testing.T) {
+		// NOTE: this is impossible to test currently because there is no way for a TC to
+		// transition from ready to unready (short of updating the TC and restarting the toochaincluster cache controller
+		// which is something we can't afford in the parallel tests).
+		//
+		// // given
+		//
+		// // we need to create a copy of the cluster and the token secret
+		// existingCluster, err := host.WaitForToolchainCluster(t)
+		// require.NoError(t, err)
+		// cluster := copyClusterWithSecret(t, host.Awaitility, existingCluster)
+		//
+		// // when
+		// spc := CreateSpaceProvisionerConfig(t, host.Awaitility, ReferencingToolchainCluster(cluster.Name))
+		//
+		// // then
+		// _, err = wait.
+		// 	For(t, host.Awaitility, &toolchainv1alpha1.SpaceProvisionerConfig{}).
+		// 	WithNameThat(spc.Name, Is(Ready()))
+		// require.NoError(t, err)
+		//
+		// // when
+		// // update the TC such that it is no longer valid.
+		// require.NoError(t, host.Client.Get(context.TODO(), client.ObjectKeyFromObject(cluster), cluster))
+		// apiEndpoint, err := url.Parse(cluster.Spec.APIEndpoint)
+		// require.NoError(t, err)
+		// apiEndpoint.Host = apiEndpoint.Hostname() + "-not:" + apiEndpoint.Port()
+		// cluster.Spec.APIEndpoint = apiEndpoint.String()
+		// require.NoError(t, host.Client.Update(context.TODO(), cluster))
+		//
+		// // then
+		// _, err = wait.
+		// 	For(t, host.Awaitility, &toolchainv1alpha1.SpaceProvisionerConfig{}).
+		// 	WithNameThat(spc.Name, Is(NotReady()))
+		// require.NoError(t, err)
+	})
+}
+
+func copyClusterWithSecret(t *testing.T, a *wait.Awaitility, cluster *toolchainv1alpha1.ToolchainCluster) *toolchainv1alpha1.ToolchainCluster {
+	clusterName := util.NewObjectNamePrefix(t) + string(uuid.NewUUID()[0:20])
+	return copyClusterWithSecretAndNewName(t, a, clusterName, cluster)
+}
+
+func copyClusterWithSecretAndNewName(t *testing.T, a *wait.Awaitility, newName string, cluster *toolchainv1alpha1.ToolchainCluster) *toolchainv1alpha1.ToolchainCluster {
+	// copy the secret
+	secret := &corev1.Secret{}
+	require.NoError(t, a.CopyWithCleanup(t,
+		client.ObjectKey{
+			Name:      cluster.Spec.SecretRef.Name,
+			Namespace: cluster.Namespace,
+		},
+		client.ObjectKey{
+			Name:      newName,
+			Namespace: cluster.Namespace,
+		},
+		secret,
+	))
+
+	// and create a new ToolchainCluster with that secret
+	// note that we can't use the CopyWithCleanup function because we also
+	// need to modify the SecretRef of the TC.
+	newCluster := cluster.DeepCopy()
+	newCluster.ResourceVersion = ""
+	newCluster.UID = ""
+	newCluster.Name = newName
+	newCluster.Spec.SecretRef.Name = secret.Name
+	require.NoError(t, a.CreateWithCleanup(t, newCluster))
+
+	return newCluster
 }

test/e2e/usersignup_test.go

@@ -207,6 +207,25 @@ func (s *userSignupIntegrationTest) TestAutomaticApproval() {
 			s.userIsNotProvisioned(t, waitingList4)
 		})
 	})
+	s.T().Run("capacity manager doesn't pick offline toolchain clusters", func(t *testing.T) {
+		t.Skip("This cannot be implemented because we don't have a way of taking TCs offline without restarting the controller")
+		// spaceprovisionerconfig.UpdateForCluster(t, hostAwait.Awaitility, memberAwait2.ClusterName, testSpc.Enabled(false))
+		// hostAwait.UpdateToolchainConfig(t, testconfig.AutomaticApproval().Enabled(true))
+		// _, err := hostAwait.UpdateToolchainClusterWithCleanup(t, memberAwait1.ClusterName, func(tc *toolchainv1alpha1.ToolchainCluster) {
+		// 	tc.Spec.APIEndpoint = fmt.Sprintf("https://%s.over.the.rainbow:6443", uuid.NewString()[0:10])
+		// })
+		// require.NoError(t, err)
+		//
+		// // when
+		// userSignup, _ := NewSignupRequest(s.Awaitilities).
+		// 	Username("automatic3").
+		// 	Email("[email protected]").
+		// 	RequireConditions(wait.ConditionSet(wait.Default(), wait.PendingApproval(), wait.PendingApprovalNoCluster())...).
+		// 	Execute(t).Resources()
+		//
+		// // then
+		// s.userIsNotProvisioned(t, userSignup)
+	})
 }
 
 func (s *userSignupIntegrationTest) TestProvisionToOtherClusterWhenOneIsFull() {

testsupport/wait/awaitility.go

@@ -586,6 +586,45 @@ func (a *Awaitility) UpdateToolchainCluster(t *testing.T, toolchainClusterName s
 	return tc, err
 }
 
+func (a *Awaitility) UpdateToolchainClusterWithCleanup(t *testing.T, toolchainClusterName string, modifyToolchainCluster func(s *toolchainv1alpha1.ToolchainCluster)) (*toolchainv1alpha1.ToolchainCluster, error) {
+	var tc *toolchainv1alpha1.ToolchainCluster
+	var origTc *toolchainv1alpha1.ToolchainCluster
+	err := wait.Poll(a.RetryInterval, a.Timeout, func() (done bool, err error) {
+		newToolchainCluster := &toolchainv1alpha1.ToolchainCluster{}
+		if err := a.Client.Get(context.TODO(), types.NamespacedName{Namespace: a.Namespace, Name: toolchainClusterName}, newToolchainCluster); err != nil {
+			return true, err
+		}
+		origTc = newToolchainCluster.DeepCopy()
+		modifyToolchainCluster(newToolchainCluster)
+		if err := a.Client.Update(context.TODO(), newToolchainCluster); err != nil {
+			t.Logf("error updating ToolchainCluster '%s': %s. Will retry again...", toolchainClusterName, err.Error())
+			return false, nil
+		}
+		tc = newToolchainCluster
+		return true, nil
+	})
+
+	t.Cleanup(func() {
+		currentTc := &toolchainv1alpha1.SpaceProvisionerConfig{}
+		err := a.Client.Get(context.TODO(), client.ObjectKeyFromObject(origTc), currentTc)
+		if err != nil {
+			if apierrors.IsNotFound(err) {
+				require.NoError(t, a.Client.Create(context.TODO(), origTc))
+				return
+			}
+			require.Fail(t, err.Error())
+		}
+
+		// make the originalTc look like we freshly obtained it from the server and updated its fields
+		// to look like the original.
+		origTc.Generation = currentTc.Generation
+		origTc.ResourceVersion = currentTc.ResourceVersion
+
+		require.NoError(t, a.Client.Update(context.TODO(), origTc))
+	})
+	return tc, err
+}
+
 // CreateWithCleanup creates the given object via client.Client.Create() and schedules the cleanup of the object at the end of the current test
 func (a *Awaitility) CreateWithCleanup(t *testing.T, obj client.Object, opts ...client.CreateOption) error {
 	if err := a.Client.Create(context.TODO(), obj, opts...); err != nil {
@@ -595,6 +634,21 @@ func (a *Awaitility) CreateWithCleanup(t *testing.T, obj client.Object, opts ...
 	return nil
 }
 
+// Creates a copy of the object specified using the `from` parameter. The created copy is named using the `to` parameter and is cleaned up
+// after the test.
+func (a *Awaitility) CopyWithCleanup(t *testing.T, from, to client.ObjectKey, object client.Object, opts ...client.CreateOption) error {
+	if err := a.Client.Get(context.TODO(), from, object); err != nil {
+		return err
+	}
+
+	object.SetName(to.Name)
+	object.SetNamespace(to.Namespace)
+	object.SetResourceVersion("")
+	object.SetUID("")
+
+	return a.CreateWithCleanup(t, object, opts...)
+}
+
 // Create creates the given object via client.Client.Create()
 func (a *Awaitility) Create(obj client.Object, opts ...client.CreateOption) error {
 	if err := a.Client.Create(context.TODO(), obj, opts...); err != nil {