Added allowedNodeTypes to ClusterSet, and fixed NetworkPolicy reconciliation (#144)

* updated CRDs

* added Mode to ClusterSet, and enum to CRD

* fix typos

* fix mode type in cli

* deletion of second clusterset in same namespace

* removed focused test, added clusterset example

* renamed modes

* added allowedNodeTypes, fixed samples

* fixed network policy reconciliation

charts/k3k/crds/k3k.io_clusters.yaml

@@ -130,19 +130,26 @@ spec:
                     type: object
                 type: object
               mode:
+                allOf:
+                - enum:
+                  - shared
+                  - virtual
+                - enum:
+                  - shared
+                  - virtual
+                default: shared
                 description: Mode is the cluster provisioning mode which can be either
-                  "virtual" or "shared". Defaults to "shared"
+                  "shared" or "virtual". Defaults to "shared"
                 type: string
                 x-kubernetes-validations:
                 - message: mode is immutable
                   rule: self == oldSelf
-                - message: invalid value for mode
-                  rule: self == "virtual" || self == "shared"
               nodeSelector:
                 additionalProperties:
                   type: string
-                description: NodeSelector is the node selector that will be applied
-                  to all server/agent pods
+                description: |-
+                  NodeSelector is the node selector that will be applied to all server/agent pods.
+                  In "shared" mode the node selector will be applied also to the workloads.
                 type: object
               persistence:
                 description: |-

charts/k3k/crds/k3k.io_clustersets.yaml

@@ -36,8 +36,26 @@ spec:
           metadata:
             type: object
           spec:
+            default: {}
             description: Spec is the spec of the ClusterSet
             properties:
+              allowedNodeTypes:
+                default:
+                - shared
+                description: AllowedNodeTypes are the allowed cluster provisioning
+                  modes. Defaults to [shared].
+                items:
+                  description: ClusterMode is the possible provisioning mode of a
+                    Cluster.
+                  enum:
+                  - shared
+                  - virtual
+                  type: string
+                minItems: 1
+                type: array
+                x-kubernetes-validations:
+                - message: mode is immutable
+                  rule: self == oldSelf
               defaultLimits:
                 description: DefaultLimits are the limits used for servers/agents
                   when a cluster in the set doesn't provide any
@@ -168,7 +186,7 @@ spec:
                 format: int64
                 type: integer
               summary:
-                description: Sumamry is a summary of the status (error, ready)
+                description: Summary is a summary of the status
                 type: string
             type: object
         required:

cli/cmds/cluster/create.go

@@ -226,6 +226,9 @@ func validateCreateFlags() error {
 	if cmds.Kubeconfig == "" && os.Getenv("KUBECONFIG") == "" {
 		return errors.New("empty kubeconfig")
 	}
+	if mode != "shared" && mode != "virtual" {
+		return errors.New(`mode should be one of "shared" or "virtual"`)
+	}
 
 	return nil
 }
@@ -248,7 +251,7 @@ func newCluster(name, namespace, mode, token string, servers, agents int32, clus
 			ServerArgs:  serverArgs,
 			AgentArgs:   agentArgs,
 			Version:     version,
-			Mode:        mode,
+			Mode:        v1alpha1.ClusterMode(mode),
 			Persistence: &v1alpha1.PersistenceConfig{
 				Type:             persistenceType,
 				StorageClassName: storageClassName,

examples/clusterset.yaml

@@ -0,0 +1,9 @@
+apiVersion: k3k.io/v1alpha1
+kind: ClusterSet
+metadata:
+  name: clusterset-example
+# spec:
+  # disableNetworkPolicy: false
+  # allowedNodeTypes:
+  # - "shared"
+  # - "virtual"

examples/multiple-servers.yaml

@@ -3,6 +3,7 @@ kind: Cluster
 metadata:
   name: example1
 spec:
+  mode: "shared"
   servers: 1
   agents: 3
   token: test

examples/single-server.yaml

@@ -3,6 +3,7 @@ kind: Cluster
 metadata:
   name: single-server
 spec:
+  mode: "shared"
   servers: 1
   agents: 3
   token: test

pkg/apis/k3k.io/v1alpha1/set_types.go

@@ -14,8 +14,11 @@ type ClusterSet struct {
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 	metav1.TypeMeta   `json:",inline"`
 
+	// +kubebuilder:default={}
+	//
 	// Spec is the spec of the ClusterSet
 	Spec ClusterSetSpec `json:"spec"`
+
 	// Status is the status of the ClusterSet
 	Status ClusterSetStatus `json:"status,omitempty"`
 }
@@ -23,21 +26,35 @@ type ClusterSet struct {
 type ClusterSetSpec struct {
 	// MaxLimits are the limits that apply to all clusters (server + agent) in the set
 	MaxLimits v1.ResourceList `json:"maxLimits,omitempty"`
+
 	// DefaultLimits are the limits used for servers/agents when a cluster in the set doesn't provide any
 	DefaultLimits *ClusterLimit `json:"defaultLimits,omitempty"`
+
 	// DefaultNodeSelector is the node selector that applies to all clusters (server + agent) in the set
 	DefaultNodeSelector map[string]string `json:"defaultNodeSelector,omitempty"`
+
 	// DisableNetworkPolicy is an option that will disable the creation of a default networkpolicy for cluster isolation
 	DisableNetworkPolicy bool `json:"disableNetworkPolicy,omitempty"`
+
+	// +kubebuilder:default={shared}
+	// +kubebuilder:validation:XValidation:message="mode is immutable",rule="self == oldSelf"
+	// +kubebuilder:validation:MinItems=1
+	//
+	// AllowedNodeTypes are the allowed cluster provisioning modes. Defaults to [shared].
+	AllowedNodeTypes []ClusterMode `json:"allowedNodeTypes,omitempty"`
 }
 
 type ClusterSetStatus struct {
+
 	// ObservedGeneration was the generation at the time the status was updated.
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+
 	// LastUpdate is the timestamp when the status was last updated
 	LastUpdate string `json:"lastUpdateTime,omitempty"`
-	// Sumamry is a summary of the status (error, ready)
+
+	// Summary is a summary of the status
 	Summary string `json:"summary,omitempty"`
+
 	// Conditions are the invidual conditions for the cluster set
 	Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`
 }

pkg/apis/k3k.io/v1alpha1/types.go

@@ -21,54 +21,80 @@ type Cluster struct {
 type ClusterSpec struct {
 	// Version is a string representing the Kubernetes version to be used by the virtual nodes.
 	Version string `json:"version"`
-	// +kubebuilder:validation:XValidation:message="cluster must have at least one server",rule="self >= 1"
+
 	// Servers is the number of K3s pods to run in server (controlplane) mode.
+	// +kubebuilder:validation:XValidation:message="cluster must have at least one server",rule="self >= 1"
 	Servers *int32 `json:"servers"`
-	// +kubebuilder:validation:XValidation:message="invalid value for agents",rule="self >= 0"
+
 	// Agents is the number of K3s pods to run in agent (worker) mode.
+	// +kubebuilder:validation:XValidation:message="invalid value for agents",rule="self >= 0"
 	Agents *int32 `json:"agents"`
-	// +optional
+
 	// NodeSelector is the node selector that will be applied to all server/agent pods.
 	// In "shared" mode the node selector will be applied also to the workloads.
+	// +optional
 	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+
 	// Limit is the limits that apply for the server/worker nodes.
 	Limit *ClusterLimit `json:"clusterLimit,omitempty"`
-	// +optional
+
 	// TokenSecretRef is Secret reference used as a token join server and worker nodes to the cluster. The controller
 	// assumes that the secret has a field "token" in its data, any other fields in the secret will be ignored.
+	// +optional
 	TokenSecretRef *v1.SecretReference `json:"tokenSecretRef"`
-	// +kubebuilder:validation:XValidation:message="clusterCIDR is immutable",rule="self == oldSelf"
+
 	// ClusterCIDR is the CIDR range for the pods of the cluster. Defaults to 10.42.0.0/16.
+	// +kubebuilder:validation:XValidation:message="clusterCIDR is immutable",rule="self == oldSelf"
 	ClusterCIDR string `json:"clusterCIDR,omitempty"`
-	// +kubebuilder:validation:XValidation:message="serviceCIDR is immutable",rule="self == oldSelf"
+
 	// ServiceCIDR is the CIDR range for the services in the cluster. Defaults to 10.43.0.0/16.
+	// +kubebuilder:validation:XValidation:message="serviceCIDR is immutable",rule="self == oldSelf"
 	ServiceCIDR string `json:"serviceCIDR,omitempty"`
-	// +kubebuilder:validation:XValidation:message="clusterDNS is immutable",rule="self == oldSelf"
+
 	// ClusterDNS is the IP address for the coredns service. Needs to be in the range provided by ServiceCIDR or CoreDNS may not deploy.
 	// Defaults to 10.43.0.10.
+	// +kubebuilder:validation:XValidation:message="clusterDNS is immutable",rule="self == oldSelf"
 	ClusterDNS string `json:"clusterDNS,omitempty"`
+
 	// ServerArgs are the ordered key value pairs (e.x. "testArg", "testValue") for the K3s pods running in server mode.
 	ServerArgs []string `json:"serverArgs,omitempty"`
+
 	// AgentArgs are the ordered key value pairs (e.x. "testArg", "testValue") for the K3s pods running in agent mode.
 	AgentArgs []string `json:"agentArgs,omitempty"`
+
 	// TLSSANs are the subjectAlternativeNames for the certificate the K3s server will use.
 	TLSSANs []string `json:"tlsSANs,omitempty"`
+
 	// Addons is a list of secrets containing raw YAML which will be deployed in the virtual K3k cluster on startup.
 	Addons []Addon `json:"addons,omitempty"`
+
+	// Mode is the cluster provisioning mode which can be either "shared" or "virtual". Defaults to "shared"
+	// +kubebuilder:default="shared"
+	// +kubebuilder:validation:Enum=shared;virtual
 	// +kubebuilder:validation:XValidation:message="mode is immutable",rule="self == oldSelf"
-	// +kubebuilder:validation:XValidation:message="invalid value for mode",rule="self == \"virtual\" || self == \"shared\""
-	// Mode is the cluster provisioning mode which can be either "virtual" or "shared". Defaults to "shared"
-	Mode string `json:"mode"`
+	Mode ClusterMode `json:"mode"`
 
 	// Persistence contains options controlling how the etcd data of the virtual cluster is persisted. By default, no data
 	// persistence is guaranteed, so restart of a virtual cluster pod may result in data loss without this field.
 	Persistence *PersistenceConfig `json:"persistence,omitempty"`
-	// +optional
+
 	// Expose contains options for exposing the apiserver inside/outside of the cluster. By default, this is only exposed as a
 	// clusterIP which is relatively secure, but difficult to access outside of the cluster.
+	// +optional
 	Expose *ExposeConfig `json:"expose,omitempty"`
 }
 
+// +kubebuilder:validation:Enum=shared;virtual
+// +kubebuilder:default="shared"
+//
+// ClusterMode is the possible provisioning mode of a Cluster.
+type ClusterMode string
+
+const (
+	SharedClusterMode  = ClusterMode("shared")
+	VirtualClusterMode = ClusterMode("virtual")
+)
+
 type ClusterLimit struct {
 	// ServerLimit is the limits (cpu/mem) that apply to the server nodes
 	ServerLimit v1.ResourceList `json:"serverLimit,omitempty"`

pkg/controller/clusterset/clusterset.go

@@ -6,13 +6,14 @@ import (
 	"github.com/rancher/k3k/pkg/apis/k3k.io/v1alpha1"
 	k3kcontroller "github.com/rancher/k3k/pkg/controller"
 	"github.com/rancher/k3k/pkg/log"
+	"go.uber.org/zap"
 	v1 "k8s.io/api/core/v1"
 	networkingv1 "k8s.io/api/networking/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlruntimeclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -50,29 +51,17 @@ func Add(ctx context.Context, mgr manager.Manager, clusterCIDR string, logger *l
 }
 
 func (c *ClusterSetReconciler) Reconcile(ctx context.Context, req reconcile.Request) (reconcile.Result, error) {
-	var (
-		clusterSet v1alpha1.ClusterSet
-		log        = c.logger.With("ClusterSet", req.NamespacedName)
-	)
-	if err := c.Client.Get(ctx, types.NamespacedName{Name: req.Name, Namespace: req.Namespace}, &clusterSet); err != nil {
+	log := c.logger.With("ClusterSet", req.NamespacedName)
+
+	var clusterSet v1alpha1.ClusterSet
+	if err := c.Client.Get(ctx, req.NamespacedName, &clusterSet); err != nil {
+		return reconcile.Result{}, client.IgnoreNotFound(err)
+	}
+
+	if err := c.reconcileNetworkPolicy(ctx, log, &clusterSet); err != nil {
 		return reconcile.Result{}, err
 	}
 
-	if !clusterSet.Spec.DisableNetworkPolicy {
-		log.Info("Creating NetworkPolicy")
-		setNetworkPolicy, err := netpol(ctx, c.ClusterCIDR, &clusterSet, c.Client)
-		if err != nil {
-			return reconcile.Result{}, err
-		}
-		if err := c.Client.Create(ctx, setNetworkPolicy); err != nil {
-			if apierrors.IsAlreadyExists(err) {
-				if err := c.Client.Update(ctx, setNetworkPolicy); err != nil {
-					return reconcile.Result{}, err
-				}
-			}
-			return reconcile.Result{}, err
-		}
-	}
 	// TODO: Add resource quota for clustersets
 	// if clusterSet.Spec.MaxLimits != nil {
 	// 	quota := v1.ResourceQuota{
@@ -97,6 +86,33 @@ func (c *ClusterSetReconciler) Reconcile(ctx context.Context, req reconcile.Requ
 	return reconcile.Result{}, nil
 }
 
+func (c *ClusterSetReconciler) reconcileNetworkPolicy(ctx context.Context, log *zap.SugaredLogger, clusterSet *v1alpha1.ClusterSet) error {
+	log.Info("reconciling NetworkPolicy")
+
+	networkPolicy, err := netpol(ctx, c.ClusterCIDR, clusterSet, c.Client)
+	if err != nil {
+		return err
+	}
+
+	if err = ctrl.SetControllerReference(clusterSet, networkPolicy, c.Scheme); err != nil {
+		return err
+	}
+
+	// if disabled then delete the existing network policy
+	if clusterSet.Spec.DisableNetworkPolicy {
+		err := c.Client.Delete(ctx, networkPolicy)
+		return client.IgnoreNotFound(err)
+	}
+
+	// otherwise try to create/update
+	err = c.Client.Create(ctx, networkPolicy)
+	if apierrors.IsAlreadyExists(err) {
+		return c.Client.Update(ctx, networkPolicy)
+	}
+
+	return err
+}
+
 func netpol(ctx context.Context, clusterCIDR string, clusterSet *v1alpha1.ClusterSet, client ctrlruntimeclient.Client) (*networkingv1.NetworkPolicy, error) {
 	var cidrList []string
 	if clusterCIDR == "" {
@@ -110,6 +126,7 @@ func netpol(ctx context.Context, clusterCIDR string, clusterSet *v1alpha1.Cluste
 	} else {
 		cidrList = []string{clusterCIDR}
 	}
+
 	return &networkingv1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      k3kcontroller.SafeConcatNameWithPrefix(clusterSet.Name),

pkg/controller/clusterset/clusterset_test.go

@@ -34,6 +34,22 @@ var _ = Describe("ClusterSet Controller", func() {
 		})
 
 		When("created with a default spec", func() {
+			It("should have only the 'shared' allowedNodeTypes", func() {
+				clusterSet := &v1alpha1.ClusterSet{
+					ObjectMeta: v1.ObjectMeta{
+						GenerateName: "clusterset-",
+						Namespace:    namespace,
+					},
+				}
+
+				err := k8sClient.Create(ctx, clusterSet)
+				Expect(err).To(Not(HaveOccurred()))
+
+				allowedModeTypes := clusterSet.Spec.AllowedNodeTypes
+				Expect(allowedModeTypes).To(HaveLen(1))
+				Expect(allowedModeTypes).To(ContainElement(v1alpha1.SharedClusterMode))
+			})
+
 			It("should create a NetworkPolicy", func() {
 				clusterSet := &v1alpha1.ClusterSet{
 					ObjectMeta: v1.ObjectMeta{
@@ -49,11 +65,11 @@ var _ = Describe("ClusterSet Controller", func() {
 				clusterSetNetworkPolicy := &networkingv1.NetworkPolicy{}
 
 				Eventually(func() bool {
-					deployKey := types.NamespacedName{
+					key := types.NamespacedName{
 						Name:      k3kcontroller.SafeConcatNameWithPrefix(clusterSet.Name),
 						Namespace: namespace,
 					}
-					err := k8sClient.Get(ctx, deployKey, clusterSetNetworkPolicy)
+					err := k8sClient.Get(ctx, key, clusterSetNetworkPolicy)
 					return err == nil
 				}, time.Minute, time.Second).Should(BeTrue())
 
@@ -96,7 +112,7 @@ var _ = Describe("ClusterSet Controller", func() {
 		})
 
 		When("created with DisableNetworkPolicy", func() {
-			It("should not create a NetworkPolicy", func() {
+			It("should not create a NetworkPolicy if true", func() {
 				clusterSet := &v1alpha1.ClusterSet{
 					ObjectMeta: v1.ObjectMeta{
 						GenerateName: "clusterset-",
@@ -112,11 +128,53 @@ var _ = Describe("ClusterSet Controller", func() {
 
 				// wait for a bit for the network policy, but it should not be created
 				Eventually(func() bool {
-					deployKey := types.NamespacedName{
+					key := types.NamespacedName{
 						Name:      k3kcontroller.SafeConcatNameWithPrefix(clusterSet.Name),
 						Namespace: namespace,
 					}
-					err := k8sClient.Get(ctx, deployKey, &networkingv1.NetworkPolicy{})
+					err := k8sClient.Get(ctx, key, &networkingv1.NetworkPolicy{})
+					return apierrors.IsNotFound(err)
+				}).
+					MustPassRepeatedly(5).
+					WithTimeout(time.Second * 10).
+					WithPolling(time.Second).
+					Should(BeTrue())
+			})
+
+			It("should delete the NetworkPolicy if changed to false", func() {
+				clusterSet := &v1alpha1.ClusterSet{
+					ObjectMeta: v1.ObjectMeta{
+						GenerateName: "clusterset-",
+						Namespace:    namespace,
+					},
+				}
+
+				err := k8sClient.Create(ctx, clusterSet)
+				Expect(err).To(Not(HaveOccurred()))
+
+				// look for network policy
+				clusterSetNetworkPolicy := &networkingv1.NetworkPolicy{}
+
+				Eventually(func() bool {
+					key := types.NamespacedName{
+						Name:      k3kcontroller.SafeConcatNameWithPrefix(clusterSet.Name),
+						Namespace: namespace,
+					}
+					err := k8sClient.Get(ctx, key, clusterSetNetworkPolicy)
+					return err == nil
+				}, time.Minute, time.Second).Should(BeTrue())
+
+				clusterSet.Spec.DisableNetworkPolicy = true
+				err = k8sClient.Update(ctx, clusterSet)
+				Expect(err).To(Not(HaveOccurred()))
+
+				// wait for a bit for the network policy to being deleted
+				Eventually(func() bool {
+					key := types.NamespacedName{
+						Name:      k3kcontroller.SafeConcatNameWithPrefix(clusterSet.Name),
+						Namespace: namespace,
+					}
+					err := k8sClient.Get(ctx, key, clusterSetNetworkPolicy)
 					return apierrors.IsNotFound(err)
 				}).
 					MustPassRepeatedly(5).
@@ -125,5 +183,72 @@ var _ = Describe("ClusterSet Controller", func() {
 					Should(BeTrue())
 			})
 		})
+
+		When("created specifing the mode", func() {
+			It("should have the 'virtual' mode if specified", func() {
+				clusterSet := &v1alpha1.ClusterSet{
+					ObjectMeta: v1.ObjectMeta{
+						GenerateName: "clusterset-",
+						Namespace:    namespace,
+					},
+					Spec: v1alpha1.ClusterSetSpec{
+						AllowedNodeTypes: []v1alpha1.ClusterMode{
+							v1alpha1.VirtualClusterMode,
+						},
+					},
+				}
+
+				err := k8sClient.Create(ctx, clusterSet)
+				Expect(err).To(Not(HaveOccurred()))
+
+				allowedModeTypes := clusterSet.Spec.AllowedNodeTypes
+				Expect(allowedModeTypes).To(HaveLen(1))
+				Expect(allowedModeTypes).To(ContainElement(v1alpha1.VirtualClusterMode))
+			})
+
+			It("should have both modes if specified", func() {
+				clusterSet := &v1alpha1.ClusterSet{
+					ObjectMeta: v1.ObjectMeta{
+						GenerateName: "clusterset-",
+						Namespace:    namespace,
+					},
+					Spec: v1alpha1.ClusterSetSpec{
+						AllowedNodeTypes: []v1alpha1.ClusterMode{
+							v1alpha1.SharedClusterMode,
+							v1alpha1.VirtualClusterMode,
+						},
+					},
+				}
+
+				err := k8sClient.Create(ctx, clusterSet)
+				Expect(err).To(Not(HaveOccurred()))
+
+				allowedModeTypes := clusterSet.Spec.AllowedNodeTypes
+				Expect(allowedModeTypes).To(HaveLen(2))
+				Expect(allowedModeTypes).To(ContainElements(
+					v1alpha1.SharedClusterMode,
+					v1alpha1.VirtualClusterMode,
+				))
+			})
+
+			It("should fail for a non-existing mode", func() {
+				clusterSet := &v1alpha1.ClusterSet{
+					ObjectMeta: v1.ObjectMeta{
+						GenerateName: "clusterset-",
+						Namespace:    namespace,
+					},
+					Spec: v1alpha1.ClusterSetSpec{
+						AllowedNodeTypes: []v1alpha1.ClusterMode{
+							v1alpha1.SharedClusterMode,
+							v1alpha1.VirtualClusterMode,
+							v1alpha1.ClusterMode("non-existing"),
+						},
+					},
+				}
+
+				err := k8sClient.Create(ctx, clusterSet)
+				Expect(err).To(HaveOccurred())
+			})
+		})
 	})
 })