Review Remarks

kubernetes-sigs · Jan 25, 2024 · a2a8538 · a2a8538
1 parent d754712
commit a2a8538
Show file tree

Hide file tree

Showing 10 changed files with 134 additions and 94 deletions.
diff --git a/apis/kueue/v1alpha1/multikueueconfig_types.go b/apis/kueue/v1alpha1/multikueueconfig_types.go
@@ -52,10 +52,15 @@ type KubeConfig struct {
 
 type MultiKueueClusterSpec struct {
 	// Information how to connect to the cluster.
-	KubeConfig KubeConfig `json:"kubeconfigRef"`
+	KubeConfig KubeConfig `json:"kubeConfig"`
 }
 
 type MultiKueueClusterStatus struct {
+	// +optional
+	// +listType=map
+	// +listMapKey=type
+	// +patchStrategy=merge
+	// +patchMergeKey=type
 	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 }
 

diff --git a/charts/kueue/templates/crd/kueue.x-k8s.io_multikueueclusters.yaml b/charts/kueue/templates/crd/kueue.x-k8s.io_multikueueclusters.yaml
@@ -46,7 +46,7 @@ spec:
             type: object
           spec:
             properties:
-              kubeconfigRef:
+              kubeConfig:
                 description: Information how to connect to the cluster.
                 properties:
                   location:
@@ -67,7 +67,7 @@ spec:
                 - name
                 type: object
             required:
-            - kubeconfigRef
+            - kubeConfig
             type: object
           status:
             properties:
@@ -139,6 +139,9 @@ spec:
                   - type
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - type
+                x-kubernetes-list-type: map
             type: object
         type: object
     served: true

diff --git a/client-go/applyconfiguration/kueue/v1alpha1/multikueueclusterspec.go b/client-go/applyconfiguration/kueue/v1alpha1/multikueueclusterspec.go
diff --git a/config/components/crd/bases/kueue.x-k8s.io_multikueueclusters.yaml b/config/components/crd/bases/kueue.x-k8s.io_multikueueclusters.yaml
@@ -33,7 +33,7 @@ spec:
             type: object
           spec:
             properties:
-              kubeconfigRef:
+              kubeConfig:
                 description: Information how to connect to the cluster.
                 properties:
                   location:
@@ -54,7 +54,7 @@ spec:
                 - name
                 type: object
             required:
-            - kubeconfigRef
+            - kubeConfig
             type: object
           status:
             properties:
@@ -126,6 +126,9 @@ spec:
                   - type
                   type: object
                 type: array
+                x-kubernetes-list-map-keys:
+                - type
+                x-kubernetes-list-type: map
             type: object
         type: object
     served: true

diff --git a/pkg/controller/admissionchecks/multikueue/admissioncheck_test.go b/pkg/controller/admissionchecks/multikueue/admissioncheck_test.go
@@ -129,6 +129,35 @@ func TestReconcile(t *testing.T) {
 					Obj(),
 			},
 		},
