Addresses comments

src/k8s/pkg/k8sd/app/hooks_bootstrap.go

@@ -232,13 +232,12 @@ func (a *App) onBootstrapWorkerNode(ctx context.Context, s state.State, encodedT
 	}
 
 	serviceConfigs := types.K8sServiceConfigs{
-		IsControlPlane:         false,
 		ExtraNodeKubeletArgs:   joinConfig.ExtraNodeKubeletArgs,
 		ExtraNodeKubeProxyArgs: joinConfig.ExtraNodeKubeProxyArgs,
 	}
 
 	// Pre-init checks
-	if err := snap.PreInitChecks(ctx, cfg, serviceConfigs); err != nil {
+	if err := snap.PreInitChecks(ctx, cfg, serviceConfigs, false); err != nil {
 		return fmt.Errorf("pre-init checks failed for worker node: %w", err)
 	}
 
@@ -426,15 +425,14 @@ func (a *App) onBootstrapControlPlane(ctx context.Context, s state.State, bootst
 	cfg.Certificates.K8sdPrivateKey = utils.Pointer(certificates.K8sdPrivateKey)
 
 	serviceConfigs := types.K8sServiceConfigs{
-		IsControlPlane:                     true,
 		ExtraNodeKubeSchedulerArgs:         bootstrapConfig.ExtraNodeKubeSchedulerArgs,
 		ExtraNodeKubeControllerManagerArgs: bootstrapConfig.ExtraNodeKubeControllerManagerArgs,
 		ExtraNodeKubeletArgs:               bootstrapConfig.ExtraNodeKubeletArgs,
 		ExtraNodeKubeProxyArgs:             bootstrapConfig.ExtraNodeKubeProxyArgs,
 	}
 
 	// Pre-init checks
-	if err := snap.PreInitChecks(ctx, cfg, serviceConfigs); err != nil {
+	if err := snap.PreInitChecks(ctx, cfg, serviceConfigs, true); err != nil {
 		return fmt.Errorf("pre-init checks failed for bootstrap node: %w", err)
 	}
 

src/k8s/pkg/k8sd/app/hooks_join.go

@@ -139,15 +139,14 @@ func (a *App) onPostJoin(ctx context.Context, s state.State, initConfig map[stri
 	}
 
 	serviceConfigs := types.K8sServiceConfigs{
-		IsControlPlane:                     true,
 		ExtraNodeKubeSchedulerArgs:         joinConfig.ExtraNodeKubeSchedulerArgs,
 		ExtraNodeKubeControllerManagerArgs: joinConfig.ExtraNodeKubeControllerManagerArgs,
 		ExtraNodeKubeletArgs:               joinConfig.ExtraNodeKubeletArgs,
 		ExtraNodeKubeProxyArgs:             joinConfig.ExtraNodeKubeProxyArgs,
 	}
 
 	// Pre-init checks
-	if err := snap.PreInitChecks(ctx, cfg, serviceConfigs); err != nil {
+	if err := snap.PreInitChecks(ctx, cfg, serviceConfigs, true); err != nil {
 		return fmt.Errorf("pre-init checks failed for joining node: %w", err)
 	}
 

src/k8s/pkg/k8sd/types/k8s_service_configs.go

@@ -16,7 +16,6 @@ const (
 )
 
 type K8sServiceConfigs struct {
-	IsControlPlane                     bool
 	ExtraNodeKubeControllerManagerArgs map[string]*string
 	ExtraNodeKubeSchedulerArgs         map[string]*string
 	ExtraNodeKubeletArgs               map[string]*string

src/k8s/pkg/snap/interface.go

@@ -66,5 +66,5 @@ type Snap interface {
 
 	K8sdClient(address string) (k8sd.Client, error) // k8sd client
 
-	PreInitChecks(ctx context.Context, config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs) error // pre-init checks before k8s-snap can start
+	PreInitChecks(ctx context.Context, config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs, isControlPlane bool) error // pre-init checks before k8s-snap can start
 }

src/k8s/pkg/snap/mock/mock.go

@@ -245,7 +245,7 @@ func (s *Snap) SnapctlSet(ctx context.Context, args ...string) error {
 	return s.SnapctlSetErr
 }
 
-func (s *Snap) PreInitChecks(ctx context.Context, config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs) error {
+func (s *Snap) PreInitChecks(ctx context.Context, config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs, isControlPlane bool) error {
 	s.PreInitChecksCalledWith = append(s.PreInitChecksCalledWith, config)
 	return s.PreInitChecksErr
 }

src/k8s/pkg/snap/snap.go

@@ -8,7 +8,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strconv"
 	"strings"
 
 	"github.com/canonical/k8s/pkg/client/dqlite"
@@ -329,8 +328,8 @@ func (s *snap) SnapctlSet(ctx context.Context, args ...string) error {
 	return s.runCommand(ctx, append([]string{"snapctl", "set"}, args...))
 }
 
-func (s *snap) PreInitChecks(ctx context.Context, config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs) error {
-	if err := checkK8sServicePorts(config, serviceConfigs); err != nil {
+func (s *snap) PreInitChecks(ctx context.Context, config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs, isControlPlane bool) error {
+	if err := utils.CheckK8sServicePorts(config, serviceConfigs, isControlPlane); err != nil {
 		return fmt.Errorf("Encountered error(s) while verifying port availability for Kubernetes services: %w", err)
 	}
 
@@ -357,51 +356,4 @@ func (s *snap) PreInitChecks(ctx context.Context, config types.ClusterConfig, se
 	return nil
 }
 
-func checkK8sServicePorts(config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs) error {
-	var allErrors []error
-	ports := map[string]string{
-		// Default values from official Kubernetes documentation.
-		"kubelet":           serviceConfigs.GetKubeletPort(),
-		"kubelet-healthz":   serviceConfigs.GetKubeletHealthzPort(),
-		"kubelet-read-only": serviceConfigs.GetKubeletReadOnlyPort(),
-		"k8s-dqlite":        strconv.Itoa(config.Datastore.GetK8sDqlitePort()),
-		"loadbalancer":      strconv.Itoa(config.LoadBalancer.GetBGPPeerPort()),
-	}
-
-	if port, err := serviceConfigs.GetKubeProxyHealthzPort(); err != nil {
-		allErrors = append(allErrors, err)
-	} else {
-		ports["kube-proxy-healhz"] = port
-	}
-
-	if port, err := serviceConfigs.GetKubeProxyMetricsPort(); err != nil {
-		allErrors = append(allErrors, err)
-	} else {
-		ports["kube-proxy-metrics"] = port
-	}
-
-	if serviceConfigs.IsControlPlane {
-		ports["kube-apiserver"] = strconv.Itoa(config.APIServer.GetSecurePort())
-		ports["kube-scheduler"] = serviceConfigs.GetKubeSchedulerPort()
-		ports["kube-controller-manager"] = serviceConfigs.GetKubeControllerManagerPort()
-	} else {
-		ports["kube-apiserver-proxy"] = strconv.Itoa(config.APIServer.GetSecurePort())
-	}
-
-	for service, port := range ports {
-		if port == "0" {
-			// Some ports may be set to 0 in order to disable them. No need to check.
-			continue
-		}
-		if open, err := utils.IsLocalPortOpen(port); err != nil {
-			// Could not open port due to error.
-			allErrors = append(allErrors, fmt.Errorf("could not check port %s (needed by: %s): %w", port, service, err))
-		} else if !open {
-			allErrors = append(allErrors, fmt.Errorf("port %s (needed by: %s) is already in use.", port, service))
-		}
-	}
-
-	return errors.Join(allErrors...)
-}
-
 var _ Snap = &snap{}

src/k8s/pkg/snap/snap_test.go

@@ -144,7 +144,7 @@ func TestSnap(t *testing.T) {
 		conf := types.ClusterConfig{}
 		serviceConfigs := types.K8sServiceConfigs{}
 
-		err = snap.PreInitChecks(context.Background(), conf, serviceConfigs)
+		err = snap.PreInitChecks(context.Background(), conf, serviceConfigs, true)
 		g.Expect(err).To(Not(HaveOccurred()))
 		expectedCalls := []string{}
 		for _, binary := range []string{"kube-apiserver", "kube-controller-manager", "kube-scheduler", "kube-proxy", "kubelet"} {
@@ -163,7 +163,7 @@ func TestSnap(t *testing.T) {
 			g.Expect(err).To(Not(HaveOccurred()))
 			defer l.Close()
 
-			err = snap.PreInitChecks(context.Background(), conf, serviceConfigs)
+			err = snap.PreInitChecks(context.Background(), conf, serviceConfigs, true)
 			g.Expect(err).To(HaveOccurred())
 		})
 
@@ -177,15 +177,15 @@ func TestSnap(t *testing.T) {
 			f.Close()
 			defer os.Remove(f.Name())
 
-			err = snap.PreInitChecks(context.Background(), conf, serviceConfigs)
+			err = snap.PreInitChecks(context.Background(), conf, serviceConfigs, true)
 			g.Expect(err).To(HaveOccurred())
 		})
 
 		t.Run("Fail run command", func(t *testing.T) {
 			g := NewWithT(t)
 			mockRunner.Err = fmt.Errorf("some error")
 
-			err := snap.PreInitChecks(context.Background(), conf, serviceConfigs)
+			err := snap.PreInitChecks(context.Background(), conf, serviceConfigs, true)
 			g.Expect(err).To(HaveOccurred())
 		})
 	})

src/k8s/pkg/utils/checks.go

@@ -0,0 +1,58 @@
+package utils
+
+import (
+	"errors"
+	"fmt"
+	"strconv"
+
+	"github.com/canonical/k8s/pkg/k8sd/types"
+)
+
+// CheckK8sServicePorts verifies that the Kubernetes-related ports are free to be used.
+// The ports checked depends on whether a node is a control plane node, or a worker node.
+func CheckK8sServicePorts(config types.ClusterConfig, serviceConfigs types.K8sServiceConfigs, isControlPlane bool) error {
+	var allErrors []error
+	ports := map[string]string{
+		// Default values from official Kubernetes documentation.
+		"kubelet":           serviceConfigs.GetKubeletPort(),
+		"kubelet-healthz":   serviceConfigs.GetKubeletHealthzPort(),
+		"kubelet-read-only": serviceConfigs.GetKubeletReadOnlyPort(),
+		"k8s-dqlite":        strconv.Itoa(config.Datastore.GetK8sDqlitePort()),
+		"loadbalancer":      strconv.Itoa(config.LoadBalancer.GetBGPPeerPort()),
+	}
+
+	if port, err := serviceConfigs.GetKubeProxyHealthzPort(); err != nil {
+		allErrors = append(allErrors, err)
+	} else {
+		ports["kube-proxy-healhz"] = port
+	}
+
+	if port, err := serviceConfigs.GetKubeProxyMetricsPort(); err != nil {
+		allErrors = append(allErrors, err)
+	} else {
+		ports["kube-proxy-metrics"] = port
+	}
+
+	if isControlPlane {
+		ports["kube-apiserver"] = strconv.Itoa(config.APIServer.GetSecurePort())
+		ports["kube-scheduler"] = serviceConfigs.GetKubeSchedulerPort()
+		ports["kube-controller-manager"] = serviceConfigs.GetKubeControllerManagerPort()
+	} else {
+		ports["kube-apiserver-proxy"] = strconv.Itoa(config.APIServer.GetSecurePort())
+	}
+
+	for service, port := range ports {
+		if port == "0" {
+			// Some ports may be set to 0 in order to disable them. No need to check.
+			continue
+		}
+		if open, err := IsLocalPortOpen(port); err != nil {
+			// Could not open port due to error.
+			allErrors = append(allErrors, fmt.Errorf("could not check port %s (needed by: %s): %w", port, service, err))
+		} else if !open {
+			allErrors = append(allErrors, fmt.Errorf("port %s (needed by: %s) is already in use.", port, service))
+		}
+	}
+
+	return errors.Join(allErrors...)
+}