Implement grace period for KCP remote conditions

Signed-off-by: Stefan Büringer [email protected]

Author: sbueringer

api/v1beta1/machine_types.go

@@ -206,15 +206,11 @@ const (
 	// during the deletion workflow, or by a users.
 	MachineNodeDeletedV1Beta2Reason = ObjectDeletedV1Beta2Reason
 
-	// MachineNodeRemoteConnectionFailedV1Beta2Reason surfaces that the remote connection failed.
-	// If the remote connection probe failed for longer than remote conditions grace period,
-	// this reason is used when setting NodeHealthy and NodeReady conditions to `Unknown`.
-	MachineNodeRemoteConnectionFailedV1Beta2Reason = RemoteConnectionFailedV1Beta2Reason
-
-	// MachineNodeRemoteConnectionDownV1Beta2Reason surfaces that the remote connection is down.
-	// This is used when setting NodeHealthy and NodeReady conditions to `Unknown`
-	// when the connection is down and they haven't been set yet.
-	MachineNodeRemoteConnectionDownV1Beta2Reason = RemoteConnectionDownV1Beta2Reason
+	// MachineNodeInspectionFailedV1Beta2Reason documents a failure when inspecting the status of a Node.
+	MachineNodeInspectionFailedV1Beta2Reason = InspectionFailedV1Beta2Reason
+
+	// MachineNodeConnectionDownV1Beta2Reason surfaces that the connection to the workload cluster is down.
+	MachineNodeConnectionDownV1Beta2Reason = ConnectionDownV1Beta2Reason
 )
 
 // Machine's HealthCheckSucceeded condition and corresponding reasons that will be used in v1Beta2 API version.

api/v1beta1/v1beta2_condition_consts.go

