From ba09a06042757a5855710719f2f795b48d04e50b Mon Sep 17 00:00:00 2001 From: dbw7 Date: Fri, 18 Oct 2024 13:40:59 -0400 Subject: [PATCH] feedback updates --- pkg/combustion/kubernetes.go | 2 - pkg/combustion/templates/k8s-vip.yaml.tpl | 3 +- .../rke2-multi-node-installer.sh.tpl | 1 + pkg/image/validation/kubernetes.go | 47 +++++++++---------- pkg/image/validation/kubernetes_test.go | 28 +---------- pkg/kubernetes/cluster.go | 35 ++++---------- pkg/kubernetes/cluster_test.go | 6 +-- 7 files changed, 39 insertions(+), 83 deletions(-) diff --git a/pkg/combustion/kubernetes.go b/pkg/combustion/kubernetes.go index e7bb82a6..86b0e8d0 100644 --- a/pkg/combustion/kubernetes.go +++ b/pkg/combustion/kubernetes.go @@ -325,12 +325,10 @@ func kubernetesVIPManifest(k *image.Kubernetes) (string, error) { manifest := struct { APIAddress string IsIPV4 bool - IsIPV6 bool RKE2 bool }{ APIAddress: k.Network.APIVIP, IsIPV4: ip.Is4(), - IsIPV6: ip.Is6(), RKE2: strings.Contains(k.Version, image.KubernetesDistroRKE2), } diff --git a/pkg/combustion/templates/k8s-vip.yaml.tpl b/pkg/combustion/templates/k8s-vip.yaml.tpl index 7dcc67bf..28c8484c 100644 --- a/pkg/combustion/templates/k8s-vip.yaml.tpl +++ b/pkg/combustion/templates/k8s-vip.yaml.tpl @@ -8,8 +8,7 @@ spec: addresses: {{- if .IsIPV4 }} - {{ .APIAddress }}/32 - {{- end }} - {{- if .IsIPV6 }} + {{- else }} - {{ .APIAddress }}/128 {{- end }} avoidBuggyIPs: true diff --git a/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl b/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl index 2d633d6e..0efec761 100644 --- a/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl +++ b/pkg/combustion/templates/rke2-multi-node-installer.sh.tpl @@ -94,6 +94,7 @@ fi {{- if and .apiVIP .apiHost }} echo "{{ .apiVIP }} {{ .apiHost }}" >> /etc/hosts {{- end }} + mkdir -p /etc/rancher/rke2/ cp $CONFIGFILE /etc/rancher/rke2/config.yaml diff --git a/pkg/image/validation/kubernetes.go b/pkg/image/validation/kubernetes.go index ec969b0f..4a60fbb7 100644 --- a/pkg/image/validation/kubernetes.go +++ b/pkg/image/validation/kubernetes.go @@ -115,41 +115,38 @@ func validateNodes(k8s *image.Kubernetes) []FailedValidation { func validateNetwork(k8s *image.Kubernetes) []FailedValidation { var failures []FailedValidation - ip := k8s.Network.APIVIP - numNodes := len(k8s.Nodes) - if numNodes <= 1 { - // Single node cluster, node configurations are not required + if k8s.Network.APIVIP == "" { + failures = append(failures, FailedValidation{ + UserMessage: "The 'apiVIP' field is required in the 'network' section when defining entries under 'nodes'.", + }) + return failures } - if ip == "" { + parsedIP, err := netip.ParseAddr(k8s.Network.APIVIP) + if err != nil { failures = append(failures, FailedValidation{ - UserMessage: "The 'apiVIP' field is required in the 'network' section when defining entries under 'nodes'.", + UserMessage: fmt.Sprintf("Invalid address value %q for field 'apiVIP'.", k8s.Network.APIVIP), + Error: err, }) + + return failures } - if ip != "" { - parsedIP, err := netip.ParseAddr(ip) - if err != nil { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("Invalid APIVIP address %q for field 'apiVIP'.", ip), - Error: err, - }) - } + if !parsedIP.Is4() && !parsedIP.Is6() { + failures = append(failures, FailedValidation{ + UserMessage: "Only IPv4 and IPv6 addresses are valid values for field 'apiVIP'.", + }) - if !parsedIP.Is4() && !parsedIP.Is6() { - failures = append(failures, FailedValidation{ - UserMessage: fmt.Sprintf("%q is not an IPV4 or IPV6 address, only IPV4 and IPV6 addresses are valid for field 'apiVIP'.", ip), - }) - } + return failures + } - if !parsedIP.IsGlobalUnicast() { - msg := fmt.Sprintf("Invalid non-unicast cluster API address (%s) for field 'apiVIP'.", ip) - failures = append(failures, FailedValidation{ - UserMessage: msg, - }) - } + if !parsedIP.IsGlobalUnicast() { + msg := fmt.Sprintf("Invalid non-unicast cluster API address (%s) for field 'apiVIP'.", k8s.Network.APIVIP) + failures = append(failures, FailedValidation{ + UserMessage: msg, + }) } return failures diff --git a/pkg/image/validation/kubernetes_test.go b/pkg/image/validation/kubernetes_test.go index b9d5c1f7..ed650c9a 100644 --- a/pkg/image/validation/kubernetes_test.go +++ b/pkg/image/validation/kubernetes_test.go @@ -18,19 +18,6 @@ var validNetwork = image.Network{ APIVIP: "192.168.1.1", } -var multiNode = []image.Node{ - { - Hostname: "foo", - Type: image.KubernetesNodeTypeServer, - Initialiser: true, - }, - { - Hostname: "bar", - Type: image.KubernetesNodeTypeServer, - Initialiser: true, - }, -} - func TestValidateKubernetes(t *testing.T) { configDir, err := os.MkdirTemp("", "eib-config-") require.NoError(t, err) @@ -1087,7 +1074,6 @@ func TestValidateNetwork(t *testing.T) { `no network defined`: { K8s: image.Kubernetes{ Network: image.Network{}, - Nodes: multiNode, }, ExpectedFailedMessages: []string{ "The 'apiVIP' field is required in the 'network' section when defining entries under 'nodes'.", @@ -1098,7 +1084,6 @@ func TestValidateNetwork(t *testing.T) { Network: image.Network{ APIVIP: "192.168.1.1", }, - Nodes: multiNode, }, }, `valid ipv6`: { @@ -1106,7 +1091,6 @@ func TestValidateNetwork(t *testing.T) { Network: image.Network{ APIVIP: "fd12:3456:789a::21", }, - Nodes: multiNode, }, }, `invalid ipv4`: { @@ -1114,12 +1098,9 @@ func TestValidateNetwork(t *testing.T) { Network: image.Network{ APIVIP: "500.168.1.1", }, - Nodes: multiNode, }, ExpectedFailedMessages: []string{ - "Invalid APIVIP address \"500.168.1.1\" for field 'apiVIP'.", - "\"500.168.1.1\" is not an IPV4 or IPV6 address, only IPV4 and IPV6 addresses are valid for field 'apiVIP'.", - "Invalid non-unicast cluster API address (500.168.1.1) for field 'apiVIP'.", + "Invalid address value \"500.168.1.1\" for field 'apiVIP'.", }, }, `non-unicast ipv4`: { @@ -1127,7 +1108,6 @@ func TestValidateNetwork(t *testing.T) { Network: image.Network{ APIVIP: "127.0.0.1", }, - Nodes: multiNode, }, ExpectedFailedMessages: []string{ "Invalid non-unicast cluster API address (127.0.0.1) for field 'apiVIP'.", @@ -1138,12 +1118,9 @@ func TestValidateNetwork(t *testing.T) { Network: image.Network{ APIVIP: "xxxx:3456:789a::21", }, - Nodes: multiNode, }, ExpectedFailedMessages: []string{ - "Invalid APIVIP address \"xxxx:3456:789a::21\" for field 'apiVIP'.", - "\"xxxx:3456:789a::21\" is not an IPV4 or IPV6 address, only IPV4 and IPV6 addresses are valid for field 'apiVIP'.", - "Invalid non-unicast cluster API address (xxxx:3456:789a::21) for field 'apiVIP'.", + "Invalid address value \"xxxx:3456:789a::21\" for field 'apiVIP'.", }, }, `non-unicast ipv6`: { @@ -1151,7 +1128,6 @@ func TestValidateNetwork(t *testing.T) { Network: image.Network{ APIVIP: "ff02::1", }, - Nodes: multiNode, }, ExpectedFailedMessages: []string{ "Invalid non-unicast cluster API address (ff02::1) for field 'apiVIP'.", diff --git a/pkg/kubernetes/cluster.go b/pkg/kubernetes/cluster.go index 74a58184..25a920f4 100644 --- a/pkg/kubernetes/cluster.go +++ b/pkg/kubernetes/cluster.go @@ -55,22 +55,12 @@ func NewCluster(kubernetes *image.Kubernetes, configPath string) (*Cluster, erro return &Cluster{ServerConfig: serverConfig}, nil } - var ip4 *netip.Addr - var ip6 *netip.Addr - if kubernetes.Network.APIVIP != "" { - ipHolder, ipErr := netip.ParseAddr(kubernetes.Network.APIVIP) - if ipErr != nil { - return nil, fmt.Errorf("parsing kubernetes APIVIP address: %w", ipErr) - } - - if ipHolder.Is4() { - ip4 = &ipHolder - } else { - ip6 = &ipHolder - } + ip, ipErr := netip.ParseAddr(kubernetes.Network.APIVIP) + if ipErr != nil { + return nil, fmt.Errorf("parsing kubernetes APIVIP address: %w", ipErr) } - setMultiNodeConfigDefaults(kubernetes, serverConfig, ip4, ip6) + setMultiNodeConfigDefaults(kubernetes, serverConfig, &ip) agentConfigPath := filepath.Join(configPath, agentConfigFile) agentConfig, err := ParseKubernetesConfig(agentConfigPath) @@ -166,17 +156,17 @@ func setSingleNodeConfigDefaults(kubernetes *image.Kubernetes, config map[string delete(config, serverKey) } -func setMultiNodeConfigDefaults(kubernetes *image.Kubernetes, config map[string]any, ip4, ip6 *netip.Addr) { +func setMultiNodeConfigDefaults(kubernetes *image.Kubernetes, config map[string]any, ip *netip.Addr) { const ( k3sServerPort = 6443 rke2ServerPort = 9345 ) if strings.Contains(kubernetes.Version, image.KubernetesDistroRKE2) { - setClusterAPIAddress(config, ip4, ip6, rke2ServerPort) + setClusterAPIAddress(config, ip, rke2ServerPort) setClusterCNI(config) } else { - setClusterAPIAddress(config, ip4, ip6, k3sServerPort) + setClusterAPIAddress(config, ip, k3sServerPort) appendDisabledServices(config, "servicelb") } @@ -213,18 +203,13 @@ func setClusterCNI(config map[string]any) { config[cniKey] = cniDefaultValue } -func setClusterAPIAddress(config map[string]any, ip4, ip6 *netip.Addr, port uint16) { - if ip4 == nil && ip6 == nil { +func setClusterAPIAddress(config map[string]any, ip *netip.Addr, port uint16) { + if ip == nil { zap.S().Warn("Attempted to set an empty cluster API address") return } - switch { - case ip4 != nil: - config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(*ip4, port).String()) - case ip6 != nil: - config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(*ip6, port).String()) - } + config[serverKey] = fmt.Sprintf("https://%s", netip.AddrPortFrom(*ip, port).String()) } func setSELinux(config map[string]any) { diff --git a/pkg/kubernetes/cluster_test.go b/pkg/kubernetes/cluster_test.go index 1ca21df2..debb2ef7 100644 --- a/pkg/kubernetes/cluster_test.go +++ b/pkg/kubernetes/cluster_test.go @@ -277,19 +277,19 @@ func TestIdentifyInitialiserNode(t *testing.T) { func TestSetClusterAPIAddress(t *testing.T) { config := map[string]any{} - setClusterAPIAddress(config, nil, nil, 9345) + setClusterAPIAddress(config, nil, 9345) assert.NotContains(t, config, "server") ip4, err := netip.ParseAddr("192.168.122.50") assert.NoError(t, err) - setClusterAPIAddress(config, &ip4, nil, 9345) + setClusterAPIAddress(config, &ip4, 9345) assert.Equal(t, "https://192.168.122.50:9345", config["server"]) ip6, err := netip.ParseAddr("fd12:3456:789a::21") assert.NoError(t, err) - setClusterAPIAddress(config, nil, &ip6, 9345) + setClusterAPIAddress(config, &ip6, 9345) assert.Equal(t, "https://[fd12:3456:789a::21]:9345", config["server"]) }