diff --git a/phase/gather_facts.go b/phase/gather_facts.go index e9ffb371..19f7bc54 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" @@ -16,6 +17,8 @@ import ( type GatherFacts struct { GenericPhase SkipMachineIDs bool + + cplbVIPs map[string]struct{} } var ( @@ -30,6 +33,19 @@ func (p *GatherFacts) Title() string { return "Gather host facts" } +// 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. 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 +} + // Run the phase func (p *GatherFacts) Run(ctx context.Context) error { return p.parallelDo(ctx, p.Config.Spec.Hosts, p.investigateHost) @@ -82,8 +98,12 @@ 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 { - h.PrivateAddress = addr - log.Infof("%s: discovered %s as private address", h, addr) + 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) + } } } } diff --git a/phase/gather_facts_test.go b/phase/gather_facts_test.go new file mode 100644 index 00000000..18bf11c2 --- /dev/null +++ b/phase/gather_facts_test.go @@ -0,0 +1,132 @@ +package phase + +import ( + "context" + "errors" + "testing" + + "github.com/k0sproject/dig" + "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" + "github.com/k0sproject/version" + "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 { + linux.Ubuntu + 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([]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 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 TestInvestigateHostPrivateAddress(t *testing.T) { + const iface = "eth0" + + makePhase := func(k0sConfig dig.Mapping) *GatherFacts { + config := &v1beta1.Cluster{ + Spec: &cluster.Spec{ + K0s: &cluster.K0s{ + Version: version.MustParse("v1.33.0"), + Config: k0sConfig, + }, + }, + } + 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 + } + + 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) + }) + } +} 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 } 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) + }) + } +}