OCPBUGS-98643: Add readiness probes to konnectivity proxy sidecars

support/controlplane-component/konnectivity-container.go

@@ -13,6 +13,7 @@ import (
 
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/ptr"
 )
 
@@ -24,6 +25,8 @@ const (
 
 	// Dual mode will inject 2 konnectivity containers, one using HTTPS mode and the other using Socks5 mode.
 	Dual ProxyMode = "dual"
+
+	defaultKonnectivityServingPort uint32 = 8090
 )
 
 type KonnectivityContainerOptions struct {
@@ -144,9 +147,6 @@ func (opts KonnectivityContainerOptions) buildContainer(hcp *hyperv1.HostedContr
 		if port := opts.HTTPSOptions.KonnectivityPort; port != 0 {
 			args = append(args, fmt.Sprintf("--konnectivity-port=%d", port))
 		}
-		if servingPort := opts.HTTPSOptions.ServingPort; servingPort != 0 {
-			args = append(args, fmt.Sprintf("--serving-port=%d", servingPort))
-		}
 		if value := opts.HTTPSOptions.ConnectDirectlyToCloudAPIs; value != nil {
 			args = append(args, fmt.Sprintf("--connect-directly-to-cloud-apis=%t", *value))
 		}
@@ -158,9 +158,6 @@ func (opts KonnectivityContainerOptions) buildContainer(hcp *hyperv1.HostedContr
 		if port := opts.Socks5Options.KonnectivityPort; port != 0 {
 			args = append(args, fmt.Sprintf("--konnectivity-port=%d", port))
 		}
-		if servingPort := opts.Socks5Options.ServingPort; servingPort != 0 {
-			args = append(args, fmt.Sprintf("--serving-port=%d", servingPort))
-		}
 		if value := opts.Socks5Options.ConnectDirectlyToCloudAPIs; value != nil {
 			args = append(args, fmt.Sprintf("--connect-directly-to-cloud-apis=%t", *value))
 		}
@@ -175,6 +172,8 @@ func (opts KonnectivityContainerOptions) buildContainer(hcp *hyperv1.HostedContr
 		}
 	}
 
+	args = append(args, fmt.Sprintf("--serving-port=%d", opts.servingPort()))
+
 	kubeconfingVolumeName := opts.KubeconfingVolumeName
 	if kubeconfingVolumeName == "" {
 		kubeconfingVolumeName = "kubeconfig"
@@ -192,6 +191,15 @@ func (opts KonnectivityContainerOptions) buildContainer(hcp *hyperv1.HostedContr
 				corev1.ResourceMemory: resource.MustParse("30Mi"),
 			},
 		},
+		ReadinessProbe: &corev1.Probe{
+			ProbeHandler: corev1.ProbeHandler{
+				TCPSocket: &corev1.TCPSocketAction{
+					Port: intstr.FromInt32(int32(opts.servingPort())),
+				},
+			},
+			InitialDelaySeconds: 5,
+			PeriodSeconds:       5,
+		},
 		Env: []corev1.EnvVar{{
 			Name:  "KUBECONFIG",
 			Value: "/etc/kubernetes/secrets/kubeconfig/kubeconfig",
@@ -234,6 +242,22 @@ func (opts KonnectivityContainerOptions) buildContainer(hcp *hyperv1.HostedContr
 	return container
 }
 
+func (opts KonnectivityContainerOptions) servingPort() uint32 {
+	switch opts.Mode {
+	case HTTPS:
+		if opts.HTTPSOptions.ServingPort != 0 {
+			return opts.HTTPSOptions.ServingPort
+		}
+	case Socks5:
+		if opts.Socks5Options.ServingPort != 0 {
+			return opts.Socks5Options.ServingPort
+		}
+	default:
+		return defaultKonnectivityServingPort
+	}
+	return defaultKonnectivityServingPort
+}
+
 func (opts KonnectivityContainerOptions) connectsDirectlyToCloudAPIs() bool {
 	switch opts.Mode {
 	case HTTPS:

support/controlplane-component/konnectivity-container_test.go

@@ -9,6 +9,7 @@ import (
 	hyperv1 "github.com/openshift/hypershift/api/hypershift/v1beta1"
 
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/utils/ptr"
 )
 
@@ -139,6 +140,66 @@ func TestBuildContainer(t *testing.T) {
 	}
 }
 
+func TestBuildContainerReadinessProbe(t *testing.T) {
+	hcp := &hyperv1.HostedControlPlane{}
+
+	tests := []struct {
+		name         string
+		opts         KonnectivityContainerOptions
+		expectedPort int32
+	}{
+		{
+			name: "When HTTPS mode uses default serving port, it should set readiness probe on default port",
+			opts: KonnectivityContainerOptions{
+				Mode: HTTPS,
+			},
+			expectedPort: int32(defaultKonnectivityServingPort),
+		},
+		{
+			name: "When Socks5 mode uses default serving port, it should set readiness probe on default port",
+			opts: KonnectivityContainerOptions{
+				Mode: Socks5,
+			},
+			expectedPort: int32(defaultKonnectivityServingPort),
+		},
+		{
+			name: "When HTTPS mode uses custom serving port, it should set readiness probe on that port",
+			opts: KonnectivityContainerOptions{
+				Mode: HTTPS,
+				HTTPSOptions: HTTPSOptions{
+					ServingPort: 8092,
+				},
+			},
+			expectedPort: 8092,
+		},
+		{
+			name: "When Socks5 mode uses custom serving port, it should set readiness probe on that port",
+			opts: KonnectivityContainerOptions{
+				Mode: Socks5,
+				Socks5Options: Socks5Options{
+					ServingPort: 9090,
+				},
+			},
+			expectedPort: 9090,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewGomegaWithT(t)
+
+			container := tt.opts.buildContainer(hcp, "test-image:latest", nil)
+
+			g.Expect(container.ReadinessProbe).NotTo(BeNil(), "ReadinessProbe should be set")
+			g.Expect(container.ReadinessProbe.TCPSocket).NotTo(BeNil(), "ReadinessProbe should use TCPSocket")
+			g.Expect(container.ReadinessProbe.TCPSocket.Port).To(Equal(intstr.FromInt32(tt.expectedPort)),
+				"ReadinessProbe should check the serving port")
+			g.Expect(container.ReadinessProbe.InitialDelaySeconds).To(Equal(int32(5)))
+			g.Expect(container.ReadinessProbe.PeriodSeconds).To(Equal(int32(5)))
+		})
+	}
+}
+
 func TestBuildContainerDualMode(t *testing.T) {
 	g := NewGomegaWithT(t)
 	hcp := &hyperv1.HostedControlPlane{}
@@ -171,4 +232,40 @@ func TestBuildContainerDualMode(t *testing.T) {
 		"Socks5 container should not have HTTP_PROXY because ConnectDirectlyToCloudAPIs is not set on Socks5Options")
 	g.Expect(findEnvVar(socks5Container.Env, "HTTPS_PROXY")).To(BeNil(),
 		"Socks5 container should not have HTTPS_PROXY")
+
+	g.Expect(httpsContainer.ReadinessProbe).NotTo(BeNil(), "HTTPS container should have ReadinessProbe")
+	g.Expect(httpsContainer.ReadinessProbe.TCPSocket).NotTo(BeNil())
+	g.Expect(httpsContainer.ReadinessProbe.TCPSocket.Port).To(Equal(intstr.FromInt32(int32(defaultKonnectivityServingPort))),
+		"HTTPS container should probe default port when no ServingPort is set on HTTPSOptions")
+
+	g.Expect(socks5Container.ReadinessProbe).NotTo(BeNil(), "Socks5 container should have ReadinessProbe")
+	g.Expect(socks5Container.ReadinessProbe.TCPSocket).NotTo(BeNil())
+	g.Expect(socks5Container.ReadinessProbe.TCPSocket.Port).To(Equal(intstr.FromInt32(int32(defaultKonnectivityServingPort))),
+		"Socks5 container should probe default port when no ServingPort is set on Socks5Options")
+}
+
+func TestBuildContainerDualModeCustomPorts(t *testing.T) {
+	g := NewGomegaWithT(t)
+	hcp := &hyperv1.HostedControlPlane{}
+
+	opts := KonnectivityContainerOptions{
+		Mode: Dual,
+		HTTPSOptions: HTTPSOptions{
+			ServingPort: 8092,
+		},
+	}
+
+	opts.Mode = HTTPS
+	httpsContainer := opts.buildContainer(hcp, "test-image:latest", nil)
+
+	opts.Mode = Socks5
+	socks5Container := opts.buildContainer(hcp, "test-image:latest", nil)
+
+	g.Expect(httpsContainer.ReadinessProbe).NotTo(BeNil())
+	g.Expect(httpsContainer.ReadinessProbe.TCPSocket.Port).To(Equal(intstr.FromInt32(8092)),
+		"HTTPS container should probe custom port")
+
+	g.Expect(socks5Container.ReadinessProbe).NotTo(BeNil())
+	g.Expect(socks5Container.ReadinessProbe.TCPSocket.Port).To(Equal(intstr.FromInt32(int32(defaultKonnectivityServingPort))),
+		"Socks5 container should probe default port when no custom port is set")
 }