fix: set hcp initialized earlier (#139)

Otherwise it gets stuck as no workload resources will get ready.

Author: cwrau

go.mod

@@ -373,13 +373,6 @@ func (r *hostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
 				return reconcile.Result{}, fmt.Errorf("failed to get HostedControlPlane: %w", err)
 			}
 
-			patchHelper, err := patch.NewHelper(hostedControlPlane, r.client)
-			if err != nil {
-				return ctrl.Result{}, fmt.Errorf("failed to create patch helper for HostedControlPlane: %w", err)
-			}
-
-			hostedControlPlane.Status.ExternalManagedControlPlane = new(true)
-
 			ctx = recorder.IntoContext(ctx, recorder.New(r.recorder, hostedControlPlane))
 
 			cluster, err := util.GetOwnerCluster(ctx, r.client, hostedControlPlane.ObjectMeta)
@@ -421,13 +414,33 @@ func (r *hostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
 			}
 
 			isPaused, requeue, err := paused.EnsurePausedCondition(ctx, r.client, cluster, hostedControlPlane)
-			if err != nil || isPaused || requeue {
-				if err != nil {
-					return ctrl.Result{}, fmt.Errorf("failed to verify paused condition: %w", err)
-				}
+			if err != nil {
+				return ctrl.Result{}, fmt.Errorf("failed to verify paused condition: %w", err)
+			} else if isPaused || requeue {
 				return ctrl.Result{RequeueAfter: 10 * time.Second}, retErr
 			}
 
+			if hostedControlPlane.DeletionTimestamp.IsZero() {
+				if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.client,
+					hostedControlPlane, r.finalizer,
+				); err != nil {
+					return ctrl.Result{}, errorsUtil.IfErrErrorf("failed to ensure finalizer: %w", err)
+				} else if finalizerAdded {
+					emit.Info(ctx, emit.SinkSpanEvent, hostedControlPlane,
+						"FinalizerMissing", "FinalizerAdded",
+						"Added missing finalizer",
+					)
+					return ctrl.Result{}, nil
+				}
+			}
+
+			patchHelper, err := patch.NewHelper(hostedControlPlane, r.client)
+			if err != nil {
+				return ctrl.Result{}, fmt.Errorf("failed to create patch helper for HostedControlPlane: %w", err)
+			}
+
+			hostedControlPlane.Status.ExternalManagedControlPlane = new(true)
+
 			defer func() {
 				hostedControlPlane.Status.ObservedGeneration = hostedControlPlane.Generation
 				if err := r.patch(ctx, patchHelper, hostedControlPlane); err != nil {
@@ -436,10 +449,10 @@ func (r *hostedControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl.R
 			}()
 
 			if !hostedControlPlane.DeletionTimestamp.IsZero() {
-				return r.reconcileDelete(ctx, patchHelper, hostedControlPlane)
+				return r.reconcileDelete(ctx, hostedControlPlane)
 			}
 
-			return r.reconcileNormal(ctx, patchHelper, hostedControlPlane, cluster)
+			return r.reconcileNormal(ctx, hostedControlPlane, cluster)
 		},
 	)
 }
