From c561b033df4f21cfe8e1f3d17a3a84f096221410 Mon Sep 17 00:00:00 2001 From: Enrico Candino Date: Wed, 27 Nov 2024 22:00:39 +0100 Subject: [PATCH] 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 | 17 ++- charts/k3k/crds/k3k.io_clustersets.yaml | 20 ++- cli/cmds/cluster/create.go | 5 +- examples/clusterset.yaml | 9 ++ examples/multiple-servers.yaml | 1 + examples/single-server.yaml | 1 + pkg/apis/k3k.io/v1alpha1/set_types.go | 19 ++- pkg/apis/k3k.io/v1alpha1/types.go | 48 +++++-- pkg/controller/clusterset/clusterset.go | 59 +++++--- pkg/controller/clusterset/clusterset_test.go | 135 ++++++++++++++++++- 10 files changed, 269 insertions(+), 45 deletions(-) create mode 100644 examples/clusterset.yaml diff --git a/charts/k3k/crds/k3k.io_clusters.yaml b/charts/k3k/crds/k3k.io_clusters.yaml index e6446386..12e3892b 100644 --- a/charts/k3k/crds/k3k.io_clusters.yaml +++ b/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: |- diff --git a/charts/k3k/crds/k3k.io_clustersets.yaml b/charts/k3k/crds/k3k.io_clustersets.yaml index 0045c5a1..fc602d09 100644 --- a/charts/k3k/crds/k3k.io_clustersets.yaml +++ b/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: diff --git a/cli/cmds/cluster/create.go b/cli/cmds/cluster/create.go index 95a4df2a..575f4ccf 100644 --- a/cli/cmds/cluster/create.go +++ b/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, diff --git a/examples/clusterset.yaml b/examples/clusterset.yaml new file mode 100644 index 00000000..bfc4462f --- /dev/null +++ b/examples/clusterset.yaml @@ -0,0 +1,9 @@ +apiVersion: k3k.io/v1alpha1 +kind: ClusterSet +metadata: + name: clusterset-example +# spec: + # disableNetworkPolicy: false + # allowedNodeTypes: + # - "shared" + # - "virtual" diff --git a/examples/multiple-servers.yaml b/examples/multiple-servers.yaml index b5e07c01..283f5381 100644 --- a/examples/multiple-servers.yaml +++ b/examples/multiple-servers.yaml @@ -3,6 +3,7 @@ kind: Cluster metadata: name: example1 spec: + mode: "shared" servers: 1 agents: 3 token: test diff --git a/examples/single-server.yaml b/examples/single-server.yaml index 43641b69..707b78ea 100644 --- a/examples/single-server.yaml +++ b/examples/single-server.yaml @@ -3,6 +3,7 @@ kind: Cluster metadata: name: single-server spec: + mode: "shared" servers: 1 agents: 3 token: test diff --git a/pkg/apis/k3k.io/v1alpha1/set_types.go b/pkg/apis/k3k.io/v1alpha1/set_types.go index ca1d79f5..38dea4fb 100644 --- a/pkg/apis/k3k.io/v1alpha1/set_types.go +++ b/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"` } diff --git a/pkg/apis/k3k.io/v1alpha1/types.go b/pkg/apis/k3k.io/v1alpha1/types.go index 729b9bb1..2c8cd522 100644 --- a/pkg/apis/k3k.io/v1alpha1/types.go +++ b/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"` diff --git a/pkg/controller/clusterset/clusterset.go b/pkg/controller/clusterset/clusterset.go index 6eaed18e..004a2d04 100644 --- a/pkg/controller/clusterset/clusterset.go +++ b/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), diff --git a/pkg/controller/clusterset/clusterset_test.go b/pkg/controller/clusterset/clusterset_test.go index c1b202bc..59a22372 100644 --- a/pkg/controller/clusterset/clusterset_test.go +++ b/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()) + }) + }) }) })