From 90568f24b1d01906c13ced9c7097c28dc820a418 Mon Sep 17 00:00:00 2001 From: Enrico Candino Date: Thu, 3 Apr 2025 16:26:25 +0200 Subject: [PATCH] Added ClusterSet as singleton (#316) * added ClusterSet as singleton * fix tests --- charts/k3k/crds/k3k.io_clustersets.yaml | 15 ++- pkg/apis/k3k.io/v1alpha1/types.go | 8 ++ pkg/controller/clusterset/clusterset.go | 109 ++++++++++--------- pkg/controller/clusterset/clusterset_test.go | 98 +++++++++++------ 4 files changed, 144 insertions(+), 86 deletions(-) diff --git a/charts/k3k/crds/k3k.io_clustersets.yaml b/charts/k3k/crds/k3k.io_clustersets.yaml index 424c5476..903bf088 100644 --- a/charts/k3k/crds/k3k.io_clustersets.yaml +++ b/charts/k3k/crds/k3k.io_clustersets.yaml @@ -14,7 +14,14 @@ spec: singular: clusterset scope: Namespaced versions: - - name: v1alpha1 + - additionalPrinterColumns: + - jsonPath: .spec.displayName + name: Display Name + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 schema: openAPIV3Schema: description: |- @@ -73,6 +80,9 @@ spec: description: DisableNetworkPolicy indicates whether to disable the creation of a default network policy for cluster isolation. type: boolean + displayName: + description: DisplayName is the human-readable name for the set. + type: string limit: description: |- Limit specifies the LimitRange that will be applied to all pods within the ClusterSet @@ -312,6 +322,9 @@ spec: required: - spec type: object + x-kubernetes-validations: + - message: Name must match 'default' + rule: self.metadata.name == "default" served: true storage: true subresources: diff --git a/pkg/apis/k3k.io/v1alpha1/types.go b/pkg/apis/k3k.io/v1alpha1/types.go index 16f785c1..2e3605a7 100644 --- a/pkg/apis/k3k.io/v1alpha1/types.go +++ b/pkg/apis/k3k.io/v1alpha1/types.go @@ -313,6 +313,9 @@ type ClusterList struct { // +kubebuilder:storageversion // +kubebuilder:subresource:status // +kubebuilder:object:root=true +// +kubebuilder:validation:XValidation:rule="self.metadata.name == \"default\"",message="Name must match 'default'" +// +kubebuilder:printcolumn:JSONPath=".spec.displayName",name=Display Name,type=string +// +kubebuilder:printcolumn:JSONPath=".metadata.creationTimestamp",name=Age,type=date // ClusterSet represents a group of virtual Kubernetes clusters managed by k3k. // It allows defining common configurations and constraints for the clusters within the set. @@ -334,6 +337,11 @@ type ClusterSet struct { // ClusterSetSpec defines the desired state of a ClusterSet. type ClusterSetSpec struct { + // DisplayName is the human-readable name for the set. + // + // +optional + DisplayName string `json:"displayName"` + // Quota specifies the resource limits for clusters within a clusterset. // // +optional diff --git a/pkg/controller/clusterset/clusterset.go b/pkg/controller/clusterset/clusterset.go index b0b5818d..f72dfed6 100644 --- a/pkg/controller/clusterset/clusterset.go +++ b/pkg/controller/clusterset/clusterset.go @@ -48,6 +48,7 @@ func Add(ctx context.Context, mgr manager.Manager, clusterCIDR string) error { return ctrl.NewControllerManagedBy(mgr). For(&v1alpha1.ClusterSet{}). Owns(&networkingv1.NetworkPolicy{}). + Owns(&v1.ResourceQuota{}). WithOptions(controller.Options{ MaxConcurrentReconciles: maxConcurrentReconciles, }). @@ -58,54 +59,33 @@ func Add(ctx context.Context, mgr manager.Manager, clusterCIDR string) error { ). Watches( &v1alpha1.Cluster{}, - handler.EnqueueRequestsFromMapFunc(sameNamespaceEventHandler(reconciler)), + handler.EnqueueRequestsFromMapFunc(namespaceEventHandler(reconciler)), ). Complete(&reconciler) } -// namespaceEventHandler will enqueue reconciling requests for all the ClusterSets in the changed namespace +// namespaceEventHandler will enqueue a reconcile request for the ClusterSet in the given namespace func namespaceEventHandler(reconciler ClusterSetReconciler) handler.MapFunc { return func(ctx context.Context, obj client.Object) []reconcile.Request { - var ( - requests []reconcile.Request - set v1alpha1.ClusterSetList - ) + // if the object is a Namespace, use the name as the namespace + namespace := obj.GetName() - _ = reconciler.Client.List(ctx, &set, client.InNamespace(obj.GetName())) - - for _, clusterSet := range set.Items { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: clusterSet.Name, - Namespace: obj.GetName(), - }, - }) + // if the object is a namespaced resource, use the namespace + if obj.GetNamespace() != "" { + namespace = obj.GetNamespace() } - return requests - } -} - -// sameNamespaceEventHandler will enqueue reconciling requests for all the ClusterSets in the changed namespace -func sameNamespaceEventHandler(reconciler ClusterSetReconciler) handler.MapFunc { - return func(ctx context.Context, obj client.Object) []reconcile.Request { - var ( - requests []reconcile.Request - set v1alpha1.ClusterSetList - ) - - _ = reconciler.Client.List(ctx, &set, client.InNamespace(obj.GetNamespace())) - - for _, clusterSet := range set.Items { - requests = append(requests, reconcile.Request{ - NamespacedName: types.NamespacedName{ - Name: clusterSet.Name, - Namespace: obj.GetNamespace(), - }, - }) + key := types.NamespacedName{ + Name: "default", + Namespace: namespace, } - return requests + var clusterSet v1alpha1.ClusterSet + if err := reconciler.Client.Get(ctx, key, &clusterSet); err != nil { + return nil + } + + return []reconcile.Request{{NamespacedName: key}} } } @@ -130,29 +110,56 @@ func (c *ClusterSetReconciler) Reconcile(ctx context.Context, req reconcile.Requ return reconcile.Result{}, client.IgnoreNotFound(err) } - if err := c.reconcileNetworkPolicy(ctx, &clusterSet); err != nil { - return reconcile.Result{}, err + orig := clusterSet.DeepCopy() + + reconcilerErr := c.reconcileClusterSet(ctx, &clusterSet) + + // update Status if needed + if !reflect.DeepEqual(orig.Status, clusterSet.Status) { + if err := c.Client.Status().Update(ctx, &clusterSet); err != nil { + return reconcile.Result{}, err + } } - if err := c.reconcileNamespacePodSecurityLabels(ctx, &clusterSet); err != nil { - return reconcile.Result{}, err + // if there was an error during the reconciliation, return + if reconcilerErr != nil { + return reconcile.Result{}, reconcilerErr } - if err := c.reconcileClusters(ctx, &clusterSet); err != nil { - return reconcile.Result{}, err - } - - if err := c.reconcileLimit(ctx, &clusterSet); err != nil { - return reconcile.Result{}, err - } - - if err := c.reconcileQuota(ctx, &clusterSet); err != nil { - return reconcile.Result{}, err + // update ClusterSet if needed + if !reflect.DeepEqual(orig.Spec, clusterSet.Spec) { + if err := c.Client.Update(ctx, &clusterSet); err != nil { + return reconcile.Result{}, err + } } return reconcile.Result{}, nil } +func (c *ClusterSetReconciler) reconcileClusterSet(ctx context.Context, clusterSet *v1alpha1.ClusterSet) error { + if err := c.reconcileNetworkPolicy(ctx, clusterSet); err != nil { + return err + } + + if err := c.reconcileNamespacePodSecurityLabels(ctx, clusterSet); err != nil { + return err + } + + if err := c.reconcileLimit(ctx, clusterSet); err != nil { + return err + } + + if err := c.reconcileQuota(ctx, clusterSet); err != nil { + return err + } + + if err := c.reconcileClusters(ctx, clusterSet); err != nil { + return err + } + + return nil +} + func (c *ClusterSetReconciler) reconcileNetworkPolicy(ctx context.Context, clusterSet *v1alpha1.ClusterSet) error { log := ctrl.LoggerFrom(ctx) log.Info("reconciling NetworkPolicy") diff --git a/pkg/controller/clusterset/clusterset_test.go b/pkg/controller/clusterset/clusterset_test.go index 9d97fe8b..3b44faf6 100644 --- a/pkg/controller/clusterset/clusterset_test.go +++ b/pkg/controller/clusterset/clusterset_test.go @@ -39,8 +39,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should have only the 'shared' allowedModeTypes", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, } @@ -52,11 +52,39 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet Expect(allowedModeTypes).To(ContainElement(v1alpha1.SharedClusterMode)) }) + It("should not be able to create a cluster with a non 'default' name", func() { + err := k8sClient.Create(ctx, &v1alpha1.ClusterSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "another-name", + Namespace: namespace, + }, + }) + Expect(err).To(HaveOccurred()) + }) + + It("should not be able to create two ClusterSets in the same namespace", func() { + err := k8sClient.Create(ctx, &v1alpha1.ClusterSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + Namespace: namespace, + }, + }) + Expect(err).To(Not(HaveOccurred())) + + err = k8sClient.Create(ctx, &v1alpha1.ClusterSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default-2", + Namespace: namespace, + }, + }) + Expect(err).To(HaveOccurred()) + }) + It("should create a NetworkPolicy", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, } @@ -119,8 +147,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should not create a NetworkPolicy if true", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ DisableNetworkPolicy: true, @@ -148,8 +176,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should delete the NetworkPolicy if changed to false", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, } @@ -192,8 +220,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should recreate the NetworkPolicy if deleted", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, } @@ -243,8 +271,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should have the 'virtual' mode if specified", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ AllowedModeTypes: []v1alpha1.ClusterMode{ @@ -264,8 +292,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should have both modes if specified", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ AllowedModeTypes: []v1alpha1.ClusterMode{ @@ -289,8 +317,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should fail for a non-existing mode", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ AllowedModeTypes: []v1alpha1.ClusterMode{ @@ -316,8 +344,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ PodSecurityAdmissionLevel: &privileged, @@ -419,8 +447,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ PodSecurityAdmissionLevel: &privileged, @@ -470,8 +498,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should update it if needed", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ DefaultPriorityClass: "foobar", @@ -511,8 +539,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should update the nodeSelector", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ DefaultNodeSelector: map[string]string{"label-1": "value-1"}, @@ -552,8 +580,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should update the nodeSelector if changed", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ DefaultNodeSelector: map[string]string{"label-1": "value-1"}, @@ -623,8 +651,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should not be update", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ DefaultPriorityClass: "foobar", @@ -671,8 +699,8 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet It("should create resourceQuota if Quota is enabled", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ Quota: &v1.ResourceQuotaSpec{ @@ -702,11 +730,12 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet Expect(resourceQuota.Spec.Hard.Cpu().String()).To(BeEquivalentTo("800m")) Expect(resourceQuota.Spec.Hard.Memory().String()).To(BeEquivalentTo("1Gi")) }) + It("should delete the ResourceQuota if Quota is deleted", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ Quota: &v1.ResourceQuotaSpec{ @@ -751,11 +780,12 @@ var _ = Describe("ClusterSet Controller", Label("controller"), Label("ClusterSet WithPolling(time.Second). Should(BeTrue()) }) + It("should create resourceQuota if Quota is enabled", func() { clusterSet := &v1alpha1.ClusterSet{ ObjectMeta: metav1.ObjectMeta{ - GenerateName: "clusterset-", - Namespace: namespace, + Name: "default", + Namespace: namespace, }, Spec: v1alpha1.ClusterSetSpec{ Limit: &v1.LimitRangeSpec{