@@ -498,80 +511,103 @@ func (r *hostedControlPlaneReconciler) patch(
 //nolint:funlen // this function is not really complex
 func (r *hostedControlPlaneReconciler) reconcileNormal(
 	ctx context.Context,
-	_ *patch.Helper,
 	hostedControlPlane *v1alpha1.HostedControlPlane,
 	cluster *capiv2.Cluster,
 ) (ctrl.Result, error) {
 	return tracing.WithSpan(ctx, r.tracer, "ReconcileNormal",
 		func(ctx context.Context, span trace.Span) (ctrl.Result, error) {
-			if finalizerAdded, err := finalizers.EnsureFinalizer(ctx, r.client,
-				hostedControlPlane, r.finalizer,
-			); err != nil || finalizerAdded {
-				return ctrl.Result{}, errorsUtil.IfErrErrorf("failed to ensure finalizer: %w", err)
-			}
 			span.SetAttributes(
 				attribute.String("hostedcontrolplane.version", hostedControlPlane.Spec.Version),
 				attribute.Int("hostedcontrolplane.replicas", int(hostedControlPlane.Spec.ReplicasOrDefault())),
 			)
 
 			type Phase struct {
 				Reconcile    func(context.Context, *v1alpha1.HostedControlPlane, *capiv2.Cluster) (string, error)
-				Condition    capiv2.ConditionType
+				Condition    string
 				FailedReason string
 				Name         string
 			}
 
 			serviceDomain := "cluster.local"
-			serviceCIDR := "10.96.0.0/12"
-			podCIDR := "10.0.0.0/16"
-
-			if clusterSpecServiceDomain := cluster.Spec.ClusterNetwork.ServiceDomain; clusterSpecServiceDomain != "" {
-				serviceDomain = clusterSpecServiceDomain
+			serviceCIDR := net.IPNet{
+				IP:   net.IPv4(10, 96, 0, 0),
+				Mask: net.CIDRMask(12, 32),
 			}
-
-			if clusterServiceCIDR := cluster.Spec.ClusterNetwork.Services.String(); clusterServiceCIDR != "" {
-				serviceCIDR = clusterServiceCIDR
+			podCIDR := net.IPNet{
+				IP:   net.IPv4(10, 0, 0, 0),
+				Mask: net.CIDRMask(16, 32),
 			}
 
-			if clusterPodCIDR := cluster.Spec.ClusterNetwork.Pods.String(); clusterPodCIDR != "" {
-				podCIDR = clusterPodCIDR
+			if clusterSpecServiceDomain := cluster.Spec.ClusterNetwork.ServiceDomain; clusterSpecServiceDomain != "" {
+				serviceDomain = clusterSpecServiceDomain
 			}
 
-			var dnsIP net.IP
-			var kubernetesServiceIP net.IP
-			if _, serviceNet, err := net.ParseCIDR(strings.Split(serviceCIDR, ",")[0]); err != nil {
-				conditions.Set(hostedControlPlane, metav1.Condition{
-					Type:    v1alpha1.WorkloadClusterResourcesReadyCondition,
-					Status:  metav1.ConditionFalse,
-					Reason:  v1alpha1.WorkloadClusterResourcesFailedReason,
-					Message: fmt.Sprintf("Failed to parse Service CIDR %q: %v", serviceCIDR, err),
-				})
-				return ctrl.Result{}, errorsUtil.IfErrErrorf("failed to parse Service CIDR %q: %w", serviceCIDR, err)
-			} else {
-				dnsIP, err = utilNet.GetIndexedIP(serviceNet, 10)
-				if err != nil {
+			if clusterNetwork := cluster.Spec.ClusterNetwork.Services.String(); clusterNetwork != "" {
+				if _, clusterServiceCIDR, err := net.ParseCIDR(clusterNetwork); err != nil {
 					conditions.Set(hostedControlPlane, metav1.Condition{
 						Type:    v1alpha1.WorkloadClusterResourcesReadyCondition,
 						Status:  metav1.ConditionFalse,
 						Reason:  v1alpha1.WorkloadClusterResourcesFailedReason,
-						Message: fmt.Sprintf("Failed to calculate DNS IP from Service CIDR %q: %v", serviceCIDR, err),
+						Message: fmt.Sprintf("Failed to parse Service CIDR %q: %v", clusterNetwork, err),
 					})
-					return ctrl.Result{}, errorsUtil.IfErrErrorf("failed to calculate DNS IP from Service CIDR %q: %w",
-						serviceCIDR, err)
+					return ctrl.Result{}, errorsUtil.IfErrErrorf(
+						"failed to parse Service CIDR %q: %w",
+						clusterNetwork,
+						err,
+					)
+				} else {
+					serviceCIDR = *clusterServiceCIDR
 				}
-				kubernetesServiceIP, err = utilNet.GetIndexedIP(serviceNet, 1)
-				if err != nil {
+			}
+
+			if clusterPodNetwork := cluster.Spec.ClusterNetwork.Pods.String(); clusterPodNetwork != "" {
+				if _, clusterPodCIDR, err := net.ParseCIDR(clusterPodNetwork); err != nil {
 					conditions.Set(hostedControlPlane, metav1.Condition{
 						Type:    v1alpha1.WorkloadClusterResourcesReadyCondition,
 						Status:  metav1.ConditionFalse,
 						Reason:  v1alpha1.WorkloadClusterResourcesFailedReason,
-						Message: fmt.Sprintf("Failed to calculate Kubernetes Service IP from Service CIDR %q: %v", serviceCIDR, err),
+						Message: fmt.Sprintf("Failed to parse Pod CIDR %q: %v", clusterPodNetwork, err),
 					})
-					return ctrl.Result{}, errorsUtil.IfErrErrorf("failed to calculate Kubernetes Service IP from Service CIDR %q: %w",
-						serviceCIDR, err)
+					return ctrl.Result{}, errorsUtil.IfErrErrorf(
+						"failed to parse Pod CIDR %q: %w",
+						clusterPodNetwork,
+						err,
+					)
+				} else {
+					podCIDR = *clusterPodCIDR
 				}
 			}
 
+			dnsIP, err := utilNet.GetIndexedIP(&serviceCIDR, 10)
+			if err != nil {
+				conditions.Set(hostedControlPlane, metav1.Condition{
+					Type:    v1alpha1.WorkloadClusterResourcesReadyCondition,
+					Status:  metav1.ConditionFalse,
+					Reason:  v1alpha1.WorkloadClusterResourcesFailedReason,
+					Message: fmt.Sprintf("Failed to calculate DNS IP from Service CIDR %q: %v", serviceCIDR, err),
+				})
+				return ctrl.Result{}, errorsUtil.IfErrErrorf("failed to calculate DNS IP from Service CIDR %q: %w",
+					serviceCIDR, err)
+			}
+			kubernetesServiceIP, err := utilNet.GetIndexedIP(&serviceCIDR, 1)
+			if err != nil {
+				conditions.Set(hostedControlPlane, metav1.Condition{
+					Type:   v1alpha1.WorkloadClusterResourcesReadyCondition,
+					Status: metav1.ConditionFalse,
+					Reason: v1alpha1.WorkloadClusterResourcesFailedReason,
+					Message: fmt.Sprintf(
+						"Failed to calculate Kubernetes Service IP from Service CIDR %q: %v",
+						serviceCIDR,
+						err,
+					),
+				})
+				return ctrl.Result{}, errorsUtil.IfErrErrorf(
+					"failed to calculate Kubernetes Service IP from Service CIDR %q: %w",
+					serviceCIDR,
+					err,
+				)
+			}
+
 			var ciliumClient ciliumclient.Interface
 
 			groups, err := r.managementClusterClient.Discovery().ServerGroups()
@@ -714,8 +750,28 @@ func (r *hostedControlPlaneReconciler) reconcileNormal(
 					FailedReason: v1alpha1.EtcdClusterFailedReason,
 				},
 				{
-					Name:         "apiserver deployments",
-					Reconcile:    apiServerResourcesReconciler.ReconcileApiServerDeployments,
+					Name: "apiserver deployments",
+					Reconcile: func(
+						ctx context.Context,
+						hostedControlPlane *v1alpha1.HostedControlPlane,
+						cluster *capiv2.Cluster,
+					) (string, error) {
+						notReady, err := apiServerResourcesReconciler.ReconcileApiServerDeployments(
+							ctx,
+							hostedControlPlane,
+							cluster,
+						)
+						if err != nil {
+							return "", fmt.Errorf("reconcile api server deployments: %w", err)
+						}
+						if notReady != "" {
+							return notReady, nil
+						}
+
+						hostedControlPlane.Status.Initialization.ControlPlaneInitialized = new(true)
+
+						return "", nil
+					},
 					Condition:    v1alpha1.APIServerDeploymentsReadyCondition,
 					FailedReason: v1alpha1.APIServerDeploymentsFailedReason,
 				},
@@ -751,23 +807,23 @@ func (r *hostedControlPlaneReconciler) reconcileNormal(
 				); {
 				case err != nil:
 					conditions.Set(hostedControlPlane, metav1.Condition{
-						Type:    string(phase.Condition),
+						Type:    phase.Condition,
 						Status:  metav1.ConditionFalse,
 						Reason:  phase.FailedReason,
 						Message: fmt.Sprintf("Reconciling phase %s failed: %v", phase.Name, err),
 					})
 					return reconcile.Result{}, err
 				case notReadyReason != "":
 					conditions.Set(hostedControlPlane, metav1.Condition{
-						Type:    string(phase.Condition),
+						Type:    phase.Condition,
 						Status:  metav1.ConditionFalse,
 						Reason:  notReadyReason,
 						Message: fmt.Sprintf("phase %s not ready", phase.Name),
 					})
 					return reconcile.Result{RequeueAfter: 10 * time.Second}, nil
 				default:
 					conditions.Set(hostedControlPlane, metav1.Condition{
-						Type:    string(phase.Condition),
+						Type:    phase.Condition,
 						Status:  metav1.ConditionTrue,
 						Reason:  "ReconcileSucceeded",
 						Message: fmt.Sprintf("Management phase %s reconciled successfully", phase.Name),
@@ -784,7 +840,6 @@ func (r *hostedControlPlaneReconciler) reconcileNormal(
 
 func (r *hostedControlPlaneReconciler) reconcileDelete(
 	ctx context.Context,
-	_ *patch.Helper,
 	hostedControlPlane *v1alpha1.HostedControlPlane,
 ) (ctrl.Result, error) {
 	return tracing.WithSpan(ctx, r.tracer, "ReconcileDelete",

go.sum

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	certmanagerfake "github.com/cert-manager/cert-manager/pkg/client/clientset/versioned/fake"
 	ciliumclient "github.com/cilium/cilium/pkg/k8s/client/clientset/versioned"
 	. "github.com/onsi/gomega"
 	"github.com/teutonet/cluster-api-provider-hosted-control-plane/api/v1alpha1"
@@ -63,7 +64,7 @@ func createTestReconcilerWithFilter(client client.Client, reconcileFilter string
 	return NewHostedControlPlaneReconciler(
 		client,
 		&alias.ManagementClusterClient{Interface: fake.NewClientset()},
-		nil,
+		&certmanagerfake.Clientset{},
 		nil,
 		func(ctx context.Context) (ciliumclient.Interface, error) {
 			return nil, nil
@@ -177,6 +178,12 @@ func withGeneration(hcp *v1alpha1.HostedControlPlane, generation int64) *v1alpha
 	return newHCP
 }
 
+func withFinalizer(hcp *v1alpha1.HostedControlPlane) *v1alpha1.HostedControlPlane {
+	newHCP := hcp.DeepCopy()
+	newHCP.Finalizers = []string{"hcp.controlplane.cluster.x-k8s.io"}
+	return newHCP
+}
+
 func TestHostedControlPlaneReconciler_ReconcileWorkflow(t *testing.T) {
 	cluster := createTestCluster("test-cluster", "default")
 	tests := []struct {
@@ -501,7 +508,7 @@ func TestHostedControlPlaneReconciler_ObservedGeneration(t *testing.T) {
 	}{
 		{
 			name: "normal reconciliation should set observedGeneration",
-			hostedControlPlane: withGeneration(
+			hostedControlPlane: withFinalizer(withGeneration(
 				withConditions(
 					withReplicas(
 						withOwnerReference(
@@ -517,7 +524,7 @@ func TestHostedControlPlaneReconciler_ObservedGeneration(t *testing.T) {
 					}},
 				),
 				2,
-			),
+			)),
 			cluster: withEndpoint(
 				createTestClusterWithPausedCondition("test-cluster", "default", false),
 				createTestHostedControlPlane("test-hcp", "default"),

pkg/hostedcontrolplane/controller.go

@@ -296,15 +296,16 @@ func TestHostedControlPlane_FullLifecycle(t *testing.T) {
 			},
 		},
 		{
-			name: "Verify ExternalManagedControlPlane Status",
+			name: "Add Finalizer",
 			verifyResources: func(ctx context.Context, g Gomega) {
-				g.Expect(hcp.Status.ExternalManagedControlPlane).To(PointTo(BeTrue()))
+				g.Expect(hcp.Finalizers).To(ContainElement("hcp.controlplane.cluster.x-k8s.io"))
 			},
+			expectNoGenerationBump: true,
 		},
 		{
-			name: "Add Finalizer",
+			name: "Verify ExternalManagedControlPlane Status",
 			verifyResources: func(ctx context.Context, g Gomega) {
-				g.Expect(hcp.Finalizers).To(ContainElement("hcp.controlplane.cluster.x-k8s.io"))
+				g.Expect(hcp.Status.ExternalManagedControlPlane).To(PointTo(BeTrue()))
 			},
 		},
 		{
@@ -794,6 +795,9 @@ func TestHostedControlPlane_FullLifecycle(t *testing.T) {
 					),
 				},
 			},
+			verifyResources: func(ctx context.Context, g Gomega) {
+				g.Expect(*hcp.Status.Initialization.ControlPlaneInitialized).To(BeTrue())
+			},
 		},
 		{
 			name: "Make Api Server TLS Route Ready",
@@ -1469,8 +1473,8 @@ func simulateK8sAPI(ctx context.Context, kubernetesClient kubernetes.Interface,
 	}
 }
 
-func verifyConditions(after map[bool][]types2.GomegaMatcher, hcp *v1alpha1.HostedControlPlane, g Gomega) {
-	for status, matchers := range after {
+func verifyConditions(conditionMatchers map[bool][]types2.GomegaMatcher, hcp *v1alpha1.HostedControlPlane, g Gomega) {
+	for status, matchers := range conditionMatchers {
 		conditionStatus := slices.Ternary(status, metav1.ConditionTrue, metav1.ConditionFalse)
 		partitionedConditions := slices.GroupBy(hcp.Status.Conditions,
 			func(condition metav1.Condition) bool {