@@ -106,8 +106,8 @@ const (
 	// set to false and with the OwnerRemediated condition set to false by the MachineHealthCheck controller.
 	RemediatingV1Beta2Reason = "Remediating"
 
-	// NotRemediatingV1Beta2Reason surfaces when an object does not own any machines marked as not healthy
-	// by the MachineHealthCheck controller.
+	// NotRemediatingV1Beta2Reason surfaces when an object does not own any machines with HealthCheckSucceeded
+	// set to false and with the OwnerRemediated condition set to false by the MachineHealthCheck controller.
 	NotRemediatingV1Beta2Reason = "NotRemediating"
 
 	// NoReplicasV1Beta2Reason surfaces when an object that manage replicas does not have any.
@@ -142,15 +142,8 @@ const (
 	// PausedV1Beta2Reason surfaces when an object is paused.
 	PausedV1Beta2Reason = "Paused"
 
-	// RemoteConnectionFailedV1Beta2Reason surfaces that the remote connection failed.
-	// This is typically used when setting remote conditions (e.g. `NodeHealthy`) to `Unknown`
-	// after the remote connection probe didn't succeed for remote conditions grace period.
-	RemoteConnectionFailedV1Beta2Reason = "RemoteConnectionFailed"
-
-	// RemoteConnectionDownV1Beta2Reason surfaces that the remote connection is down.
-	// This is typically used when setting remote conditions (e.g. `NodeHealthy`) to `Unknown`
-	// when the connection is down and they haven't been set yet.
-	RemoteConnectionDownV1Beta2Reason = "RemoteConnectionDown"
+	// ConnectionDownV1Beta2Reason surfaces that the connection to the workload cluster is down.
+	ConnectionDownV1Beta2Reason = "ConnectionDown"
 
 	// DeletionTimestampNotSetV1Beta2Reason surfaces when an object is not deleting because the
 	// DeletionTimestamp is not set.

controlplane/kubeadm/api/v1beta1/v1beta2_condition_consts.go

@@ -52,6 +52,10 @@ const (
 	// etcd cluster hosted on KubeadmControlPlane controlled machines.
 	KubeadmControlPlaneEtcdClusterInspectionFailedV1Beta2Reason = clusterv1.InspectionFailedV1Beta2Reason
 
+	// KubeadmControlPlaneEtcdClusterConnectionDownV1Beta2Reason surfaces that the connection to the workload
+	// cluster is down.
+	KubeadmControlPlaneEtcdClusterConnectionDownV1Beta2Reason = clusterv1.ConnectionDownV1Beta2Reason
+
 	// KubeadmControlPlaneEtcdClusterHealthyV1Beta2Reason surfaces when the etcd cluster hosted on KubeadmControlPlane
 	// machines is healthy.
 	KubeadmControlPlaneEtcdClusterHealthyV1Beta2Reason = "Healthy"
@@ -77,6 +81,10 @@ const (
 	// control plane components hosted on KubeadmControlPlane controlled machines.
 	KubeadmControlPlaneControlPlaneComponentsInspectionFailedV1Beta2Reason = clusterv1.InspectionFailedV1Beta2Reason
 
+	// KubeadmControlPlaneControlPlaneComponentsConnectionDownV1Beta2Reason surfaces that the connection to the workload
+	// cluster is down.
+	KubeadmControlPlaneControlPlaneComponentsConnectionDownV1Beta2Reason = clusterv1.ConnectionDownV1Beta2Reason
+
 	// KubeadmControlPlaneControlPlaneComponentsHealthyV1Beta2Reason surfaces when the Kubernetes control plane components
 	// hosted on KubeadmControlPlane machines are healthy.
 	KubeadmControlPlaneControlPlaneComponentsHealthyV1Beta2Reason = "Healthy"
@@ -233,13 +241,13 @@ const (
 	// pod hosted on a KubeadmControlPlane controlled machine.
 	KubeadmControlPlaneMachinePodInspectionFailedV1Beta2Reason = clusterv1.InspectionFailedV1Beta2Reason
 
+	// KubeadmControlPlaneMachinePodConnectionDownV1Beta2Reason surfaces that the connection to the workload
+	// cluster is down.
+	KubeadmControlPlaneMachinePodConnectionDownV1Beta2Reason = clusterv1.ConnectionDownV1Beta2Reason
+
 	// KubeadmControlPlaneMachinePodDeletingV1Beta2Reason surfaces when the machine hosting control plane components
 	// is being deleted.
 	KubeadmControlPlaneMachinePodDeletingV1Beta2Reason = "Deleting"
-
-	// KubeadmControlPlaneMachinePodInternalErrorV1Beta2Reason surfaces unexpected failures when reading pod hosted
-	// on a KubeadmControlPlane controlled machine.
-	KubeadmControlPlaneMachinePodInternalErrorV1Beta2Reason = clusterv1.InternalErrorV1Beta2Reason
 )
 
 // EtcdMemberHealthy condition and corresponding reasons that will be used for KubeadmControlPlane controlled machines in v1Beta2 API version.
@@ -257,6 +265,10 @@ const (
 	// etcd member hosted on a KubeadmControlPlane controlled machine.
 	KubeadmControlPlaneMachineEtcdMemberInspectionFailedV1Beta2Reason = clusterv1.InspectionFailedV1Beta2Reason
 
+	// KubeadmControlPlaneMachineEtcdMemberConnectionDownV1Beta2Reason surfaces that the connection to the workload
+	// cluster is down.
+	KubeadmControlPlaneMachineEtcdMemberConnectionDownV1Beta2Reason = clusterv1.ConnectionDownV1Beta2Reason
+
 	// KubeadmControlPlaneMachineEtcdMemberDeletingV1Beta2Reason surfaces when the machine hosting an etcd member
 	// is being deleted.
 	KubeadmControlPlaneMachineEtcdMemberDeletingV1Beta2Reason = "Deleting"

controlplane/kubeadm/controllers/alias.go

@@ -40,6 +40,8 @@ type KubeadmControlPlaneReconciler struct {
 	// WatchFilterValue is the label value used to filter events prior to reconciliation.
 	WatchFilterValue string
 
+	RemoteConditionsGracePeriod time.Duration
+
 	// Deprecated: DeprecatedInfraMachineNaming. Name the InfraStructureMachines after the InfraMachineTemplate.
 	DeprecatedInfraMachineNaming bool
 }
@@ -53,6 +55,7 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
 		EtcdDialTimeout:              r.EtcdDialTimeout,
 		EtcdCallTimeout:              r.EtcdCallTimeout,
 		WatchFilterValue:             r.WatchFilterValue,
+		RemoteConditionsGracePeriod:  r.RemoteConditionsGracePeriod,
 		DeprecatedInfraMachineNaming: r.DeprecatedInfraMachineNaming,
 	}).SetupWithManager(ctx, mgr, options)
 }

controlplane/kubeadm/internal/controllers/controller.go

@@ -86,6 +86,8 @@ type KubeadmControlPlaneReconciler struct {
 	// WatchFilterValue is the label value used to filter events prior to reconciliation.
 	WatchFilterValue string
 
+	RemoteConditionsGracePeriod time.Duration
+
 	// Deprecated: DeprecatedInfraMachineNaming. Name the InfraStructureMachines after the InfraMachineTemplate.
 	DeprecatedInfraMachineNaming bool
 
@@ -95,6 +97,14 @@ type KubeadmControlPlaneReconciler struct {
 }
 
 func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error {
+	if r.Client == nil || r.SecretCachingClient == nil || r.ClusterCache == nil ||
+		r.EtcdDialTimeout == time.Duration(0) || r.EtcdCallTimeout == time.Duration(0) ||
+		r.RemoteConditionsGracePeriod < 2*time.Minute {
+		return errors.New("Client, SecretCachingClient and ClusterCache must not be nil and " +
+			"EtcdDialTimeout and EtcdCallTimeout must not be 0 and " +
+			"RemoteConditionsGracePeriod must not be < 2m")
+	}
+
 	predicateLog := ctrl.LoggerFrom(ctx).WithValues("controller", "kubeadmcontrolplane")
 	c, err := ctrl.NewControllerManagedBy(mgr).
 		For(&controlplanev1.KubeadmControlPlane{}).
@@ -111,7 +121,8 @@ func (r *KubeadmControlPlaneReconciler) SetupWithManager(ctx context.Context, mg
 				),
 			),
 		).
-		WatchesRawSource(r.ClusterCache.GetClusterSource("kubeadmcontrolplane", r.ClusterToKubeadmControlPlane)).
+		WatchesRawSource(r.ClusterCache.GetClusterSource("kubeadmcontrolplane", r.ClusterToKubeadmControlPlane,
+			clustercache.WatchForProbeFailure(r.RemoteConditionsGracePeriod))).
 		Build(r)
 	if err != nil {
 		return errors.Wrap(err, "failed setting up with a controller manager")
@@ -802,45 +813,169 @@ func (r *KubeadmControlPlaneReconciler) syncMachines(ctx context.Context, contro
 
 // reconcileControlPlaneConditions is responsible of reconciling conditions reporting the status of static pods and
 // the status of the etcd cluster.
-func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneConditions(ctx context.Context, controlPlane *internal.ControlPlane) error {
+func (r *KubeadmControlPlaneReconciler) reconcileControlPlaneConditions(ctx context.Context, controlPlane *internal.ControlPlane) (reterr error) {
 	// If the cluster is not yet initialized, there is no way to connect to the workload cluster and fetch information
 	// for updating conditions. Return early.
-	if !controlPlane.KCP.Status.Initialized {
+	// We additionally check for the Available condition. The Available condition is set at the same time
+	// as .status.initialized and is never changed to false again. Below we'll need the transition time of the
+	// Available condition to check if the remote conditions grace period is already reached.
+	// Note: The Machine controller uses the ControlPlaneInitialized condition on the Cluster instead for
+	// the same check. We don't use the ControlPlaneInitialized condition from the Cluster here because KCP
+	// Reconcile does (currently) not get triggered from condition changes to the Cluster object.
+	controlPlaneInitialized := conditions.Get(controlPlane.KCP, controlplanev1.AvailableCondition)
+	if !controlPlane.KCP.Status.Initialized ||
+		controlPlaneInitialized == nil || controlPlaneInitialized.Status != corev1.ConditionTrue {
 		v1beta2conditions.Set(controlPlane.KCP, metav1.Condition{
 			Type:    controlplanev1.KubeadmControlPlaneEtcdClusterHealthyV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
 			Reason:  controlplanev1.KubeadmControlPlaneEtcdClusterInspectionFailedV1Beta2Reason,
-			Message: "Waiting for remote connection",
+			Message: "Waiting for Cluster control plane to be initialized",
 		})
 
 		v1beta2conditions.Set(controlPlane.KCP, metav1.Condition{
 			Type:    controlplanev1.KubeadmControlPlaneControlPlaneComponentsHealthyV1Beta2Condition,
 			Status:  metav1.ConditionUnknown,
 			Reason:  controlplanev1.KubeadmControlPlaneControlPlaneComponentsInspectionFailedV1Beta2Reason,
-			Message: "Waiting for remote connection",
+			Message: "Waiting for Cluster control plane to be initialized",
 		})
 
 		return nil
 	}
 
+	defer func() {
+		// Patch machines with the updated conditions.
+		reterr = kerrors.NewAggregate([]error{reterr, controlPlane.PatchMachines(ctx)})
+	}()
+
+	// Remote conditions grace period is counted from the later of last probe success and control plane initialized.
+	lastProbeSuccessTime := r.ClusterCache.GetLastProbeSuccessTimestamp(ctx, client.ObjectKeyFromObject(controlPlane.Cluster))
+	if time.Since(maxTime(lastProbeSuccessTime, controlPlaneInitialized.LastTransitionTime.Time)) > r.RemoteConditionsGracePeriod {
+		// Overwrite conditions to ConnectionDown.
+		setConditionsToUnknown(setConditionsToUnknownInput{
+			ControlPlane:                        controlPlane,
+			Overwrite:                           true,
+			EtcdClusterHealthyReason:            controlplanev1.KubeadmControlPlaneEtcdClusterConnectionDownV1Beta2Reason,
+			ControlPlaneComponentsHealthyReason: controlplanev1.KubeadmControlPlaneControlPlaneComponentsConnectionDownV1Beta2Reason,
+			StaticPodReason:                     controlplanev1.KubeadmControlPlaneMachinePodConnectionDownV1Beta2Reason,
+			EtcdMemberHealthyReason:             controlplanev1.KubeadmControlPlaneMachineEtcdMemberConnectionDownV1Beta2Reason,
+			Message:                             lastProbeSuccessMessage(lastProbeSuccessTime),
+		})
+		return errors.Errorf("connection to the workload cluster is down")
+	}
+
 	workloadCluster, err := controlPlane.GetWorkloadCluster(ctx)
 	if err != nil {
-		return errors.Wrap(err, "cannot get remote client to workload cluster")
+		if errors.Is(err, clustercache.ErrClusterNotConnected) {
+			// If conditions are not set, set them to ConnectionDown.
+			setConditionsToUnknown(setConditionsToUnknownInput{
+				ControlPlane:                        controlPlane,
+				Overwrite:                           false, // Don't overwrite.
+				EtcdClusterHealthyReason:            controlplanev1.KubeadmControlPlaneEtcdClusterConnectionDownV1Beta2Reason,
+				ControlPlaneComponentsHealthyReason: controlplanev1.KubeadmControlPlaneControlPlaneComponentsConnectionDownV1Beta2Reason,
+				StaticPodReason:                     controlplanev1.KubeadmControlPlaneMachinePodConnectionDownV1Beta2Reason,
+				EtcdMemberHealthyReason:             controlplanev1.KubeadmControlPlaneMachineEtcdMemberConnectionDownV1Beta2Reason,
+				Message:                             lastProbeSuccessMessage(lastProbeSuccessTime),
+			})
+			return errors.Wrap(err, "cannot get client for the workload cluster")
+		}
+
+		// Overwrite conditions to InspectionFailed.
+		setConditionsToUnknown(setConditionsToUnknownInput{
+			ControlPlane:                        controlPlane,
+			Overwrite:                           true,
+			EtcdClusterHealthyReason:            controlplanev1.KubeadmControlPlaneEtcdClusterInspectionFailedV1Beta2Reason,
+			ControlPlaneComponentsHealthyReason: controlplanev1.KubeadmControlPlaneControlPlaneComponentsInspectionFailedV1Beta2Reason,
+			StaticPodReason:                     controlplanev1.KubeadmControlPlaneMachinePodInspectionFailedV1Beta2Reason,
+			EtcdMemberHealthyReason:             controlplanev1.KubeadmControlPlaneMachineEtcdMemberInspectionFailedV1Beta2Reason,
+			Message:                             "Please check controller logs for errors",
+		})
+		return errors.Wrap(err, "cannot get client for the workload cluster")
 	}
 
 	// Update conditions status
 	workloadCluster.UpdateStaticPodConditions(ctx, controlPlane)
 	workloadCluster.UpdateEtcdConditions(ctx, controlPlane)
 
-	// Patch machines with the updated conditions.
-	if err := controlPlane.PatchMachines(ctx); err != nil {
-		return err
-	}
-
 	// KCP will be patched at the end of Reconcile to reflect updated conditions, so we can return now.
 	return nil
 }
 
+type setConditionsToUnknownInput struct {
+	ControlPlane                        *internal.ControlPlane
+	Overwrite                           bool
+	EtcdClusterHealthyReason            string
+	ControlPlaneComponentsHealthyReason string
+	StaticPodReason                     string
+	EtcdMemberHealthyReason             string
+	Message                             string
+}
+
+func setConditionsToUnknown(input setConditionsToUnknownInput) {
+	etcdClusterHealthySet := v1beta2conditions.Has(input.ControlPlane.KCP, controlplanev1.KubeadmControlPlaneEtcdClusterHealthyV1Beta2Condition)
+	controlPlaneComponentsHealthySet := v1beta2conditions.Has(input.ControlPlane.KCP, controlplanev1.KubeadmControlPlaneControlPlaneComponentsHealthyV1Beta2Condition)
+
+	if input.Overwrite || !etcdClusterHealthySet {
+		v1beta2conditions.Set(input.ControlPlane.KCP, metav1.Condition{
+			Type:    controlplanev1.KubeadmControlPlaneEtcdClusterHealthyV1Beta2Condition,
+			Status:  metav1.ConditionUnknown,
+			Reason:  input.EtcdClusterHealthyReason,
+			Message: input.Message,
+		})
+		for _, machine := range input.ControlPlane.Machines {
+			if input.ControlPlane.IsEtcdManaged() {
+				v1beta2conditions.Set(machine, metav1.Condition{
+					Type:    controlplanev1.KubeadmControlPlaneMachineEtcdMemberHealthyV1Beta2Condition,
+					Status:  metav1.ConditionUnknown,
+					Reason:  input.EtcdMemberHealthyReason,
+					Message: input.Message,
+				})
+			}
+		}
+	}
+
+	if input.Overwrite || !controlPlaneComponentsHealthySet {
+		v1beta2conditions.Set(input.ControlPlane.KCP, metav1.Condition{
+			Type:    controlplanev1.KubeadmControlPlaneControlPlaneComponentsHealthyV1Beta2Condition,
+			Status:  metav1.ConditionUnknown,
+			Reason:  input.ControlPlaneComponentsHealthyReason,
+			Message: input.Message,
+		})
+
+		allMachinePodV1beta2Conditions := []string{
+			controlplanev1.KubeadmControlPlaneMachineAPIServerPodHealthyV1Beta2Condition,
+			controlplanev1.KubeadmControlPlaneMachineControllerManagerPodHealthyV1Beta2Condition,
+			controlplanev1.KubeadmControlPlaneMachineSchedulerPodHealthyV1Beta2Condition,
+		}
+		if input.ControlPlane.IsEtcdManaged() {
+			allMachinePodV1beta2Conditions = append(allMachinePodV1beta2Conditions, controlplanev1.KubeadmControlPlaneMachineEtcdPodHealthyV1Beta2Condition)
+		}
+		for _, machine := range input.ControlPlane.Machines {
+			for _, condition := range allMachinePodV1beta2Conditions {
+				v1beta2conditions.Set(machine, metav1.Condition{
+					Type:    condition,
+					Status:  metav1.ConditionUnknown,
+					Reason:  input.StaticPodReason,
+					Message: input.Message,
+				})
+			}
+		}
+	}
+}
+
+func lastProbeSuccessMessage(lastProbeSuccessTime time.Time) string {
+	if lastProbeSuccessTime.IsZero() {
+		return ""
+	}
+	return fmt.Sprintf("Last successful probe at %s", lastProbeSuccessTime.Format(time.RFC3339))
+}
+
+func maxTime(t1, t2 time.Time) time.Time {
+	if t1.After(t2) {
+		return t1
+	}
+	return t2
+}
+
 // reconcileEtcdMembers ensures the number of etcd members is in sync with the number of machines/nodes.
 // This is usually required after a machine deletion.
 //