From 8457d3a20b63fa5fc65dd49afaff21c8fdb3e7bd Mon Sep 17 00:00:00 2001 From: Hussein Galal Date: Tue, 12 May 2026 15:57:24 +0300 Subject: [PATCH] Refactor servers and agents configuration management (#827) * Refactor server config secret Signed-off-by: galal-hussein * lint fixes Signed-off-by: galal-hussein * Refactor Agents configuration for virtual and shared mode Signed-off-by: galal-hussein * wsl Signed-off-by: galal-hussein * fixes Signed-off-by: galal-hussein * fix typo Signed-off-by: galal-hussein * fix unit tests Signed-off-by: galal-hussein * Remove go assert depednecny Signed-off-by: galal-hussein --------- Signed-off-by: galal-hussein --- pkg/controller/cluster/agent/shared.go | 42 ++-- pkg/controller/cluster/agent/shared_test.go | 5 +- pkg/controller/cluster/agent/virtual.go | 26 ++- pkg/controller/cluster/agent/virtual_test.go | 5 +- pkg/controller/cluster/server/config.go | 86 ++++---- pkg/controller/cluster/server/config_test.go | 194 +++++++++++++++++++ 6 files changed, 292 insertions(+), 66 deletions(-) create mode 100644 pkg/controller/cluster/server/config_test.go diff --git a/pkg/controller/cluster/agent/shared.go b/pkg/controller/cluster/agent/shared.go index 9e70c77..465d158 100644 --- a/pkg/controller/cluster/agent/shared.go +++ b/pkg/controller/cluster/agent/shared.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "gopkg.in/yaml.v3" "k8s.io/apimachinery/pkg/util/intstr" appsv1 "k8s.io/api/apps/v1" @@ -35,6 +36,17 @@ type SharedAgent struct { imagePullSecrets []string } +type sharedAgentConfig struct { + ClusterName string `yaml:"clusterName"` + ClusterNamespace string `yaml:"clusterNamespace"` + KubeletPort int `yaml:"kubeletPort"` + MirrorHostNodes bool `yaml:"mirrorHostNodes"` + ServerIP string `yaml:"serverIP"` + ServiceName string `yaml:"serviceName"` + Token string `yaml:"token"` + Version string `yaml:"version"` +} + func NewSharedAgent(config *Config, serviceIP, image, imagePullPolicy, token string, kubeletPort int, imagePullSecrets []string) *SharedAgent { return &SharedAgent{ Config: config, @@ -72,7 +84,10 @@ func (s *SharedAgent) ensureObject(ctx context.Context, obj ctrlruntimeclient.Ob } func (s *SharedAgent) config(ctx context.Context) error { - config := sharedAgentData(s.cluster, s.Name(), s.token, s.serviceIP, s.kubeletPort) + config, err := sharedAgentData(s.cluster, s.Name(), s.token, s.serviceIP, s.kubeletPort) + if err != nil { + return err + } configSecret := &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -84,28 +99,31 @@ func (s *SharedAgent) config(ctx context.Context) error { Namespace: s.cluster.Namespace, }, Data: map[string][]byte{ - "config.yaml": []byte(config), + "config.yaml": config, }, } return s.ensureObject(ctx, configSecret) } -func sharedAgentData(cluster *v1beta1.Cluster, serviceName, token, ip string, kubeletPort int) string { +func sharedAgentData(cluster *v1beta1.Cluster, serviceName, token, ip string, kubeletPort int) ([]byte, error) { version := cluster.Spec.Version if cluster.Spec.Version == "" { version = cluster.Status.HostVersion } - return fmt.Sprintf(`clusterName: %s -clusterNamespace: %s -serverIP: %s -serviceName: %s -token: %v -mirrorHostNodes: %t -version: %s -kubeletPort: %d`, - cluster.Name, cluster.Namespace, ip, serviceName, token, cluster.Spec.MirrorHostNodes, version, kubeletPort) + sharedAgentConfig := sharedAgentConfig{ + ClusterName: cluster.Name, + ClusterNamespace: cluster.Namespace, + ServerIP: ip, + ServiceName: serviceName, + Token: token, + MirrorHostNodes: cluster.Spec.MirrorHostNodes, + Version: version, + KubeletPort: kubeletPort, + } + + return yaml.Marshal(sharedAgentConfig) } func (s *SharedAgent) daemonset(ctx context.Context) error { diff --git a/pkg/controller/cluster/agent/shared_test.go b/pkg/controller/cluster/agent/shared_test.go index 0943b57..7371970 100644 --- a/pkg/controller/cluster/agent/shared_test.go +++ b/pkg/controller/cluster/agent/shared_test.go @@ -720,10 +720,11 @@ func Test_sharedAgentData(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - config := sharedAgentData(tt.args.cluster, tt.args.serviceName, tt.args.token, tt.args.ip, tt.args.kubeletPort) + config, err := sharedAgentData(tt.args.cluster, tt.args.serviceName, tt.args.token, tt.args.ip, tt.args.kubeletPort) + assert.NoError(t, err) data := make(map[string]string) - err := yaml.Unmarshal([]byte(config), data) + err = yaml.Unmarshal(config, data) assert.NoError(t, err) assert.Equal(t, tt.expectedData, data) diff --git a/pkg/controller/cluster/agent/virtual.go b/pkg/controller/cluster/agent/virtual.go index 0dbd734..47871d2 100644 --- a/pkg/controller/cluster/agent/virtual.go +++ b/pkg/controller/cluster/agent/virtual.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" + "gopkg.in/yaml.v3" "k8s.io/utils/ptr" appsv1 "k8s.io/api/apps/v1" @@ -32,6 +33,12 @@ type VirtualAgent struct { imagePullSecrets []string } +type virtualAgentConfig struct { + Server string `yaml:"server"` + Token string `yaml:"token"` + WithNodeId bool `yaml:"with-node-id"` +} + func NewVirtualAgent(config *Config, serviceIP, token, Image, ImagePullPolicy string, imagePullSecrets []string) *VirtualAgent { return &VirtualAgent{ Config: config, @@ -63,7 +70,10 @@ func (v *VirtualAgent) ensureObject(ctx context.Context, obj ctrlruntimeclient.O } func (v *VirtualAgent) config(ctx context.Context) error { - config := virtualAgentData(v.serviceIP, v.token) + config, err := virtualAgentData(v.serviceIP, v.token) + if err != nil { + return err + } configSecret := &corev1.Secret{ TypeMeta: metav1.TypeMeta{ @@ -75,17 +85,21 @@ func (v *VirtualAgent) config(ctx context.Context) error { Namespace: v.cluster.Namespace, }, Data: map[string][]byte{ - "config.yaml": []byte(config), + "config.yaml": config, }, } return v.ensureObject(ctx, configSecret) } -func virtualAgentData(serviceIP, token string) string { - return fmt.Sprintf(`server: https://%s -token: %s -with-node-id: true`, serviceIP, token) +func virtualAgentData(serviceIP, token string) ([]byte, error) { + agentConfig := virtualAgentConfig{ + Server: "https://" + serviceIP, + Token: token, + WithNodeId: true, + } + + return yaml.Marshal(agentConfig) } func (v *VirtualAgent) deployment(ctx context.Context) error { diff --git a/pkg/controller/cluster/agent/virtual_test.go b/pkg/controller/cluster/agent/virtual_test.go index 6861403..58d5614 100644 --- a/pkg/controller/cluster/agent/virtual_test.go +++ b/pkg/controller/cluster/agent/virtual_test.go @@ -152,10 +152,11 @@ func Test_virtualAgentData(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - config := virtualAgentData(tt.args.serviceIP, tt.args.token) + config, err := virtualAgentData(tt.args.serviceIP, tt.args.token) + assert.NoError(t, err) data := make(map[string]string) - err := yaml.Unmarshal([]byte(config), data) + err = yaml.Unmarshal([]byte(config), data) assert.NoError(t, err) assert.Equal(t, tt.expectedData, data) diff --git a/pkg/controller/cluster/server/config.go b/pkg/controller/cluster/server/config.go index 1df57a7..52136af 100644 --- a/pkg/controller/cluster/server/config.go +++ b/pkg/controller/cluster/server/config.go @@ -3,6 +3,7 @@ package server import ( "fmt" + "gopkg.in/yaml.v3" "k8s.io/apimachinery/pkg/util/sets" corev1 "k8s.io/api/core/v1" @@ -13,21 +14,29 @@ import ( "github.com/rancher/k3k/pkg/controller/cluster/agent" ) +// serverConfig are few options from k3s server options that will +// construct the yaml config file for k3s server +type serverConfig struct { + ClusterCIDR string `yaml:"cluster-cidr,omitempty"` + ClusterDNS string `yaml:"cluster-dns,omitempty"` + ClusterInit bool `yaml:"cluster-init,omitempty"` + DisableAgent bool `yaml:"disable-agent,omitempty"` + Disable []string `yaml:"disable,omitempty"` + EgressSelectorMode string `yaml:"egress-selector-mode,omitempty"` + Server string `yaml:"server,omitempty"` + ServiceCIDR string `yaml:"service-cidr,omitempty"` + TLSSAN []string `yaml:"tls-san,omitempty"` + Token string `yaml:"token,omitempty"` +} + func (s *Server) Config(init bool, serviceIP string) (*corev1.Secret, error) { name := configSecretName(s.cluster.Name, init) - sans := sets.NewString(s.cluster.Spec.TLSSANs...) - sans.Insert( - serviceIP, - ServiceName(s.cluster.Name), - fmt.Sprintf("%s.%s", ServiceName(s.cluster.Name), s.cluster.Namespace), - ) + serverConfig := buildServerConfig(s.cluster, init, serviceIP, s.token) - s.cluster.Status.TLSSANs = sans.List() - - config := serverConfigData(serviceIP, s.cluster, s.token) - if init { - config = initConfigData(s.cluster, s.token) + config, err := yaml.Marshal(serverConfig) + if err != nil { + return nil, err } return &corev1.Secret{ @@ -40,52 +49,41 @@ func (s *Server) Config(init bool, serviceIP string) (*corev1.Secret, error) { Namespace: s.cluster.Namespace, }, Data: map[string][]byte{ - "config.yaml": []byte(config), + "config.yaml": config, }, }, nil } -func serverConfigData(serviceIP string, cluster *v1beta1.Cluster, token string) string { - return "cluster-init: true\nserver: https://" + serviceIP + "\n" + serverOptions(cluster, token) -} +func buildServerConfig(cluster *v1beta1.Cluster, initServer bool, serviceIP, token string) serverConfig { + sans := sets.NewString(cluster.Spec.TLSSANs...) + sans.Insert( + serviceIP, + ServiceName(cluster.Name), + fmt.Sprintf("%s.%s", ServiceName(cluster.Name), cluster.Namespace), + ) -func initConfigData(cluster *v1beta1.Cluster, token string) string { - return "cluster-init: true\n" + serverOptions(cluster, token) -} + cluster.Status.TLSSANs = sans.List() -func serverOptions(cluster *v1beta1.Cluster, token string) string { - var opts string - - // TODO: generate token if not found - if token != "" { - opts = "token: " + token + "\n" + serverConfig := serverConfig{ + ClusterInit: true, + Token: token, + TLSSAN: cluster.Status.TLSSANs, + ServiceCIDR: cluster.Status.ServiceCIDR, + ClusterCIDR: cluster.Status.ClusterCIDR, + ClusterDNS: cluster.Spec.ClusterDNS, } - if cluster.Status.ClusterCIDR != "" { - opts = opts + "cluster-cidr: " + cluster.Status.ClusterCIDR + "\n" - } - - if cluster.Status.ServiceCIDR != "" { - opts = opts + "service-cidr: " + cluster.Status.ServiceCIDR + "\n" - } - - if cluster.Spec.ClusterDNS != "" { - opts = opts + "cluster-dns: " + cluster.Spec.ClusterDNS + "\n" - } - - if len(cluster.Status.TLSSANs) > 0 { - opts = opts + "tls-san:\n" - for _, addr := range cluster.Status.TLSSANs { - opts = opts + "- " + addr + "\n" - } + if !initServer { + serverConfig.Server = "https://" + serviceIP } if cluster.Spec.Mode != agent.VirtualNodeMode { - opts = opts + "disable-agent: true\negress-selector-mode: disabled\ndisable:\n- servicelb\n- traefik\n- metrics-server\n- local-storage\n" + serverConfig.DisableAgent = true + serverConfig.EgressSelectorMode = "disabled" + serverConfig.Disable = []string{"servicelb", "traefik", "metrics-server", "local-storage"} } - // TODO: Add extra args to the options - return opts + return serverConfig } func configSecretName(clusterName string, init bool) string { diff --git a/pkg/controller/cluster/server/config_test.go b/pkg/controller/cluster/server/config_test.go new file mode 100644 index 0000000..fe873d5 --- /dev/null +++ b/pkg/controller/cluster/server/config_test.go @@ -0,0 +1,194 @@ +package server + +import ( + "fmt" + "reflect" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1" +) + +const ( + defaultVirtualClusterCIDR = "10.52.0.0/16" + defaultVirtualServiceCIDR = "10.53.0.0/16" + defaultSharedClusterCIDR = "10.42.0.0/16" + defaultSharedServiceCIDR = "10.43.0.0/16" + testClusterDNS = "10.42.0.10" + testToken = "123456" + testServiceIP = "1.1.1.1" + testClusterName = "test-cluster" + testClusterNamespace = "test-ns" +) + +func Test_BuildServerConfig(t *testing.T) { + type args struct { + cluster *v1beta1.Cluster + initServer bool + token string + serviceIP string + } + + tests := []struct { + name string + args args + expectedData serverConfig + }{ + { + name: "Init server config with default cluster spec", + args: args{ + cluster: &v1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + Namespace: testClusterNamespace, + }, + Status: v1beta1.ClusterStatus{ + ClusterCIDR: defaultSharedClusterCIDR, + ServiceCIDR: defaultSharedServiceCIDR, + }, + }, + initServer: true, + token: testToken, + serviceIP: testServiceIP, + }, + expectedData: serverConfig{ + ClusterInit: true, + ClusterCIDR: defaultSharedClusterCIDR, + ServiceCIDR: defaultSharedServiceCIDR, + DisableAgent: true, + EgressSelectorMode: "disabled", + Token: testToken, + Disable: []string{"servicelb", "traefik", "metrics-server", "local-storage"}, + TLSSAN: []string{testServiceIP, ServiceName(testClusterName), fmt.Sprintf("%s.%s", ServiceName(testClusterName), testClusterNamespace)}, + }, + }, + { + name: "server config with default cluster spec", + args: args{ + cluster: &v1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + Namespace: testClusterNamespace, + }, + Status: v1beta1.ClusterStatus{ + ClusterCIDR: defaultSharedClusterCIDR, + ServiceCIDR: defaultSharedServiceCIDR, + }, + }, + initServer: false, + token: testToken, + serviceIP: testServiceIP, + }, + expectedData: serverConfig{ + ClusterInit: true, + ClusterCIDR: defaultSharedClusterCIDR, + ServiceCIDR: defaultSharedServiceCIDR, + DisableAgent: true, + EgressSelectorMode: "disabled", + Token: testToken, + Disable: []string{"servicelb", "traefik", "metrics-server", "local-storage"}, + TLSSAN: []string{testServiceIP, ServiceName(testClusterName), fmt.Sprintf("%s.%s", ServiceName(testClusterName), testClusterNamespace)}, + Server: "https://" + testServiceIP, + }, + }, + { + name: "Init server config with virtual mode cluster", + args: args{ + cluster: &v1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + Namespace: testClusterNamespace, + }, + Spec: v1beta1.ClusterSpec{ + Mode: v1beta1.VirtualClusterMode, + }, + Status: v1beta1.ClusterStatus{ + ClusterCIDR: defaultVirtualClusterCIDR, + ServiceCIDR: defaultVirtualServiceCIDR, + }, + }, + initServer: true, + token: testToken, + serviceIP: testServiceIP, + }, + expectedData: serverConfig{ + ClusterInit: true, + ClusterCIDR: defaultVirtualClusterCIDR, + ServiceCIDR: defaultVirtualServiceCIDR, + Token: testToken, + TLSSAN: []string{testServiceIP, ServiceName(testClusterName), fmt.Sprintf("%s.%s", ServiceName(testClusterName), testClusterNamespace)}, + }, + }, + { + name: "server config with virtual mode cluster", + args: args{ + cluster: &v1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + Namespace: testClusterNamespace, + }, + Spec: v1beta1.ClusterSpec{ + Mode: v1beta1.VirtualClusterMode, + }, + Status: v1beta1.ClusterStatus{ + ClusterCIDR: defaultVirtualClusterCIDR, + ServiceCIDR: defaultVirtualServiceCIDR, + }, + }, + initServer: false, + token: testToken, + serviceIP: testServiceIP, + }, + expectedData: serverConfig{ + ClusterInit: true, + ClusterCIDR: defaultVirtualClusterCIDR, + ServiceCIDR: defaultVirtualServiceCIDR, + Token: testToken, + TLSSAN: []string{testServiceIP, ServiceName(testClusterName), fmt.Sprintf("%s.%s", ServiceName(testClusterName), testClusterNamespace)}, + Server: "https://" + testServiceIP, + }, + }, + { + name: "Init server config with clusterDNS cluster spec", + args: args{ + cluster: &v1beta1.Cluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + Namespace: testClusterNamespace, + }, + Spec: v1beta1.ClusterSpec{ + ClusterDNS: testClusterDNS, + }, + Status: v1beta1.ClusterStatus{ + ClusterCIDR: defaultSharedClusterCIDR, + ServiceCIDR: defaultSharedServiceCIDR, + }, + }, + initServer: true, + token: testToken, + serviceIP: testServiceIP, + }, + expectedData: serverConfig{ + ClusterInit: true, + ClusterDNS: testClusterDNS, + ClusterCIDR: defaultSharedClusterCIDR, + ServiceCIDR: defaultSharedServiceCIDR, + DisableAgent: true, + EgressSelectorMode: "disabled", + Token: testToken, + Disable: []string{"servicelb", "traefik", "metrics-server", "local-storage"}, + TLSSAN: []string{testServiceIP, ServiceName(testClusterName), fmt.Sprintf("%s.%s", ServiceName(testClusterName), testClusterNamespace)}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + serverConfig := buildServerConfig(tt.args.cluster, tt.args.initServer, tt.args.serviceIP, tt.args.token) + if !reflect.DeepEqual(tt.expectedData, serverConfig) { + t.Errorf("found %v, expected %v", serverConfig, tt.expectedData) + } + }) + } +}