+		"missing and inactive cluster": {
+			reconcileFor: "ac1",
+			checks: []kueue.AdmissionCheck{
+				*utiltesting.MakeAdmissionCheck("ac1").
+					ControllerName(ControllerName).
+					Parameters(kueue.GroupVersion.Group, "MultiKueueConfig", "config1").
+					Obj(),
+			},
+			configs: []kueuealpha.MultiKueueConfig{
+				*utiltesting.MakeMultiKueueConfig("config1").Clusters("worker1", "worker2", "worker3").Obj(),
+			},
+
+			clusters: []kueuealpha.MultiKueueCluster{
+				*utiltesting.MakeMultiKueueCluster("worker1").Active(metav1.ConditionFalse, "ByTest", "by test").Obj(),
+				*utiltesting.MakeMultiKueueCluster("worker2").Active(metav1.ConditionTrue, "ByTest", "by test").Obj(),
+			},
+			wantChecks: []kueue.AdmissionCheck{
+				*utiltesting.MakeAdmissionCheck("ac1").
+					ControllerName(ControllerName).
+					Parameters(kueue.GroupVersion.Group, "MultiKueueConfig", "config1").
+					Condition(metav1.Condition{
+						Type:    kueue.AdmissionCheckActive,
+						Status:  metav1.ConditionFalse,
+						Reason:  "Inactive",
+						Message: "Missing clusters: [worker3], Inactive clusters: [worker1]",
+					}).
+					Obj(),
+			},
+		},
 		"active": {
 			reconcileFor: "ac1",
 			checks: []kueue.AdmissionCheck{

diff --git a/pkg/controller/admissionchecks/multikueue/indexer_test.go b/pkg/controller/admissionchecks/multikueue/indexer_test.go
@@ -103,7 +103,7 @@ func TestListMultikueClustersUsingKubeconfig(t *testing.T) {
 			k8sclient := builder.Build()
 			for _, req := range tc.clusters {
 				if err := k8sclient.Create(ctx, req); err != nil {
-					t.Errorf("Unable to create %s request: %v", client.ObjectKeyFromObject(req), err)
+					t.Fatalf("Unable to create %s cluster: %v", client.ObjectKeyFromObject(req), err)
 				}
 			}
 
@@ -160,7 +160,7 @@ func TestListMultikueConfigsUsingMultikueueClusters(t *testing.T) {
 			k8sclient := builder.Build()
 			for _, config := range tc.configs {
 				if err := k8sclient.Create(ctx, config); err != nil {
-					t.Errorf("Unable to create %s config: %v", client.ObjectKeyFromObject(config), err)
+					t.Fatalf("Unable to create %s config: %v", client.ObjectKeyFromObject(config), err)
 				}
 			}
 

diff --git a/pkg/controller/admissionchecks/multikueue/multikueuecluster.go b/pkg/controller/admissionchecks/multikueue/multikueuecluster.go
@@ -213,7 +213,7 @@ func (c *clustersReconciler) Reconcile(ctx context.Context, req reconcile.Reques
 	// get the kubeconfig
 	kubeConfig, retry, err := c.getKubeConfig(ctx, &cluster.Spec.KubeConfig)
 	if retry {
-		return reconcile.Result{}, nil
+		return reconcile.Result{}, err
 	}
 	if err != nil {
 		log.Error(err, "reading kubeconfig")

diff --git a/test/e2e/multikueue/e2e_test.go b/test/e2e/multikueue/e2e_test.go
@@ -49,13 +49,13 @@ var _ = ginkgo.Describe("MultiKueue", func() {
 		worker1Ns *corev1.Namespace
 		worker2Ns *corev1.Namespace
 
-		multiKueueCluster1 *kueuealpha.MultiKueueCluster
-		multiKueueCluster2 *kueuealpha.MultiKueueCluster
-		multiKueueConfig   *kueuealpha.MultiKueueConfig
-		multiKueueAc       *kueue.AdmissionCheck
-		managerFlavor      *kueue.ResourceFlavor
-		managerCq          *kueue.ClusterQueue
-		managerLq          *kueue.LocalQueue
+		workerCluster1   *kueuealpha.MultiKueueCluster
+		workerCluster2   *kueuealpha.MultiKueueCluster
+		multiKueueConfig *kueuealpha.MultiKueueConfig
+		multiKueueAc     *kueue.AdmissionCheck
+		managerFlavor    *kueue.ResourceFlavor
+		managerCq        *kueue.ClusterQueue
+		managerLq        *kueue.LocalQueue
 
 		worker1Flavor *kueue.ResourceFlavor
 		worker1Cq     *kueue.ClusterQueue
@@ -88,11 +88,11 @@ var _ = ginkgo.Describe("MultiKueue", func() {
 		}
 		gomega.Expect(k8sWorker2Client.Create(ctx, worker2Ns)).To(gomega.Succeed())
 
-		multiKueueCluster1 = utiltesting.MakeMultiKueueCluster("worker1").Secret("worker1", "multikueue1").Obj()
-		gomega.Expect(k8sManagerClient.Create(ctx, multiKueueCluster1)).To(gomega.Succeed())
+		workerCluster1 = utiltesting.MakeMultiKueueCluster("worker1").Secret("worker1", "multikueue1").Obj()
+		gomega.Expect(k8sManagerClient.Create(ctx, workerCluster1)).To(gomega.Succeed())
 
-		multiKueueCluster2 = utiltesting.MakeMultiKueueCluster("worker2").Secret("worker2", "multikueue2").Obj()
-		gomega.Expect(k8sManagerClient.Create(ctx, multiKueueCluster2)).To(gomega.Succeed())
+		workerCluster2 = utiltesting.MakeMultiKueueCluster("worker2").Secret("worker2", "multikueue2").Obj()
+		gomega.Expect(k8sManagerClient.Create(ctx, workerCluster2)).To(gomega.Succeed())
 
 		multiKueueConfig = utiltesting.MakeMultiKueueConfig("multikueueconfig").Clusters("worker1", "worker2").Obj()
 		gomega.Expect(k8sManagerClient.Create(ctx, multiKueueConfig)).Should(gomega.Succeed())
@@ -177,8 +177,8 @@ var _ = ginkgo.Describe("MultiKueue", func() {
 		util.ExpectResourceFlavorToBeDeleted(ctx, k8sManagerClient, managerFlavor, true)
 		util.ExpectAdmissionCheckToBeDeleted(ctx, k8sManagerClient, multiKueueAc, true)
 		gomega.Expect(k8sManagerClient.Delete(ctx, multiKueueConfig)).To(gomega.Succeed())
-		gomega.Expect(k8sManagerClient.Delete(ctx, multiKueueCluster1)).To(gomega.Succeed())
-		gomega.Expect(k8sManagerClient.Delete(ctx, multiKueueCluster2)).To(gomega.Succeed())
+		gomega.Expect(k8sManagerClient.Delete(ctx, workerCluster1)).To(gomega.Succeed())
+		gomega.Expect(k8sManagerClient.Delete(ctx, workerCluster2)).To(gomega.Succeed())
 	})
 
 	ginkgo.When("Creating a multikueue admission check", func() {