addressed comments

This commit is contained in:
Enrico Candino
2026-06-16 12:16:48 +02:00
parent 5ed97d942e
commit 65c30fb755
8 changed files with 179 additions and 52 deletions
+1 -2
View File
@@ -5,7 +5,6 @@ import (
"net/url"
"os"
"path/filepath"
"strings"
"time"
"github.com/sirupsen/logrus"
@@ -147,7 +146,7 @@ func resolveServerHost(restConfigHost, override string) (string, error) {
return "", err
}
return strings.Split(u.Host, ":")[0], nil
return u.Hostname(), nil
}
func writeKubeconfigFile(cluster *v1beta1.Cluster, kubeconfig *clientcmdapi.Config, configName string) error {
+11 -8
View File
@@ -448,11 +448,13 @@ func (c *ClusterReconciler) reconcile(ctx context.Context, cluster *v1beta1.Clus
return err
}
// In hcp mode, derive the K3s installer command end-users run on their
// external nodes and surface it on the Cluster status. We also own the
// default/kubernetes Endpoints inside the virtual cluster (the apiserver
// reconciler is disabled for HCP) so external-node pods can reach the
// in-cluster apiserver ClusterIP.
// In hcp mode, validate that the cluster has an externally-routable
// API server endpoint (NodePort / LoadBalancer / Ingress) so external
// workers can join. The join command itself is printed by the CLI
// (`k3kcli cluster create` / `k3kcli kubeconfig generate`).
// We also own the default/kubernetes Endpoints/EndpointSlice inside the
// virtual cluster (the apiserver reconciler is disabled for HCP) so
// external-node pods can reach the in-cluster apiserver ClusterIP.
if cluster.Spec.Mode == v1beta1.HCPClusterMode {
if err := c.ensureHCPRegistration(ctx, cluster); err != nil {
return err
@@ -916,9 +918,10 @@ func (c *ClusterReconciler) bindClusterRoles(ctx context.Context, cluster *v1bet
}
func (c *ClusterReconciler) ensureAgent(ctx context.Context, cluster *v1beta1.Cluster, serviceIP, token string) error {
// hcp mode is BYO-node by design: external (out-of-host-cluster) nodes join
// using the standard K3s installer command surfaced via Status.HCPRegistration.
// k3k therefore does not provision any agent pods on the host cluster.
// hcp mode is BYO-node by design: external (out-of-host-cluster) nodes
// join using the standard K3s installer command, which is printed by
// `k3kcli cluster create` / `k3kcli kubeconfig generate`. k3k therefore
// does not provision any agent pods on the host cluster.
if cluster.Spec.Mode == v1beta1.HCPClusterMode {
return nil
}
+34 -28
View File
@@ -2,12 +2,12 @@ package cluster
import (
"context"
"errors"
"fmt"
"net"
"net/url"
"strconv"
"k8s.io/apimachinery/pkg/api/meta"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
corev1 "k8s.io/api/core/v1"
@@ -19,13 +19,22 @@ import (
"github.com/rancher/k3k/pkg/controller/cluster/server"
)
// ensureHCPRegistration computes the K3s installer command external nodes can
// run to join an HCP-mode cluster and stores it on cluster.Status.HCPRegistration.
// ErrHCPNoExternalEndpoint is returned by ensureHCPRegistration when an
// HCP-mode cluster has no externally-routable endpoint (no NodePort,
// LoadBalancer or Ingress) so external worker nodes cannot reach the API
// server. updateStatus translates it into a Ready=False condition with
// reason HCPNoExternalEndpoint instead of failing the reconcile outright.
var ErrHCPNoExternalEndpoint = errors.New("HCP cluster has no external endpoint")
// ensureHCPRegistration verifies that an HCP-mode cluster exposes an
// externally-routable API server endpoint so external worker nodes can join.
// Join instructions (the `curl ... | sh -` line) are printed by the CLI
// (`k3kcli cluster create` / `k3kcli kubeconfig generate`); the controller
// does not persist them on the Cluster object.
//
// When the cluster's Service is not externally reachable (no NodePort,
// LoadBalancer or Ingress configured) the command cannot be built; in that
// case the Ready condition is set to False with reason HCPNoExternalEndpoint
// so the operator surfaces the problem without failing the reconciliation.
// Returns ErrHCPNoExternalEndpoint when no NodePort, LoadBalancer or Ingress
// is configured, which updateStatus surfaces as Ready=False with reason
// HCPNoExternalEndpoint.
func (c *ClusterReconciler) ensureHCPRegistration(ctx context.Context, cluster *v1beta1.Cluster) error {
log := ctrl.LoggerFrom(ctx)
@@ -35,15 +44,10 @@ func (c *ClusterReconciler) ensureHCPRegistration(ctx context.Context, cluster *
}
if !external {
log.Info("HCP cluster has no externally-routable endpoint; skipping registration command",
log.Info("HCP cluster has no externally-routable endpoint",
"cluster", cluster.Name, "namespace", cluster.Namespace)
meta.SetStatusCondition(&cluster.Status.Conditions, metav1.Condition{
Type: ConditionReady,
Status: metav1.ConditionFalse,
Reason: ReasonHCPNoExternalEndpoint,
Message: "HCP cluster has no external endpoint; set spec.expose.nodePort, spec.expose.loadBalancer or spec.expose.ingress so external nodes can reach the API server",
})
return ErrHCPNoExternalEndpoint
}
return nil
@@ -103,8 +107,9 @@ func (c *ClusterReconciler) ensureHCPKubernetesEndpointSlice(ctx context.Context
}
if !external {
// ensureHCPRegistration already surfaces this via Ready=False;
// nothing for us to do here.
// Defensive: reconcile would have already short-circuited with
// ErrHCPNoExternalEndpoint via ensureHCPRegistration before reaching
// here, but skip gracefully if invoked directly.
return nil
}
@@ -113,7 +118,7 @@ func (c *ClusterReconciler) ensureHCPKubernetesEndpointSlice(ctx context.Context
return fmt.Errorf("parsing HCP server URL %q: %w", rawURL, err)
}
addr, err := hcpEndpointAddress(host)
addr, err := hcpEndpointAddress(ctx, host)
if err != nil {
return err
}
@@ -185,8 +190,9 @@ func (c *ClusterReconciler) ensureHCPKubernetesEndpoints(ctx context.Context, cl
}
if !external {
// ensureHCPRegistration already surfaces this via Ready=False;
// nothing for us to do here.
// Defensive: reconcile would have already short-circuited with
// ErrHCPNoExternalEndpoint via ensureHCPRegistration before reaching
// here, but skip gracefully if invoked directly.
return nil
}
@@ -195,7 +201,7 @@ func (c *ClusterReconciler) ensureHCPKubernetesEndpoints(ctx context.Context, cl
return fmt.Errorf("parsing HCP server URL %q: %w", rawURL, err)
}
addr, err := hcpEndpointAddress(host)
addr, err := hcpEndpointAddress(ctx, host)
if err != nil {
return err
}
@@ -282,10 +288,10 @@ func parseHCPHostPort(rawURL string) (string, int32, error) {
// hcpEndpointAddress builds a corev1.EndpointAddress from the externally
// reachable host. Endpoints require an IP; if the host is a DNS name we
// resolve it. The Hostname field is intentionally left unset: the
// kubernetes API validates it as a DNS-1123 label (no dots), so an FQDN
// like "host.example.com" would be rejected.
func hcpEndpointAddress(host string) (corev1.EndpointAddress, error) {
// resolve it. The Hostname field is intentionally left unset:
// the kubernetes API validates it as a DNS-1123 label (no dots),
// so an FQDN like "host.example.com" would be rejected.
func hcpEndpointAddress(ctx context.Context, host string) (corev1.EndpointAddress, error) {
if ip := net.ParseIP(host); ip != nil {
if ip.IsLoopback() {
return corev1.EndpointAddress{}, fmt.Errorf("HCP endpoint host %q is a loopback address and cannot be used", host)
@@ -294,16 +300,16 @@ func hcpEndpointAddress(host string) (corev1.EndpointAddress, error) {
return corev1.EndpointAddress{IP: host}, nil
}
ips, err := net.LookupIP(host)
ipAddrs, err := net.DefaultResolver.LookupIPAddr(ctx, host)
if err != nil {
return corev1.EndpointAddress{}, fmt.Errorf("HCP endpoint host %q is not an IP and does not resolve: %w", host, err)
}
var filteredIPs []net.IP
for _, ip := range ips {
if !ip.IsLoopback() {
filteredIPs = append(filteredIPs, ip)
for _, addr := range ipAddrs {
if !addr.IP.IsLoopback() {
filteredIPs = append(filteredIPs, addr.IP)
}
}
+27 -8
View File
@@ -2,11 +2,11 @@ package cluster
import (
"context"
"errors"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -35,7 +35,7 @@ func Test_ensureHCPRegistration(t *testing.T) {
},
}
t.Run("clusterip-only service sets degraded condition and clears registration", func(t *testing.T) {
t.Run("clusterip-only service returns ErrHCPNoExternalEndpoint", func(t *testing.T) {
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: server.ServiceName(cluster.Name),
@@ -54,12 +54,31 @@ func Test_ensureHCPRegistration(t *testing.T) {
r := &ClusterReconciler{Client: fakeClient}
c := cluster.DeepCopy()
require.NoError(t, r.ensureHCPRegistration(context.Background(), c))
err := r.ensureHCPRegistration(context.Background(), c)
require.Error(t, err)
assert.True(t, errors.Is(err, ErrHCPNoExternalEndpoint))
})
cond := meta.FindStatusCondition(c.Status.Conditions, ConditionReady)
require.NotNil(t, cond)
assert.Equal(t, metav1.ConditionFalse, cond.Status)
assert.Equal(t, ReasonHCPNoExternalEndpoint, cond.Reason)
t.Run("nodeport service returns no error", func(t *testing.T) {
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: server.ServiceName(cluster.Name),
Namespace: cluster.Namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeNodePort,
ClusterIP: "10.43.0.50",
Ports: []corev1.ServicePort{
{Name: "k3s-server-port", Port: 443, NodePort: 30443},
},
},
}
fakeClient := fake.NewClientBuilder().WithScheme(scheme).WithObjects(svc).Build()
r := &ClusterReconciler{Client: fakeClient}
c := cluster.DeepCopy()
assert.NoError(t, r.ensureHCPRegistration(context.Background(), c))
})
}
@@ -240,7 +259,7 @@ func Test_hcpEndpointAddress(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := hcpEndpointAddress(tt.input)
got, err := hcpEndpointAddress(context.Background(), tt.input)
if tt.wantErr {
require.Error(t, err)
return
+1 -1
View File
@@ -79,7 +79,7 @@ func buildServerConfig(cluster *v1beta1.Cluster, initServer bool, serviceIP, tok
// shared and hcp modes both run K3s with --disable-agent (agentless server).
switch cluster.Spec.Mode {
case v1beta1.SharedClusterMode:
case "", v1beta1.SharedClusterMode:
serverConfig.DisableAgent = true
serverConfig.EgressSelectorMode = "disabled"
serverConfig.Disable = []string{"servicelb", "traefik", "metrics-server", "local-storage"}
+13 -5
View File
@@ -51,12 +51,20 @@ func ServerURL(ctx context.Context, c client.Client, cluster *v1beta1.Cluster, h
port = k3kService.Spec.Ports[0].NodePort
}
case corev1.ServiceTypeLoadBalancer:
external = true
if len(k3kService.Status.LoadBalancer.Ingress) > 0 {
ip = k3kService.Status.LoadBalancer.Ingress[0].IP
} else {
logrus.Warn("No ingress found in LoadBalancer service.")
ingress := k3kService.Status.LoadBalancer.Ingress[0]
switch {
case ingress.IP != "":
ip = ingress.IP
external = true
case ingress.Hostname != "":
ip = ingress.Hostname
external = true
}
}
if !external {
logrus.Warn("No usable ingress address found in LoadBalancer service.")
}
if len(k3kService.Spec.Ports) > 0 {
@@ -0,0 +1,80 @@
package server
import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/runtime"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"github.com/rancher/k3k/pkg/apis/k3k.io/v1beta1"
)
func Test_ServerURL_LoadBalancer(t *testing.T) {
scheme := runtime.NewScheme()
require.NoError(t, corev1.AddToScheme(scheme))
require.NoError(t, v1beta1.AddToScheme(scheme))
cluster := &v1beta1.Cluster{
ObjectMeta: metav1.ObjectMeta{Name: "demo", Namespace: "team-a"},
Status: v1beta1.ClusterStatus{TLSSANs: []string{"203.0.113.10", "lb.example.com"}},
}
tests := []struct {
name string
ingress []corev1.LoadBalancerIngress
wantURL string
wantExternal bool
}{
{
name: "no ingress yet (LB still provisioning)",
ingress: nil,
wantExternal: false,
},
{
name: "ingress with IP",
ingress: []corev1.LoadBalancerIngress{{IP: "203.0.113.10"}},
wantURL: "https://203.0.113.10",
wantExternal: true,
},
{
name: "ingress with hostname only",
ingress: []corev1.LoadBalancerIngress{{Hostname: "lb.example.com"}},
wantURL: "https://lb.example.com",
wantExternal: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
svc := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: ServiceName(cluster.Name),
Namespace: cluster.Namespace,
},
Spec: corev1.ServiceSpec{
Type: corev1.ServiceTypeLoadBalancer,
ClusterIP: "10.43.0.50",
Ports: []corev1.ServicePort{{Name: "k3s-server-port", Port: 443}},
},
Status: corev1.ServiceStatus{
LoadBalancer: corev1.LoadBalancerStatus{Ingress: tt.ingress},
},
}
c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(svc).Build()
url, external, err := ServerURL(context.Background(), c, cluster, "", 0)
require.NoError(t, err)
assert.Equal(t, tt.wantExternal, external)
if tt.wantExternal {
assert.Equal(t, tt.wantURL, url)
}
})
}
}
+12
View File
@@ -70,6 +70,18 @@ func (c *ClusterReconciler) updateStatus(ctx context.Context, cluster *v1beta1.C
return
}
if errors.Is(reconcileErr, ErrHCPNoExternalEndpoint) {
cluster.Status.Phase = v1beta1.ClusterPending
meta.SetStatusCondition(&cluster.Status.Conditions, metav1.Condition{
Type: ConditionReady,
Status: metav1.ConditionFalse,
Reason: ReasonHCPNoExternalEndpoint,
Message: "HCP cluster has no external endpoint; set spec.expose.nodePort, spec.expose.loadBalancer or spec.expose.ingress so external nodes can reach the API server",
})
return
}
// If there's an error, but it's not a validation error, the cluster is in a failed state.
if reconcileErr != nil {
cluster.Status.Phase = v1beta1.ClusterFailed