feedback updates

suse-edge · Oct 18, 2024 · ba09a06 · ba09a06
1 parent 090d385
commit ba09a06
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 83 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,36 +1084,30 @@ func TestValidateNetwork(t *testing.T) {
 				Network: image.Network{
 					APIVIP: "192.168.1.1",
 				},
-				Nodes: multiNode,
 			},
 		},
 		`valid ipv6`: {
 			K8s: image.Kubernetes{
 				Network: image.Network{
 					APIVIP: "fd12:3456:789a::21",
 				},
-				Nodes: multiNode,
 			},
 		},
 		`invalid ipv4`: {
 			K8s: image.Kubernetes{
 				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`: {
 			K8s: image.Kubernetes{
 				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,20 +1118,16 @@ 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`: {
 			K8s: image.Kubernetes{
 				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
@@ -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
@@ -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"])
 }