From e177f8a52c48ec96eca9c2abd189e3fa04caadec Mon Sep 17 00:00:00 2001 From: dshishliannikov Date: Mon, 15 Jun 2026 18:02:45 -0400 Subject: [PATCH 1/6] Don't autodetect CPLB IP as host private address When CPLB is enabled, k0sctl may detect VRRP VIPs as node private address. This commit checks for the match and skips setting the private address if it is one of the CPLB IPs. Fixes #1100 Signed-off-by: dshishliannikov Signed-off-by: Kimmo Lehto --- phase/gather_facts.go | 66 +++++++- phase/gather_facts_test.go | 318 +++++++++++++++++++++++++++++++++++++ 2 files changed, 383 insertions(+), 1 deletion(-) create mode 100644 phase/gather_facts_test.go diff --git a/phase/gather_facts.go b/phase/gather_facts.go index e9ffb371..23731010 100644 --- a/phase/gather_facts.go +++ b/phase/gather_facts.go @@ -3,11 +3,14 @@ package phase import ( "context" "fmt" + "net" "strings" + "github.com/k0sproject/dig" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" "github.com/k0sproject/version" log "github.com/sirupsen/logrus" + "gopkg.in/yaml.v2" ) // Note: Passwordless sudo has not yet been confirmed when this runs @@ -81,7 +84,7 @@ func (p *GatherFacts) investigateHost(_ context.Context, h *cluster.Host) error } if h.PrivateInterface != "" { - if addr, err := h.Configurer.PrivateAddress(h, h.PrivateInterface, h.Address()); err == nil { + if addr, err := h.Configurer.PrivateAddress(h, h.PrivateInterface, h.Address()); err == nil && !isCPLBIP(addr, p.Config.Spec.K0s.Config) { h.PrivateAddress = addr log.Infof("%s: discovered %s as private address", h, addr) } @@ -108,3 +111,64 @@ func (p *GatherFacts) investigateHost(_ context.Context, h *cluster.Host) error return nil } + +type k0sCPLBConfig struct { + Spec struct { + Network struct { + ControlPlaneLoadBalancing struct { + Enabled bool `yaml:"enabled"` + Type string `yaml:"type"` + Keepalived struct { + VRRPInstances struct { + VirtualIPs []string `yaml:"virtualIPs"` + } `yaml:"vrrpInstances"` + VirtualServers []struct { + IPAddress string `yaml:"ipAddress"` + } `yaml:"virtualServers"` + } `yaml:"keepalived"` + } `yaml:"controlPlaneLoadBalancing"` + } `yaml:"network"` + } `yaml:"spec"` +} + +// isCPLBIP returns true if the given IP is a CPLB IP: either VRRP instance or a virtual server +func isCPLBIP(ip string, k0sConfig dig.Mapping) bool { + if k0sConfig == nil || net.ParseIP(ip) == nil { + return false + } + + if cfg, err := yaml.Marshal(k0sConfig); err == nil { + k0scfg := k0sCPLBConfig{} + if err := yaml.Unmarshal(cfg, &k0scfg); err == nil { + cplb := k0scfg.Spec.Network.ControlPlaneLoadBalancing + if !cplb.Enabled { + return false + } + + if cplb.Enabled && cplb.Type == "Keepalived" { + for _, vs := range cplb.Keepalived.VirtualServers { + if vs.IPAddress == ip { + return true + } + } + for _, vipCIDR := range cplb.Keepalived.VRRPInstances.VirtualIPs { + // since it's not validated to be CIDR, + // make a direct comparison just in case + if vipCIDR == ip { + return true + } + + vip, _, err := net.ParseCIDR(vipCIDR) + if err != nil { + continue + } + if vip.String() == ip { + return true + } + } + } + } + } + + return false +} diff --git a/phase/gather_facts_test.go b/phase/gather_facts_test.go new file mode 100644 index 00000000..a8a14deb --- /dev/null +++ b/phase/gather_facts_test.go @@ -0,0 +1,318 @@ +package phase + +import ( + "context" + "errors" + "testing" + + "github.com/k0sproject/dig" + "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1" + "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" + rigOS "github.com/k0sproject/rig/os" + "github.com/k0sproject/version" + "github.com/stretchr/testify/require" +) + +type privateAddressMock struct { + mockconfigurer + addr string + err error +} + +func (m *privateAddressMock) PrivateAddress(_ rigOS.Host, _, _ string) (string, error) { + return m.addr, m.err +} + +// makeCPLBConfig builds a dig.Mapping representing a k0s config with CPLB settings. +func makeCPLBConfig(enabled bool, cplbType string, vrrpVIPs []string, virtualServerIPs []string) dig.Mapping { + vsEntries := make([]interface{}, len(virtualServerIPs)) + for i, ip := range virtualServerIPs { + vsEntries[i] = dig.Mapping{"ipAddress": ip} + } + vrrpEntries := make([]interface{}, len(vrrpVIPs)) + for i, ip := range vrrpVIPs { + vrrpEntries[i] = ip + } + return dig.Mapping{ + "spec": dig.Mapping{ + "network": dig.Mapping{ + "controlPlaneLoadBalancing": dig.Mapping{ + "enabled": enabled, + "type": cplbType, + "keepalived": dig.Mapping{ + "vrrpInstances": dig.Mapping{"virtualIPs": vrrpEntries}, + "virtualServers": vsEntries, + }, + }, + }, + }, + } +} + +func TestIsCPLBIP(t *testing.T) { + tests := []struct { + name string + ip string + config dig.Mapping + want bool + }{ + // nil / invalid inputs + { + name: "nil config", + ip: "10.0.0.1", + config: nil, + want: false, + }, + { + name: "empty config", + ip: "10.0.0.1", + config: dig.Mapping{}, + want: false, + }, + { + name: "invalid IP string", + ip: "not-an-ip", + config: makeCPLBConfig(true, "Keepalived", nil, []string{"10.0.0.1"}), + want: false, + }, + { + name: "empty IP string", + ip: "", + config: makeCPLBConfig(true, "Keepalived", nil, []string{"10.0.0.1"}), + want: false, + }, + + // CPLB absent from config + { + name: "no controlPlaneLoadBalancing section", + ip: "10.0.0.1", + config: dig.Mapping{ + "spec": dig.Mapping{ + "network": dig.Mapping{}, + }, + }, + want: false, + }, + + // CPLB disabled + { + name: "CPLB disabled, IP matches virtual server", + ip: "10.0.0.1", + config: makeCPLBConfig(false, "Keepalived", nil, []string{"10.0.0.1"}), + want: false, + }, + { + name: "CPLB disabled, IP matches VRRP plain VIP", + ip: "10.0.0.1", + config: makeCPLBConfig(false, "Keepalived", []string{"10.0.0.1"}, nil), + want: false, + }, + { + name: "CPLB disabled, IP matches VRRP CIDR", + ip: "10.0.0.1", + config: makeCPLBConfig(false, "Keepalived", []string{"10.0.0.1/24"}, nil), + want: false, + }, + + // CPLB enabled but wrong type + { + name: "CPLB enabled, type not Keepalived", + ip: "10.0.0.1", + config: makeCPLBConfig(true, "Other", nil, []string{"10.0.0.1"}), + want: false, + }, + + // Virtual Server IPs + { + name: "first virtual server IP matches", + ip: "192.168.1.10", + config: makeCPLBConfig(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), + want: true, + }, + { + name: "non-first virtual server IP matches", + ip: "192.168.1.11", + config: makeCPLBConfig(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), + want: true, + }, + { + name: "virtual server IP no match", + ip: "192.168.1.99", + config: makeCPLBConfig(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), + want: false, + }, + + // VRRP VIPs as plain IPs (direct string match) + { + name: "first VRRP plain IP matches", + ip: "10.0.0.1", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), + want: true, + }, + { + name: "non-first VRRP plain IP matches", + ip: "10.0.0.2", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), + want: true, + }, + { + name: "VRRP plain IP no match", + ip: "10.0.0.99", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), + want: false, + }, + + // VRRP VIPs as CIDRs (host address extracted) + { + name: "first VRRP CIDR host IP matches", + ip: "10.0.0.1", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), + want: true, + }, + { + name: "non-first VRRP CIDR host IP matches", + ip: "10.0.0.2", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), + want: true, + }, + { + name: "VRRP CIDR host IP no match", + ip: "10.0.0.99", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), + want: false, + }, + // Network address of CIDR does not match a different host in that subnet + { + name: "VRRP CIDR network address does not match sibling host", + ip: "10.0.0.2", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, nil), + want: false, + }, + + // Invalid CIDR entries in VRRP list (should be skipped) + { + name: "invalid CIDR skipped, subsequent valid CIDR matches", + ip: "10.0.0.2", + config: makeCPLBConfig(true, "Keepalived", []string{"bad-cidr", "10.0.0.2/24"}, nil), + want: true, + }, + { + name: "invalid CIDR only, no match", + ip: "10.0.0.1", + config: makeCPLBConfig(true, "Keepalived", []string{"bad-cidr"}, nil), + want: false, + }, + + // No CPLB entries at all + { + name: "CPLB enabled Keepalived, no VRRPs and no virtual servers", + ip: "10.0.0.1", + config: makeCPLBConfig(true, "Keepalived", nil, nil), + want: false, + }, + + // Mixed: both VRRP and virtual servers present + { + name: "IP matches virtual server among mixed entries", + ip: "172.16.0.5", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), + want: true, + }, + { + name: "IP matches VRRP among mixed entries", + ip: "10.0.0.1", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), + want: true, + }, + { + name: "IP matches neither in mixed entries", + ip: "192.168.0.1", + config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.want, isCPLBIP(tc.ip, tc.config)) + }) + } +} + +func TestInvestigateHostPrivateAddress(t *testing.T) { + const iface = "eth0" + + makePhase := func(k0sConfig dig.Mapping) *GatherFacts { + return &GatherFacts{ + GenericPhase: GenericPhase{ + Config: &v1beta1.Cluster{ + Spec: &cluster.Spec{ + K0s: &cluster.K0s{ + Version: version.MustParse("v1.33.0"), + Config: k0sConfig, + }, + }, + }, + }, + SkipMachineIDs: true, + } + } + + makeHost := func(addr string, addrErr error) *cluster.Host { + return &cluster.Host{ + HostnameOverride: "test-host", + PrivateInterface: iface, + Metadata: cluster.HostMetadata{Arch: "amd64"}, + Configurer: &privateAddressMock{addr: addr, err: addrErr}, + } + } + + const cplbVIP = "10.0.0.1" + const normalIP = "192.168.1.5" + cplbCfg := makeCPLBConfig(true, "Keepalived", nil, []string{cplbVIP}) + + tests := []struct { + name string + phase *GatherFacts + host *cluster.Host + wantPrivate string + }{ + { + name: "CPLB IP returned: private address not set", + phase: makePhase(cplbCfg), + host: makeHost(cplbVIP, nil), + wantPrivate: "", + }, + { + name: "non-CPLB IP returned: private address set", + phase: makePhase(cplbCfg), + host: makeHost(normalIP, nil), + wantPrivate: normalIP, + }, + { + name: "configurer error: private address not set", + phase: makePhase(cplbCfg), + host: makeHost("", errors.New("lookup failed")), + wantPrivate: "", + }, + { + name: "nil k0s config: non-CPLB check skipped, address set", + phase: makePhase(nil), + host: makeHost(normalIP, nil), + wantPrivate: normalIP, + }, + { + name: "CPLB disabled: configured VIP not treated as CPLB, address set", + phase: makePhase(makeCPLBConfig(false, "Keepalived", nil, []string{cplbVIP})), + host: makeHost(cplbVIP, nil), + wantPrivate: cplbVIP, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.NoError(t, tc.phase.investigateHost(context.Background(), tc.host)) + require.Equal(t, tc.wantPrivate, tc.host.PrivateAddress) + }) + } +} From 7dcacecb1d44a27a34212ec2be82c49e62fdcb03 Mon Sep 17 00:00:00 2001 From: Kimmo Lehto Date: Thu, 18 Jun 2026 09:36:55 +0300 Subject: [PATCH 2/6] fix: correctly parse vrrpInstances as a YAML list VRRPInstances is a list in k0s config, not a mapping. The previous struct definition caused yaml.Unmarshal to silently fail, so VRRP VIPs were never detected and the CPLB IP check was effectively broken. Fix by making VRRPInstances a slice and iterating over all instances when checking VIPs. Signed-off-by: Kimmo Lehto --- phase/gather_facts.go | 30 ++++++++++++++++-------------- phase/gather_facts_test.go | 6 +++--- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/phase/gather_facts.go b/phase/gather_facts.go index 23731010..b874708d 100644 --- a/phase/gather_facts.go +++ b/phase/gather_facts.go @@ -119,7 +119,7 @@ type k0sCPLBConfig struct { Enabled bool `yaml:"enabled"` Type string `yaml:"type"` Keepalived struct { - VRRPInstances struct { + VRRPInstances []struct { VirtualIPs []string `yaml:"virtualIPs"` } `yaml:"vrrpInstances"` VirtualServers []struct { @@ -151,19 +151,21 @@ func isCPLBIP(ip string, k0sConfig dig.Mapping) bool { return true } } - for _, vipCIDR := range cplb.Keepalived.VRRPInstances.VirtualIPs { - // since it's not validated to be CIDR, - // make a direct comparison just in case - if vipCIDR == ip { - return true - } - - vip, _, err := net.ParseCIDR(vipCIDR) - if err != nil { - continue - } - if vip.String() == ip { - return true + for _, instance := range cplb.Keepalived.VRRPInstances { + for _, vipCIDR := range instance.VirtualIPs { + // since it's not validated to be CIDR, + // make a direct comparison just in case + if vipCIDR == ip { + return true + } + + vip, _, err := net.ParseCIDR(vipCIDR) + if err != nil { + continue + } + if vip.String() == ip { + return true + } } } } diff --git a/phase/gather_facts_test.go b/phase/gather_facts_test.go index a8a14deb..86c50b59 100644 --- a/phase/gather_facts_test.go +++ b/phase/gather_facts_test.go @@ -25,11 +25,11 @@ func (m *privateAddressMock) PrivateAddress(_ rigOS.Host, _, _ string) (string, // makeCPLBConfig builds a dig.Mapping representing a k0s config with CPLB settings. func makeCPLBConfig(enabled bool, cplbType string, vrrpVIPs []string, virtualServerIPs []string) dig.Mapping { - vsEntries := make([]interface{}, len(virtualServerIPs)) + vsEntries := make([]any, len(virtualServerIPs)) for i, ip := range virtualServerIPs { vsEntries[i] = dig.Mapping{"ipAddress": ip} } - vrrpEntries := make([]interface{}, len(vrrpVIPs)) + vrrpEntries := make([]any, len(vrrpVIPs)) for i, ip := range vrrpVIPs { vrrpEntries[i] = ip } @@ -40,7 +40,7 @@ func makeCPLBConfig(enabled bool, cplbType string, vrrpVIPs []string, virtualSer "enabled": enabled, "type": cplbType, "keepalived": dig.Mapping{ - "vrrpInstances": dig.Mapping{"virtualIPs": vrrpEntries}, + "vrrpInstances": []any{dig.Mapping{"virtualIPs": vrrpEntries}}, "virtualServers": vsEntries, }, }, From 1d982f0a6ea9df7953fa1c3250c8405254f1b692 Mon Sep 17 00:00:00 2001 From: Kimmo Lehto Date: Thu, 18 Jun 2026 10:03:50 +0300 Subject: [PATCH 3/6] refactor: consolidate CPLB VIP parsing into cluster package Address review feedback on the CPLB private-address detection: - Move the duplicated k0sCPLBConfig struct and parsing into a single shared helper in the cluster package (Spec.CPLBVIPs / cplbConfig), reused by both clusterExternalAddress and gather facts. The two structs had already diverged (the cluster one lacked vrrpInstances). - Precompute the CPLB VIP set once per GatherFacts.Run instead of marshalling/unmarshalling the full k0s config for every host. - Log a debug message when an autodetected private address is skipped because it matches a CPLB VIP, so an empty PrivateAddress is explainable. - Migrate the matching-semantics tests to TestCPLBVIPs in the cluster package. Signed-off-by: Kimmo Lehto --- phase/gather_facts.go | 82 ++----- phase/gather_facts_test.go | 201 +--------------- .../v1beta1/cluster/spec.go | 80 ++++++- .../v1beta1/cluster/spec_test.go | 215 ++++++++++++++++++ 4 files changed, 307 insertions(+), 271 deletions(-) diff --git a/phase/gather_facts.go b/phase/gather_facts.go index b874708d..8f8da699 100644 --- a/phase/gather_facts.go +++ b/phase/gather_facts.go @@ -3,14 +3,11 @@ package phase import ( "context" "fmt" - "net" "strings" - "github.com/k0sproject/dig" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" "github.com/k0sproject/version" log "github.com/sirupsen/logrus" - "gopkg.in/yaml.v2" ) // Note: Passwordless sudo has not yet been confirmed when this runs @@ -19,6 +16,8 @@ import ( type GatherFacts struct { GenericPhase SkipMachineIDs bool + + cplbVIPs map[string]struct{} } var ( @@ -35,6 +34,10 @@ func (p *GatherFacts) Title() string { // Run the phase func (p *GatherFacts) Run(ctx context.Context) error { + // Precompute the set of control plane load balancing virtual IPs once so + // that investigateHost (which may run concurrently per host) can do a cheap + // lookup instead of re-parsing the k0s config for every host. + p.cplbVIPs = p.Config.Spec.CPLBVIPs() return p.parallelDo(ctx, p.Config.Spec.Hosts, p.investigateHost) } @@ -84,9 +87,13 @@ func (p *GatherFacts) investigateHost(_ context.Context, h *cluster.Host) error } if h.PrivateInterface != "" { - if addr, err := h.Configurer.PrivateAddress(h, h.PrivateInterface, h.Address()); err == nil && !isCPLBIP(addr, p.Config.Spec.K0s.Config) { - h.PrivateAddress = addr - log.Infof("%s: discovered %s as private address", h, addr) + if addr, err := h.Configurer.PrivateAddress(h, h.PrivateInterface, h.Address()); err == nil { + if _, isVIP := p.cplbVIPs[addr]; isVIP { + log.Debugf("%s: skipping autodetected private address %s because it is a control plane load balancing virtual IP", h, addr) + } else { + h.PrivateAddress = addr + log.Infof("%s: discovered %s as private address", h, addr) + } } } } @@ -111,66 +118,3 @@ func (p *GatherFacts) investigateHost(_ context.Context, h *cluster.Host) error return nil } - -type k0sCPLBConfig struct { - Spec struct { - Network struct { - ControlPlaneLoadBalancing struct { - Enabled bool `yaml:"enabled"` - Type string `yaml:"type"` - Keepalived struct { - VRRPInstances []struct { - VirtualIPs []string `yaml:"virtualIPs"` - } `yaml:"vrrpInstances"` - VirtualServers []struct { - IPAddress string `yaml:"ipAddress"` - } `yaml:"virtualServers"` - } `yaml:"keepalived"` - } `yaml:"controlPlaneLoadBalancing"` - } `yaml:"network"` - } `yaml:"spec"` -} - -// isCPLBIP returns true if the given IP is a CPLB IP: either VRRP instance or a virtual server -func isCPLBIP(ip string, k0sConfig dig.Mapping) bool { - if k0sConfig == nil || net.ParseIP(ip) == nil { - return false - } - - if cfg, err := yaml.Marshal(k0sConfig); err == nil { - k0scfg := k0sCPLBConfig{} - if err := yaml.Unmarshal(cfg, &k0scfg); err == nil { - cplb := k0scfg.Spec.Network.ControlPlaneLoadBalancing - if !cplb.Enabled { - return false - } - - if cplb.Enabled && cplb.Type == "Keepalived" { - for _, vs := range cplb.Keepalived.VirtualServers { - if vs.IPAddress == ip { - return true - } - } - for _, instance := range cplb.Keepalived.VRRPInstances { - for _, vipCIDR := range instance.VirtualIPs { - // since it's not validated to be CIDR, - // make a direct comparison just in case - if vipCIDR == ip { - return true - } - - vip, _, err := net.ParseCIDR(vipCIDR) - if err != nil { - continue - } - if vip.String() == ip { - return true - } - } - } - } - } - } - - return false -} diff --git a/phase/gather_facts_test.go b/phase/gather_facts_test.go index 86c50b59..ce75e5e9 100644 --- a/phase/gather_facts_test.go +++ b/phase/gather_facts_test.go @@ -6,6 +6,8 @@ import ( "testing" "github.com/k0sproject/dig" + cfg "github.com/k0sproject/k0sctl/configurer" + "github.com/k0sproject/k0sctl/configurer/linux" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" rigOS "github.com/k0sproject/rig/os" @@ -14,7 +16,8 @@ import ( ) type privateAddressMock struct { - mockconfigurer + cfg.Linux + linux.Ubuntu addr string err error } @@ -49,201 +52,11 @@ func makeCPLBConfig(enabled bool, cplbType string, vrrpVIPs []string, virtualSer } } -func TestIsCPLBIP(t *testing.T) { - tests := []struct { - name string - ip string - config dig.Mapping - want bool - }{ - // nil / invalid inputs - { - name: "nil config", - ip: "10.0.0.1", - config: nil, - want: false, - }, - { - name: "empty config", - ip: "10.0.0.1", - config: dig.Mapping{}, - want: false, - }, - { - name: "invalid IP string", - ip: "not-an-ip", - config: makeCPLBConfig(true, "Keepalived", nil, []string{"10.0.0.1"}), - want: false, - }, - { - name: "empty IP string", - ip: "", - config: makeCPLBConfig(true, "Keepalived", nil, []string{"10.0.0.1"}), - want: false, - }, - - // CPLB absent from config - { - name: "no controlPlaneLoadBalancing section", - ip: "10.0.0.1", - config: dig.Mapping{ - "spec": dig.Mapping{ - "network": dig.Mapping{}, - }, - }, - want: false, - }, - - // CPLB disabled - { - name: "CPLB disabled, IP matches virtual server", - ip: "10.0.0.1", - config: makeCPLBConfig(false, "Keepalived", nil, []string{"10.0.0.1"}), - want: false, - }, - { - name: "CPLB disabled, IP matches VRRP plain VIP", - ip: "10.0.0.1", - config: makeCPLBConfig(false, "Keepalived", []string{"10.0.0.1"}, nil), - want: false, - }, - { - name: "CPLB disabled, IP matches VRRP CIDR", - ip: "10.0.0.1", - config: makeCPLBConfig(false, "Keepalived", []string{"10.0.0.1/24"}, nil), - want: false, - }, - - // CPLB enabled but wrong type - { - name: "CPLB enabled, type not Keepalived", - ip: "10.0.0.1", - config: makeCPLBConfig(true, "Other", nil, []string{"10.0.0.1"}), - want: false, - }, - - // Virtual Server IPs - { - name: "first virtual server IP matches", - ip: "192.168.1.10", - config: makeCPLBConfig(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), - want: true, - }, - { - name: "non-first virtual server IP matches", - ip: "192.168.1.11", - config: makeCPLBConfig(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), - want: true, - }, - { - name: "virtual server IP no match", - ip: "192.168.1.99", - config: makeCPLBConfig(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), - want: false, - }, - - // VRRP VIPs as plain IPs (direct string match) - { - name: "first VRRP plain IP matches", - ip: "10.0.0.1", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), - want: true, - }, - { - name: "non-first VRRP plain IP matches", - ip: "10.0.0.2", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), - want: true, - }, - { - name: "VRRP plain IP no match", - ip: "10.0.0.99", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), - want: false, - }, - - // VRRP VIPs as CIDRs (host address extracted) - { - name: "first VRRP CIDR host IP matches", - ip: "10.0.0.1", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), - want: true, - }, - { - name: "non-first VRRP CIDR host IP matches", - ip: "10.0.0.2", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), - want: true, - }, - { - name: "VRRP CIDR host IP no match", - ip: "10.0.0.99", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), - want: false, - }, - // Network address of CIDR does not match a different host in that subnet - { - name: "VRRP CIDR network address does not match sibling host", - ip: "10.0.0.2", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, nil), - want: false, - }, - - // Invalid CIDR entries in VRRP list (should be skipped) - { - name: "invalid CIDR skipped, subsequent valid CIDR matches", - ip: "10.0.0.2", - config: makeCPLBConfig(true, "Keepalived", []string{"bad-cidr", "10.0.0.2/24"}, nil), - want: true, - }, - { - name: "invalid CIDR only, no match", - ip: "10.0.0.1", - config: makeCPLBConfig(true, "Keepalived", []string{"bad-cidr"}, nil), - want: false, - }, - - // No CPLB entries at all - { - name: "CPLB enabled Keepalived, no VRRPs and no virtual servers", - ip: "10.0.0.1", - config: makeCPLBConfig(true, "Keepalived", nil, nil), - want: false, - }, - - // Mixed: both VRRP and virtual servers present - { - name: "IP matches virtual server among mixed entries", - ip: "172.16.0.5", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), - want: true, - }, - { - name: "IP matches VRRP among mixed entries", - ip: "10.0.0.1", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), - want: true, - }, - { - name: "IP matches neither in mixed entries", - ip: "192.168.0.1", - config: makeCPLBConfig(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), - want: false, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - require.Equal(t, tc.want, isCPLBIP(tc.ip, tc.config)) - }) - } -} - func TestInvestigateHostPrivateAddress(t *testing.T) { const iface = "eth0" makePhase := func(k0sConfig dig.Mapping) *GatherFacts { - return &GatherFacts{ + p := &GatherFacts{ GenericPhase: GenericPhase{ Config: &v1beta1.Cluster{ Spec: &cluster.Spec{ @@ -256,6 +69,10 @@ func TestInvestigateHostPrivateAddress(t *testing.T) { }, SkipMachineIDs: true, } + // Run() precomputes this before investigateHost is called; mirror that + // here since these tests exercise investigateHost directly. + p.cplbVIPs = p.Config.Spec.CPLBVIPs() + return p } makeHost := func(addr string, addrErr error) *cluster.Host { diff --git a/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec.go b/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec.go index bb296d46..0219c263 100644 --- a/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec.go +++ b/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec.go @@ -2,6 +2,7 @@ package cluster import ( "fmt" + "net" "strings" "github.com/creasty/defaults" @@ -118,6 +119,9 @@ type k0sCPLBConfig struct { Enabled bool `yaml:"enabled"` Type string `yaml:"type"` Keepalived struct { + VRRPInstances []struct { + VirtualIPs []string `yaml:"virtualIPs"` + } `yaml:"vrrpInstances"` VirtualServers []struct { IPAddress string `yaml:"ipAddress"` } `yaml:"virtualServers"` @@ -127,22 +131,78 @@ type k0sCPLBConfig struct { } `yaml:"spec"` } +// cplbConfig parses the keepalived control plane load balancing configuration +// from the k0s config. The second return value is false if the config can't be +// parsed or CPLB is not enabled with the Keepalived type. +func (s *Spec) cplbConfig() (*k0sCPLBConfig, bool) { + if s.K0s == nil { + return nil, false + } + + cfg, err := yaml.Marshal(s.K0s.Config) + if err != nil { + return nil, false + } + + k0scfg := &k0sCPLBConfig{} + if err := yaml.Unmarshal(cfg, k0scfg); err != nil { + return nil, false + } + + cplb := k0scfg.Spec.Network.ControlPlaneLoadBalancing + if !cplb.Enabled || cplb.Type != "Keepalived" { + return nil, false + } + + return k0scfg, true +} + +// CPLBVIPs returns the set of control plane load balancing virtual IPs +// (keepalived virtualServers and vrrpInstances virtualIPs) declared in the k0s +// config. VRRP virtual IPs are included both in their raw configured form and, +// when configured in CIDR notation, as the bare IP address so that either form +// matches. Returns an empty set if CPLB is not enabled or not using Keepalived. +func (s *Spec) CPLBVIPs() map[string]struct{} { + vips := make(map[string]struct{}) + + k0scfg, ok := s.cplbConfig() + if !ok { + return vips + } + + keepalived := k0scfg.Spec.Network.ControlPlaneLoadBalancing.Keepalived + for _, vs := range keepalived.VirtualServers { + if vs.IPAddress != "" { + vips[vs.IPAddress] = struct{}{} + } + } + for _, instance := range keepalived.VRRPInstances { + for _, vipCIDR := range instance.VirtualIPs { + if vipCIDR == "" { + continue + } + // the value is not validated to be in CIDR notation, so keep the + // raw form as well as the parsed bare IP to match either way + vips[vipCIDR] = struct{}{} + if vip, _, err := net.ParseCIDR(vipCIDR); err == nil { + vips[vip.String()] = struct{}{} + } + } + } + + return vips +} + func (s *Spec) clusterExternalAddress() string { if s.K0s != nil { if a := s.K0s.Config.DigString("spec", "api", "externalAddress"); a != "" { return a } - if cfg, err := yaml.Marshal(s.K0s.Config); err == nil { - k0scfg := k0sCPLBConfig{} - if err := yaml.Unmarshal(cfg, &k0scfg); err == nil { - cplb := k0scfg.Spec.Network.ControlPlaneLoadBalancing - if cplb.Enabled && cplb.Type == "Keepalived" { - for _, vs := range cplb.Keepalived.VirtualServers { - if addr := vs.IPAddress; addr != "" { - return addr - } - } + if k0scfg, ok := s.cplbConfig(); ok { + for _, vs := range k0scfg.Spec.Network.ControlPlaneLoadBalancing.Keepalived.VirtualServers { + if addr := vs.IPAddress; addr != "" { + return addr } } } diff --git a/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec_test.go b/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec_test.go index 89bc6a88..90c2bacb 100644 --- a/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec_test.go +++ b/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster/spec_test.go @@ -80,3 +80,218 @@ k0s: require.Equal(t, "https://192.168.0.10:6443", spec.KubeAPIURL()) }) } + +// cplbSpec builds a Spec with a k0s config containing the given CPLB settings. +func cplbSpec(enabled bool, cplbType string, vrrpVIPs []string, virtualServerIPs []string) *Spec { + vsEntries := make([]any, len(virtualServerIPs)) + for i, ip := range virtualServerIPs { + vsEntries[i] = dig.Mapping{"ipAddress": ip} + } + vrrpEntries := make([]any, len(vrrpVIPs)) + for i, ip := range vrrpVIPs { + vrrpEntries[i] = ip + } + return &Spec{ + K0s: &K0s{ + Config: dig.Mapping{ + "spec": dig.Mapping{ + "network": dig.Mapping{ + "controlPlaneLoadBalancing": dig.Mapping{ + "enabled": enabled, + "type": cplbType, + "keepalived": dig.Mapping{ + "vrrpInstances": []any{dig.Mapping{"virtualIPs": vrrpEntries}}, + "virtualServers": vsEntries, + }, + }, + }, + }, + }, + }, + } +} + +func TestCPLBVIPs(t *testing.T) { + tests := []struct { + name string + ip string + spec *Spec + want bool // whether ip is reported as a CPLB VIP + }{ + // nil / empty configs + { + name: "nil k0s", + ip: "10.0.0.1", + spec: &Spec{}, + want: false, + }, + { + name: "nil config", + ip: "10.0.0.1", + spec: &Spec{K0s: &K0s{}}, + want: false, + }, + { + name: "empty config", + ip: "10.0.0.1", + spec: &Spec{K0s: &K0s{Config: dig.Mapping{}}}, + want: false, + }, + { + name: "invalid IP string", + ip: "not-an-ip", + spec: cplbSpec(true, "Keepalived", nil, []string{"10.0.0.1"}), + want: false, + }, + { + name: "empty IP string", + ip: "", + spec: cplbSpec(true, "Keepalived", nil, []string{"10.0.0.1"}), + want: false, + }, + + // CPLB disabled + { + name: "CPLB disabled, IP matches virtual server", + ip: "10.0.0.1", + spec: cplbSpec(false, "Keepalived", nil, []string{"10.0.0.1"}), + want: false, + }, + { + name: "CPLB disabled, IP matches VRRP plain VIP", + ip: "10.0.0.1", + spec: cplbSpec(false, "Keepalived", []string{"10.0.0.1"}, nil), + want: false, + }, + { + name: "CPLB disabled, IP matches VRRP CIDR", + ip: "10.0.0.1", + spec: cplbSpec(false, "Keepalived", []string{"10.0.0.1/24"}, nil), + want: false, + }, + + // CPLB enabled but wrong type + { + name: "CPLB enabled, type not Keepalived", + ip: "10.0.0.1", + spec: cplbSpec(true, "Other", nil, []string{"10.0.0.1"}), + want: false, + }, + + // Virtual server IPs + { + name: "first virtual server IP matches", + ip: "192.168.1.10", + spec: cplbSpec(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), + want: true, + }, + { + name: "non-first virtual server IP matches", + ip: "192.168.1.11", + spec: cplbSpec(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), + want: true, + }, + { + name: "virtual server IP no match", + ip: "192.168.1.99", + spec: cplbSpec(true, "Keepalived", nil, []string{"192.168.1.10", "192.168.1.11"}), + want: false, + }, + + // VRRP VIPs as plain IPs (direct string match) + { + name: "first VRRP plain IP matches", + ip: "10.0.0.1", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), + want: true, + }, + { + name: "non-first VRRP plain IP matches", + ip: "10.0.0.2", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), + want: true, + }, + { + name: "VRRP plain IP no match", + ip: "10.0.0.99", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1", "10.0.0.2"}, nil), + want: false, + }, + + // VRRP VIPs as CIDRs (bare host address extracted) + { + name: "first VRRP CIDR host IP matches", + ip: "10.0.0.1", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), + want: true, + }, + { + name: "non-first VRRP CIDR host IP matches", + ip: "10.0.0.2", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), + want: true, + }, + { + name: "VRRP CIDR host IP no match", + ip: "10.0.0.99", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24", "10.0.0.2/24"}, nil), + want: false, + }, + { + name: "VRRP CIDR network address does not match sibling host", + ip: "10.0.0.2", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24"}, nil), + want: false, + }, + + // Invalid CIDR entries in VRRP list (should be skipped) + { + name: "invalid CIDR skipped, subsequent valid CIDR matches", + ip: "10.0.0.2", + spec: cplbSpec(true, "Keepalived", []string{"bad-cidr", "10.0.0.2/24"}, nil), + want: true, + }, + { + name: "invalid CIDR only, no match", + ip: "10.0.0.1", + spec: cplbSpec(true, "Keepalived", []string{"bad-cidr"}, nil), + want: false, + }, + + // No CPLB entries at all + { + name: "CPLB enabled Keepalived, no VRRPs and no virtual servers", + ip: "10.0.0.1", + spec: cplbSpec(true, "Keepalived", nil, nil), + want: false, + }, + + // Mixed: both VRRP and virtual servers present + { + name: "IP matches virtual server among mixed entries", + ip: "172.16.0.5", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), + want: true, + }, + { + name: "IP matches VRRP among mixed entries", + ip: "10.0.0.1", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), + want: true, + }, + { + name: "IP matches neither in mixed entries", + ip: "192.168.0.1", + spec: cplbSpec(true, "Keepalived", []string{"10.0.0.1/24"}, []string{"172.16.0.5"}), + want: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + vips := tc.spec.CPLBVIPs() + _, got := vips[tc.ip] + require.Equal(t, tc.want, got) + }) + } +} From add96b68622adfcb50fcb8d76b1cf4f06d8f9594 Mon Sep 17 00:00:00 2001 From: Kimmo Lehto Date: Thu, 18 Jun 2026 10:20:06 +0300 Subject: [PATCH 4/6] test: drop redundant configurer.Linux embed from mocks linux.Ubuntu already embeds configurer.Linux (via linux.Debian) and satisfies configurer.Configurer on its own, so embedding cfg.Linux alongside it was redundant. Embed only linux.Ubuntu and drop the now unused configurer import. Signed-off-by: Kimmo Lehto --- phase/gather_facts_test.go | 5 +++-- phase/validate_hosts_test.go | 2 -- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/phase/gather_facts_test.go b/phase/gather_facts_test.go index ce75e5e9..c87dbc98 100644 --- a/phase/gather_facts_test.go +++ b/phase/gather_facts_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/k0sproject/dig" - cfg "github.com/k0sproject/k0sctl/configurer" "github.com/k0sproject/k0sctl/configurer/linux" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" @@ -15,8 +14,10 @@ import ( "github.com/stretchr/testify/require" ) +// linux.Ubuntu already embeds configurer.Linux (via linux.Debian) and satisfies +// configurer.Configurer, so embedding it alone gives the mock all the no-op +// configurer methods. type privateAddressMock struct { - cfg.Linux linux.Ubuntu addr string err error diff --git a/phase/validate_hosts_test.go b/phase/validate_hosts_test.go index 34f196be..f39b29dd 100644 --- a/phase/validate_hosts_test.go +++ b/phase/validate_hosts_test.go @@ -6,7 +6,6 @@ import ( "testing" "time" - cfg "github.com/k0sproject/k0sctl/configurer" "github.com/k0sproject/k0sctl/configurer/linux" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" @@ -15,7 +14,6 @@ import ( ) type mockconfigurer struct { - cfg.Linux linux.Ubuntu skew time.Duration } From dbe32dc26ed6fae2ad54c457c6d7aa1cba2ff321 Mon Sep 17 00:00:00 2001 From: Kimmo Lehto Date: Thu, 18 Jun 2026 10:47:42 +0300 Subject: [PATCH 5/6] refactor: precompute CPLB VIP set in GatherFacts.Prepare Move the CPLB VIP set precomputation out of Run and into a Prepare override, which is the idiomatic place for deriving per-phase state from the config. The manager calls Prepare before Run, so the set is ready by the time investigateHost runs. Signed-off-by: Kimmo Lehto --- phase/gather_facts.go | 13 ++++++++++--- phase/gather_facts_test.go | 22 +++++++++------------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/phase/gather_facts.go b/phase/gather_facts.go index 8f8da699..71a1c4e1 100644 --- a/phase/gather_facts.go +++ b/phase/gather_facts.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1" "github.com/k0sproject/k0sctl/pkg/apis/k0sctl.k0sproject.io/v1beta1/cluster" "github.com/k0sproject/version" log "github.com/sirupsen/logrus" @@ -32,12 +33,18 @@ func (p *GatherFacts) Title() string { return "Gather host facts" } -// Run the phase -func (p *GatherFacts) Run(ctx context.Context) error { +// Prepare the phase +func (p *GatherFacts) Prepare(config *v1beta1.Cluster) error { + p.Config = config // Precompute the set of control plane load balancing virtual IPs once so // that investigateHost (which may run concurrently per host) can do a cheap // lookup instead of re-parsing the k0s config for every host. - p.cplbVIPs = p.Config.Spec.CPLBVIPs() + p.cplbVIPs = config.Spec.CPLBVIPs() + return nil +} + +// Run the phase +func (p *GatherFacts) Run(ctx context.Context) error { return p.parallelDo(ctx, p.Config.Spec.Hosts, p.investigateHost) } diff --git a/phase/gather_facts_test.go b/phase/gather_facts_test.go index c87dbc98..18bf11c2 100644 --- a/phase/gather_facts_test.go +++ b/phase/gather_facts_test.go @@ -57,22 +57,18 @@ func TestInvestigateHostPrivateAddress(t *testing.T) { const iface = "eth0" makePhase := func(k0sConfig dig.Mapping) *GatherFacts { - p := &GatherFacts{ - GenericPhase: GenericPhase{ - Config: &v1beta1.Cluster{ - Spec: &cluster.Spec{ - K0s: &cluster.K0s{ - Version: version.MustParse("v1.33.0"), - Config: k0sConfig, - }, - }, + config := &v1beta1.Cluster{ + Spec: &cluster.Spec{ + K0s: &cluster.K0s{ + Version: version.MustParse("v1.33.0"), + Config: k0sConfig, }, }, - SkipMachineIDs: true, } - // Run() precomputes this before investigateHost is called; mirror that - // here since these tests exercise investigateHost directly. - p.cplbVIPs = p.Config.Spec.CPLBVIPs() + p := &GatherFacts{SkipMachineIDs: true} + // Prepare() precomputes the CPLB VIP set the same way the manager would + // before Run/investigateHost run. + require.NoError(t, p.Prepare(config)) return p } From 0f87724c110b82ec7e28b8a994f4908f27c8014b Mon Sep 17 00:00:00 2001 From: Kimmo Lehto Date: Thu, 18 Jun 2026 10:53:21 +0300 Subject: [PATCH 6/6] fix: guard against nil Spec in GatherFacts.Prepare Cluster.Spec is a pointer and can be nil when a Cluster is constructed programmatically or partially decoded. Calling CPLBVIPs on a nil Spec would panic, so only precompute the VIP set when a spec is present; a nil cplbVIPs map is safe to read from. Signed-off-by: Kimmo Lehto --- phase/gather_facts.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/phase/gather_facts.go b/phase/gather_facts.go index 71a1c4e1..19f7bc54 100644 --- a/phase/gather_facts.go +++ b/phase/gather_facts.go @@ -38,8 +38,11 @@ func (p *GatherFacts) Prepare(config *v1beta1.Cluster) error { p.Config = config // Precompute the set of control plane load balancing virtual IPs once so // that investigateHost (which may run concurrently per host) can do a cheap - // lookup instead of re-parsing the k0s config for every host. - p.cplbVIPs = config.Spec.CPLBVIPs() + // lookup instead of re-parsing the k0s config for every host. A nil cplbVIPs + // map is safe to read from, so leave it unset when there is no spec. + if config.Spec != nil { + p.cplbVIPs = config.Spec.CPLBVIPs() + } return nil }