Use empty target id for in-cluster (flyteorg#330)

* Use empty target id for in-cluster Signed-off-by: Katrina Rogan <[email protected]> * Add unit test Signed-off-by: Katrina Rogan <[email protected]> * more Signed-off-by: Katrina Rogan <[email protected]> * more Signed-off-by: Katrina Rogan <[email protected]> * better test Signed-off-by: Katrina Rogan <[email protected]> * lint you bothersome thing Signed-off-by: Katrina Rogan <[email protected]>
eapolinario · Jan 26, 2022 · fde80de · fde80de
1 parent 383243f
commit fde80de
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 8 deletions.
diff --git a/pkg/executioncluster/impl/in_cluster.go b/pkg/executioncluster/impl/in_cluster.go
@@ -14,6 +14,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
+// DO NOT USE: only for backwards compatibility
 const defaultInClusterTargetID = "id"
 
 type InCluster struct {
@@ -22,7 +23,7 @@ type InCluster struct {
 }
 
 func (i InCluster) GetTarget(ctx context.Context, spec *executioncluster.ExecutionTargetSpec) (*executioncluster.ExecutionTarget, error) {
-	if spec != nil && spec.TargetID != "" {
+	if spec != nil && !(spec.TargetID == "" || spec.TargetID == defaultInClusterTargetID) {
 		return nil, errors.New(fmt.Sprintf("remote target %s is not supported", spec.TargetID))
 	}
 	return &i.target, nil
@@ -54,7 +55,6 @@ func NewInCluster(initializationErrorCounter prometheus.Counter, kubeConfig, mas
 		return nil, err
 	}
 	target := executioncluster.ExecutionTarget{
-		ID:            defaultInClusterTargetID,
 		Client:        kubeClient,
 		FlyteClient:   flyteClient,
 		DynamicClient: dynamicClient,

diff --git a/pkg/executioncluster/impl/in_cluster_test.go b/pkg/executioncluster/impl/in_cluster_test.go
@@ -20,6 +20,30 @@ func TestInClusterGetTarget(t *testing.T) {
 	assert.Equal(t, "t1", target.ID)
 }
 
+func TestInClusterGetTarget_AllowableSpecIDs(t *testing.T) {
+	cluster := InCluster{
+		target: executioncluster.ExecutionTarget{},
+	}
+	target, err := cluster.GetTarget(context.Background(), nil)
+	assert.Nil(t, err)
+	assert.Equal(t, *target, cluster.target)
+
+	target, err = cluster.GetTarget(context.Background(), &executioncluster.ExecutionTargetSpec{})
+	assert.Nil(t, err)
+	assert.Equal(t, *target, cluster.target)
+
+	target, err = cluster.GetTarget(context.Background(), &executioncluster.ExecutionTargetSpec{
+		TargetID: defaultInClusterTargetID,
+	})
+	assert.Nil(t, err)
+	assert.Equal(t, *target, cluster.target)
+
+	_, err = cluster.GetTarget(context.Background(), &executioncluster.ExecutionTargetSpec{
+		TargetID: "t1",
+	})
+	assert.Error(t, err)
+}
+
 func TestInClusterGetRemoteTarget(t *testing.T) {
 	cluster := InCluster{
 		target: executioncluster.ExecutionTarget{},
@@ -29,20 +53,20 @@ func TestInClusterGetRemoteTarget(t *testing.T) {
 }
 
 func TestInClusterGetAllValidTargets(t *testing.T) {
-	target := executioncluster.ExecutionTarget{
-		ID: defaultInClusterTargetID,
+	target := &executioncluster.ExecutionTarget{
+		Enabled: true,
 	}
 	cluster := InCluster{
-		target: target,
+		target: *target,
 		asTargets: map[string]*executioncluster.ExecutionTarget{
-			defaultInClusterTargetID: &target,
+			target.ID: target,
 		},
 	}
 	targets := cluster.GetValidTargets()
 	assert.Equal(t, 1, len(targets))
-	assert.Equal(t, defaultInClusterTargetID, targets[defaultInClusterTargetID].ID)
+	assert.Equal(t, targets[target.ID], target)
 
 	targets = cluster.GetAllTargets()
 	assert.Equal(t, 1, len(targets))
-	assert.Equal(t, defaultInClusterTargetID, targets[defaultInClusterTargetID].ID)
+	assert.Equal(t, targets[target.ID], target)
 }