update based on comments

kubernetes-sigs · May 14, 2024 · 646f495 · 646f495
1 parent a5d7cf3
commit 646f495
Show file tree

Hide file tree

Showing 13 changed files with 216 additions and 127 deletions.
diff --git a/apis/visibility/v1alpha1/openapi/zz_generated.openapi.go b/apis/visibility/v1alpha1/openapi/zz_generated.openapi.go
diff --git a/apis/visibility/v1alpha1/types.go b/apis/visibility/v1alpha1/types.go
@@ -86,6 +86,8 @@ type RunningWorkload struct {
 
 	// Priority indicates the workload's priority
 	Priority int32 `json:"priority"`
+	// AdmissionTime indecates the time workloads admitted
+	AdmissionTime metav1.Time `json:"admissionTime"`
 }
 
 // +k8s:openapi-gen=true
@@ -133,6 +135,22 @@ type PendingWorkloadsSummaryList struct {
 // +k8s:conversion-gen:explicit-from=net/url.Values
 // +k8s:defaulter-gen=true
 
+// RunningWorkloadOptions are query params used in the visibility queries
+type RunningWorkloadOptions struct {
+	metav1.TypeMeta `json:",inline"`
+
+	// Offset indicates position of the first pending workload that should be fetched, starting from 0. 0 by default
+	Offset int64 `json:"offset"`
+
+	// Limit indicates max number of pending workloads that should be fetched. 1000 by default
+	Limit int64 `json:"limit,omitempty"`
+}
+
+// +kubebuilder:object:root=true
+// +k8s:openapi-gen=true
+// +k8s:conversion-gen:explicit-from=net/url.Values
+// +k8s:defaulter-gen=true
+
 // PendingWorkloadOptions are query params used in the visibility queries
 type PendingWorkloadOptions struct {
 	metav1.TypeMeta `json:",inline"`
@@ -149,5 +167,6 @@ func init() {
 		&PendingWorkloadsSummary{},
 		&PendingWorkloadOptions{},
 		&RunningWorkloadsSummary{},
+		&RunningWorkloadOptions{},
 	)
 }
diff --git a/apis/visibility/v1alpha1/zz_generated.conversion.go b/apis/visibility/v1alpha1/zz_generated.conversion.go
diff --git a/apis/visibility/v1alpha1/zz_generated.deepcopy.go b/apis/visibility/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/cache/cache.go b/pkg/cache/cache.go
@@ -699,8 +699,8 @@ func (c *Cache) RunningWorkload(name string) ([]*workload.Info, error) {
 
 	count := 0
 	wkls := make([]*workload.Info, 0, len(cq.Workloads))
-	for k, wkl := range cq.Workloads {
-		if _, ok := c.assumedWorkloads[k]; ok {
+	for _, wkl := range cq.Workloads {
+		if !workload.IsAdmitted(wkl.Obj) || workload.IsFinished(wkl.Obj) {
 			continue
 		}
 		wkls = append(wkls, wkl)

diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go
@@ -40,6 +40,7 @@ const (
 	PodPriorityClassSource      = "scheduling.k8s.io/priorityclass"
 
 	DefaultPendingWorkloadsLimit = 1000
+	DefaultRunningWorkloadsLimit = 1000
 
 	IsNegativeErrorMsg string = `must be greater than or equal to 0`
 )
diff --git a/pkg/visibility/api/rest/pending_workloads_cq_test.go b/pkg/visibility/api/rest/pending_workloads_cq_test.go
@@ -63,8 +63,8 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 		clusterQueues []*kueue.ClusterQueue
 		queues        []*kueue.LocalQueue
 		workloads     []*kueue.Workload
-		req           *req
-		wantResp      *resp
+		req           *pendingReq
+		wantResp      *pendingResp
 		wantErrMatch  func(error) bool
 	}{
 		"single ClusterQueue and single LocalQueue setup with two workloads and default query parameters": {
@@ -78,11 +78,11 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 				utiltesting.MakeWorkload("a", nsName).Queue(lqNameA).Priority(highPrio).Creation(now).Obj(),
 				utiltesting.MakeWorkload("b", nsName).Queue(lqNameA).Priority(lowPrio).Creation(now).Obj(),
 			},
-			req: &req{
+			req: &pendingReq{
 				queueName:   cqNameA,
 				queryParams: defaultQueryParams,
 			},
-			wantResp: &resp{
+			wantResp: &pendingResp{
 				wantPendingWorkloads: []visibility.PendingWorkload{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -122,11 +122,11 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 				utiltesting.MakeWorkload("lqB-high-prio", nsName).Queue(lqNameB).Priority(highPrio).Creation(now.Add(time.Second)).Obj(),
 				utiltesting.MakeWorkload("lqB-low-prio", nsName).Queue(lqNameB).Priority(lowPrio).Creation(now.Add(time.Second)).Obj(),
 			},
-			req: &req{
+			req: &pendingReq{
 				queueName:   cqNameA,
 				queryParams: defaultQueryParams,
 			},
-			wantResp: &resp{
+			wantResp: &pendingResp{
 				wantPendingWorkloads: []visibility.PendingWorkload{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -186,13 +186,13 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 				utiltesting.MakeWorkload("b", nsName).Queue(lqNameA).Priority(highPrio).Creation(now.Add(time.Second)).Obj(),
 				utiltesting.MakeWorkload("c", nsName).Queue(lqNameA).Priority(highPrio).Creation(now.Add(time.Second * 2)).Obj(),
 			},
-			req: &req{
+			req: &pendingReq{
 				queueName: cqNameA,
 				queryParams: &visibility.PendingWorkloadOptions{
 					Limit: 2,
 				},
 			},
-			wantResp: &resp{
+			wantResp: &pendingResp{
 				wantPendingWorkloads: []visibility.PendingWorkload{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -230,14 +230,14 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 				utiltesting.MakeWorkload("b", nsName).Queue(lqNameA).Priority(highPrio).Creation(now.Add(time.Second)).Obj(),
 				utiltesting.MakeWorkload("c", nsName).Queue(lqNameA).Priority(highPrio).Creation(now.Add(time.Second * 2)).Obj(),
 			},
-			req: &req{
+			req: &pendingReq{
 				queueName: cqNameA,
 				queryParams: &visibility.PendingWorkloadOptions{
 					Offset: 1,
 					Limit:  constants.DefaultPendingWorkloadsLimit,
 				},
 			},
-			wantResp: &resp{
+			wantResp: &pendingResp{
 				wantPendingWorkloads: []visibility.PendingWorkload{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -275,14 +275,14 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 				utiltesting.MakeWorkload("b", nsName).Queue(lqNameA).Priority(highPrio).Creation(now.Add(time.Second)).Obj(),
 				utiltesting.MakeWorkload("c", nsName).Queue(lqNameA).Priority(highPrio).Creation(now.Add(time.Second * 2)).Obj(),
 			},
-			req: &req{
+			req: &pendingReq{
 				queueName: cqNameA,
 				queryParams: &visibility.PendingWorkloadOptions{
 					Offset: 1,
 					Limit:  1,
 				},
 			},
-			wantResp: &resp{
+			wantResp: &pendingResp{
 				wantPendingWorkloads: []visibility.PendingWorkload{
 					{
 						ObjectMeta: v1.ObjectMeta{
@@ -301,18 +301,18 @@ func TestPendingWorkloadsInCQ(t *testing.T) {
 			clusterQueues: []*kueue.ClusterQueue{
 				utiltesting.MakeClusterQueue(cqNameA).Obj(),
 			},
-			req: &req{
+			req: &pendingReq{
 				queueName:   cqNameA,
 				queryParams: defaultQueryParams,
 			},
-			wantResp: &resp{},
+			wantResp: &pendingResp{},
 		},
 		"nonexistent queue name": {
-			req: &req{
+			req: &pendingReq{
 				queueName:   "nonexistent-queue",
 				queryParams: defaultQueryParams,
 			},
-			wantResp: &resp{
+			wantResp: &pendingResp{
 				wantErr: errors.NewNotFound(visibility.Resource("clusterqueue"), "nonexistent-queue"),
 			},
 			wantErrMatch: errors.IsNotFound,