Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPV6 APIVIP Handling #589

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/combustion/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down
1 change: 0 additions & 1 deletion pkg/combustion/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -847,7 +847,6 @@ func TestKubernetesVIPManifestValidIPV6(t *testing.T) {

manifest, err := kubernetesVIPManifest(k8s)
require.NoError(t, err)
fmt.Println(manifest)

assert.Contains(t, manifest, "- fd12:3456:789a::21/128")
assert.Contains(t, manifest, "- name: k8s-api")
Expand Down
3 changes: 1 addition & 2 deletions pkg/combustion/templates/k8s-vip.yaml.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ spec:
addresses:
{{- if .IsIPV4 }}
- {{ .APIAddress }}/32
{{- end }}
{{- if .IsIPV6 }}
{{- else }}
- {{ .APIAddress }}/128
{{- end }}
avoidBuggyIPs: true
Expand Down
1 change: 1 addition & 0 deletions pkg/combustion/templates/rke2-multi-node-installer.sh.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ fi
{{- if and .apiVIP .apiHost }}
atanasdinov marked this conversation as resolved.
Show resolved Hide resolved
echo "{{ .apiVIP }} {{ .apiHost }}" >> /etc/hosts
{{- end }}

mkdir -p /etc/rancher/rke2/
cp $CONFIGFILE /etc/rancher/rke2/config.yaml

Expand Down
47 changes: 22 additions & 25 deletions pkg/image/validation/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
atanasdinov marked this conversation as resolved.
Show resolved Hide resolved
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() {
Copy link

@mchiappero mchiappero Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two checks are redundant, when the address is parsed successfully it's either IPv4 or IPv6. And would also not pass the following IsGlobalUnicast() test anyway.

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
Expand Down
28 changes: 2 additions & 26 deletions pkg/image/validation/kubernetes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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'.",
Expand All @@ -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'.",
Expand All @@ -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'.",
Expand Down
35 changes: 10 additions & 25 deletions pkg/kubernetes/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
atanasdinov marked this conversation as resolved.
Show resolved Hide resolved
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)
Expand Down Expand Up @@ -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")
}

Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/kubernetes/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"])
}

Expand Down