Adding featuregate for cluster queue visibility (#1139)

* adding featuregate for cluster queue visibility * rename featuregate * move enabling feature to clusterqueue controller test * beforeAll instead of BeforeSuite * added test case where enableQueueVisibility is false * added ginkgo.ContinueOnFailure
kubernetes-sigs · Sep 20, 2023 · 542d315 · 542d315
1 parent 4e67b53
commit 542d315
Show file tree

Hide file tree

Showing 10 changed files with 111 additions and 25 deletions.
diff --git a/pkg/controller/core/clusterqueue_controller.go b/pkg/controller/core/clusterqueue_controller.go
@@ -40,6 +40,7 @@ import (
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/cache"
 	"sigs.k8s.io/kueue/pkg/constants"
+	"sigs.k8s.io/kueue/pkg/features"
 	"sigs.k8s.io/kueue/pkg/metrics"
 	"sigs.k8s.io/kueue/pkg/queue"
 	"sigs.k8s.io/kueue/pkg/util/resource"
@@ -624,7 +625,7 @@ func (r *ClusterQueueReconciler) updateCqStatusIfChanged(
 
 // Taking snapshot of cluster queue is enabled when maxcount non-zero
 func (r *ClusterQueueReconciler) isVisibilityEnabled() bool {
-	return r.queueVisibilityClusterQueuesMaxCount > 0
+	return features.Enabled(features.QueueVisibility) && r.queueVisibilityClusterQueuesMaxCount > 0
 }
 
 func (r *ClusterQueueReconciler) getWorkloadsStatus(cq *kueue.ClusterQueue) *kueue.ClusterQueuePendingWorkloadsStatus {

diff --git a/pkg/controller/core/clusterqueue_controller_test.go b/pkg/controller/core/clusterqueue_controller_test.go
@@ -31,6 +31,7 @@ import (
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/cache"
+	"sigs.k8s.io/kueue/pkg/features"
 	"sigs.k8s.io/kueue/pkg/metrics"
 	"sigs.k8s.io/kueue/pkg/queue"
 	utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
@@ -499,11 +500,16 @@ func TestClusterQueuePendingWorkloadsStatus(t *testing.T) {
 		queueVisibilityUpdateInterval        time.Duration
 		queueVisibilityClusterQueuesMaxCount int32
 		wantPendingWorkloadsStatus           *kueue.ClusterQueuePendingWorkloadsStatus
+		enableQueueVisibility                bool
 	}{
-		"taking snapshot of cluster queue is disabled": {},
-		"taking snapshot of cluster queue is enabled": {
+		"queue visibility is disabled": {},
+		"queue visibility is disabled but maxcount is provided": {
+			queueVisibilityClusterQueuesMaxCount: 2,
+		},
+		"queue visibility is enabled": {
 			queueVisibilityClusterQueuesMaxCount: 2,
 			queueVisibilityUpdateInterval:        10 * time.Millisecond,
+			enableQueueVisibility:                true,
 			wantPendingWorkloadsStatus: &kueue.ClusterQueuePendingWorkloadsStatus{
 				Head: []kueue.ClusterQueuePendingWorkload{
 					{Name: "one"}, {Name: "two"},
@@ -513,6 +519,7 @@ func TestClusterQueuePendingWorkloadsStatus(t *testing.T) {
 		"verify the head of pending workloads when the number of pending workloads exceeds MaxCount": {
 			queueVisibilityClusterQueuesMaxCount: 1,
 			queueVisibilityUpdateInterval:        10 * time.Millisecond,
+			enableQueueVisibility:                true,
 			wantPendingWorkloadsStatus: &kueue.ClusterQueuePendingWorkloadsStatus{
 				Head: []kueue.ClusterQueuePendingWorkload{
 					{Name: "one"},
@@ -521,6 +528,7 @@ func TestClusterQueuePendingWorkloadsStatus(t *testing.T) {
 		},
 	}
 	for name, tc := range testCases {
+		defer features.SetFeatureGateDuringTest(t, features.QueueVisibility, tc.enableQueueVisibility)()
 		t.Run(name, func(t *testing.T) {
 			cq := utiltesting.MakeClusterQueue(cqName).
 				QueueingStrategy(kueue.StrictFIFO).Obj()

diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go
@@ -33,6 +33,12 @@ const (
 	//
 	// Enables partial admission.
 	PartialAdmission featuregate.Feature = "PartialAdmission"
+	// owner: @stuton
+	// kep: https://github.com/kubernetes-sigs/kueue/tree/main/keps/168-pending-workloads-visibility
+	// alpha: v0.5
+	//
+	// Enables queue visibility.
+	QueueVisibility featuregate.Feature = "QueueVisibility"
 )
 
 func init() {
@@ -47,6 +53,7 @@ func init() {
 // when adding or removing one entry.
 var defaultFeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{
 	PartialAdmission: {Default: false, PreRelease: featuregate.Alpha},
+	QueueVisibility:  {Default: false, PreRelease: featuregate.Alpha},
 }
 
 func SetFeatureGateDuringTest(tb testing.TB, f featuregate.Feature, value bool) func() {

diff --git a/site/content/en/docs/installation/_index.md b/site/content/en/docs/installation/_index.md
@@ -226,3 +226,4 @@ The currently supported features are:
 | Feature | Default | Stage | Since | Until |
 |---------|---------|-------|-------|-------|
 | `PartialAdmission` | `false` | Alpha | 0.4 |  |
+| `QueueVisibility` | `false` | Alpha | 0.5 |  |
diff --git a/test/integration/controller/core/admissioncheck_controller_test.go b/test/integration/controller/core/admissioncheck_controller_test.go
@@ -26,14 +26,24 @@ import (
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
+	"sigs.k8s.io/kueue/test/integration/framework"
 	"sigs.k8s.io/kueue/test/util"
 )
 
 // +kubebuilder:docs-gen:collapse=Imports
 
-var _ = ginkgo.Describe("AdmissionCheck controller", func() {
+var _ = ginkgo.Describe("AdmissionCheck controller", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
 	var ns *corev1.Namespace
 
+	ginkgo.BeforeAll(func() {
+		fwk = &framework.Framework{CRDPath: crdPath, WebhookPath: webhookPath}
+		cfg = fwk.Init()
+		ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
+	})
+	ginkgo.AfterAll(func() {
+		fwk.Teardown()
+	})
+
 	ginkgo.BeforeEach(func() {
 		ns = &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{

diff --git a/test/integration/controller/core/clusterqueue_controller_test.go b/test/integration/controller/core/clusterqueue_controller_test.go
@@ -27,9 +27,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
+	"sigs.k8s.io/kueue/pkg/features"
 	"sigs.k8s.io/kueue/pkg/metrics"
 	"sigs.k8s.io/kueue/pkg/util/testing"
 	"sigs.k8s.io/kueue/pkg/workload"
+	"sigs.k8s.io/kueue/test/integration/framework"
 	"sigs.k8s.io/kueue/test/util"
 )
 
@@ -50,7 +52,7 @@ var ignoreConditionTimestamps = cmpopts.IgnoreFields(metav1.Condition{}, "LastTr
 var ignoreLastChangeTime = cmpopts.IgnoreFields(kueue.ClusterQueuePendingWorkloadsStatus{}, "LastChangeTime")
 var ignorePendingWorkloadsStatus = cmpopts.IgnoreFields(kueue.ClusterQueueStatus{}, "PendingWorkloadsStatus")
 
-var _ = ginkgo.Describe("ClusterQueue controller", func() {
+var _ = ginkgo.Describe("ClusterQueue controller", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
 	var (
 		ns               *corev1.Namespace
 		emptyUsedFlavors = []kueue.FlavorUsage{
@@ -81,6 +83,15 @@ var _ = ginkgo.Describe("ClusterQueue controller", func() {
 		}
 	)
 
+	ginkgo.BeforeAll(func() {
+		fwk = &framework.Framework{CRDPath: crdPath, WebhookPath: webhookPath}
+		cfg = fwk.Init()
+		ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
+	})
+	ginkgo.AfterAll(func() {
+		fwk.Teardown()
+	})
+
 	ginkgo.BeforeEach(func() {
 		ns = &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{
@@ -675,6 +686,35 @@ var _ = ginkgo.Describe("ClusterQueue controller", func() {
 			}, util.Timeout, util.Interval).Should(testing.BeNotFoundError())
 		})
 	})
+})
+
+var _ = ginkgo.Describe("ClusterQueue controller with queue visibility is enabled", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
+	var ns *corev1.Namespace
+
+	ginkgo.BeforeAll(func() {
+		ginkgo.By("Enabling queue visibility feature", func() {
+			gomega.Expect(features.SetEnable(features.QueueVisibility, true)).To(gomega.Succeed())
+		})
+		fwk = &framework.Framework{CRDPath: crdPath, WebhookPath: webhookPath}
+		cfg = fwk.Init()
+		ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
+	})
+	ginkgo.AfterAll(func() {
+		fwk.Teardown()
+	})
+
+	ginkgo.BeforeEach(func() {
+		ns = &corev1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				GenerateName: "core-clusterqueue-",
+			},
+		}
+		gomega.Expect(k8sClient.Create(ctx, ns)).To(gomega.Succeed())
+	})
+
+	ginkgo.AfterEach(func() {
+		gomega.Expect(util.DeleteNamespace(ctx, k8sClient, ns)).To(gomega.Succeed())
+	})
 
 	ginkgo.When("Reconciling clusterQueue pending workload status", func() {
 		var (

diff --git a/test/integration/controller/core/localqueue_controller_test.go b/test/integration/controller/core/localqueue_controller_test.go
@@ -26,12 +26,13 @@ import (
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/util/testing"
+	"sigs.k8s.io/kueue/test/integration/framework"
 	"sigs.k8s.io/kueue/test/util"
 )
 
 // +kubebuilder:docs-gen:collapse=Imports
 
-var _ = ginkgo.Describe("Queue controller", func() {
+var _ = ginkgo.Describe("Queue controller", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
 	const (
 		flavorModelC = "model-c"
 		flavorModelD = "model-d"
@@ -46,6 +47,15 @@ var _ = ginkgo.Describe("Queue controller", func() {
 		}
 	)
 
+	ginkgo.BeforeAll(func() {
+		fwk = &framework.Framework{CRDPath: crdPath, WebhookPath: webhookPath}
+		cfg = fwk.Init()
+		ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
+	})
+	ginkgo.AfterAll(func() {
+		fwk.Teardown()
+	})
+
 	ginkgo.BeforeEach(func() {
 		ns = &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{

diff --git a/test/integration/controller/core/resourceflavor_controller_test.go b/test/integration/controller/core/resourceflavor_controller_test.go
@@ -26,14 +26,24 @@ import (
 
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	utiltesting "sigs.k8s.io/kueue/pkg/util/testing"
+	"sigs.k8s.io/kueue/test/integration/framework"
 	"sigs.k8s.io/kueue/test/util"
 )
 
 // +kubebuilder:docs-gen:collapse=Imports
 
-var _ = ginkgo.Describe("ResourceFlavor controller", func() {
+var _ = ginkgo.Describe("ResourceFlavor controller", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
 	var ns *corev1.Namespace
 
+	ginkgo.BeforeAll(func() {
+		fwk = &framework.Framework{CRDPath: crdPath, WebhookPath: webhookPath}
+		cfg = fwk.Init()
+		ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
+	})
+	ginkgo.AfterAll(func() {
+		fwk.Teardown()
+	})
+
 	ginkgo.BeforeEach(func() {
 		ns = &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{

diff --git a/test/integration/controller/core/suite_test.go b/test/integration/controller/core/suite_test.go
@@ -38,10 +38,12 @@ import (
 )
 
 var (
-	cfg       *rest.Config
-	k8sClient client.Client
-	ctx       context.Context
-	fwk       *framework.Framework
+	cfg         *rest.Config
+	k8sClient   client.Client
+	ctx         context.Context
+	fwk         *framework.Framework
+	crdPath     = filepath.Join("..", "..", "..", "..", "config", "components", "crd", "bases")
+	webhookPath = filepath.Join("..", "..", "..", "..", "config", "components", "webhook")
 )
 
 func TestAPIs(t *testing.T) {
@@ -52,19 +54,6 @@ func TestAPIs(t *testing.T) {
 	)
 }
 
-var _ = ginkgo.BeforeSuite(func() {
-	fwk = &framework.Framework{
-		CRDPath:     filepath.Join("..", "..", "..", "..", "config", "components", "crd", "bases"),
-		WebhookPath: filepath.Join("..", "..", "..", "..", "config", "components", "webhook"),
-	}
-	cfg = fwk.Init()
-	ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
-})
-
-var _ = ginkgo.AfterSuite(func() {
-	fwk.Teardown()
-})
-
 func managerSetup(mgr manager.Manager, ctx context.Context) {
 	err := indexer.Setup(ctx, mgr.GetFieldIndexer())
 	gomega.Expect(err).NotTo(gomega.HaveOccurred())

diff --git a/test/integration/controller/core/workload_controller_test.go b/test/integration/controller/core/workload_controller_test.go
@@ -29,12 +29,13 @@ import (
 	kueue "sigs.k8s.io/kueue/apis/kueue/v1beta1"
 	"sigs.k8s.io/kueue/pkg/util/slices"
 	"sigs.k8s.io/kueue/pkg/util/testing"
+	"sigs.k8s.io/kueue/test/integration/framework"
 	"sigs.k8s.io/kueue/test/util"
 )
 
 // +kubebuilder:docs-gen:collapse=Imports
 
-var _ = ginkgo.Describe("Workload controller", func() {
+var _ = ginkgo.Describe("Workload controller", ginkgo.Ordered, ginkgo.ContinueOnFailure, func() {
 	var (
 		ns                   *corev1.Namespace
 		updatedQueueWorkload kueue.Workload
@@ -44,6 +45,15 @@ var _ = ginkgo.Describe("Workload controller", func() {
 		clusterQueue         *kueue.ClusterQueue
 	)
 
+	ginkgo.BeforeAll(func() {
+		fwk = &framework.Framework{CRDPath: crdPath, WebhookPath: webhookPath}
+		cfg = fwk.Init()
+		ctx, k8sClient = fwk.RunManager(cfg, managerSetup)
+	})
+	ginkgo.AfterAll(func() {
+		fwk.Teardown()
+	})
+
 	ginkgo.BeforeEach(func() {
 		ns = &corev1.Namespace{
 			ObjectMeta: metav1.ObjectMeta{