From 51e4fab73a553b4ca5ff79b5e722c73673312f3c Mon Sep 17 00:00:00 2001 From: nasusoba Date: Wed, 29 May 2024 16:21:00 +0800 Subject: [PATCH] add version contract Signed-off-by: nasusoba --- controlplane/api/v1beta1/conversion.go | 5 ++++ .../api/v1beta1/zz_generated.conversion.go | 16 +++++------- .../api/v1beta2/kthreescontrolplane_types.go | 5 ++++ .../api/v1beta2/zz_generated.deepcopy.go | 5 ++++ ...cluster.x-k8s.io_kthreescontrolplanes.yaml | 5 ++++ .../kthreescontrolplane_controller.go | 25 ++++++++++++++++--- pkg/machinefilters/machine_filters.go | 12 +++++++++ 7 files changed, 59 insertions(+), 14 deletions(-) diff --git a/controlplane/api/v1beta1/conversion.go b/controlplane/api/v1beta1/conversion.go index b7e9ad76..2c1fa0ec 100644 --- a/controlplane/api/v1beta1/conversion.go +++ b/controlplane/api/v1beta1/conversion.go @@ -71,6 +71,10 @@ func Convert_v1beta2_KThreesConfigSpec_To_v1beta1_KThreesConfigSpec(in *bootstra return bootstrapv1beta1.Convert_v1beta2_KThreesConfigSpec_To_v1beta1_KThreesConfigSpec(in, out, s) } +func Convert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(in *controlplanev1beta2.KThreesControlPlaneStatus, out *KThreesControlPlaneStatus, s conversion.Scope) error { //nolint: stylecheck + return autoConvert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(in, out, s) +} + // ConvertTo converts the v1beta1 KThreesControlPlane receiver to a v1beta2 KThreesControlPlane. func (in *KThreesControlPlane) ConvertTo(dstRaw ctrlconversion.Hub) error { dst := dstRaw.(*controlplanev1beta2.KThreesControlPlane) @@ -91,6 +95,7 @@ func (in *KThreesControlPlane) ConvertTo(dstRaw ctrlconversion.Hub) error { dst.Spec.KThreesConfigSpec.ServerConfig.DisableCloudController = restored.Spec.KThreesConfigSpec.ServerConfig.DisableCloudController dst.Spec.MachineTemplate.NodeVolumeDetachTimeout = restored.Spec.MachineTemplate.NodeVolumeDetachTimeout dst.Spec.MachineTemplate.NodeDeletionTimeout = restored.Spec.MachineTemplate.NodeDeletionTimeout + dst.Status.Version = restored.Status.Version return nil } diff --git a/controlplane/api/v1beta1/zz_generated.conversion.go b/controlplane/api/v1beta1/zz_generated.conversion.go index 5f479db9..5dc7e220 100644 --- a/controlplane/api/v1beta1/zz_generated.conversion.go +++ b/controlplane/api/v1beta1/zz_generated.conversion.go @@ -70,11 +70,6 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } - if err := s.AddGeneratedConversionFunc((*v1beta2.KThreesControlPlaneStatus)(nil), (*KThreesControlPlaneStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { - return Convert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(a.(*v1beta2.KThreesControlPlaneStatus), b.(*KThreesControlPlaneStatus), scope) - }); err != nil { - return err - } if err := s.AddGeneratedConversionFunc((*LastRemediationStatus)(nil), (*v1beta2.LastRemediationStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { return Convert_v1beta1_LastRemediationStatus_To_v1beta2_LastRemediationStatus(a.(*LastRemediationStatus), b.(*v1beta2.LastRemediationStatus), scope) }); err != nil { @@ -120,6 +115,11 @@ func RegisterConversions(s *runtime.Scheme) error { }); err != nil { return err } + if err := s.AddConversionFunc((*v1beta2.KThreesControlPlaneStatus)(nil), (*KThreesControlPlaneStatus)(nil), func(a, b interface{}, scope conversion.Scope) error { + return Convert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(a.(*v1beta2.KThreesControlPlaneStatus), b.(*KThreesControlPlaneStatus), scope) + }); err != nil { + return err + } return nil } @@ -270,6 +270,7 @@ func Convert_v1beta1_KThreesControlPlaneStatus_To_v1beta2_KThreesControlPlaneSta func autoConvert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(in *v1beta2.KThreesControlPlaneStatus, out *KThreesControlPlaneStatus, s conversion.Scope) error { out.Selector = in.Selector out.Replicas = in.Replicas + // WARNING: in.Version requires manual conversion: does not exist in peer-type out.UpdatedReplicas = in.UpdatedReplicas out.ReadyReplicas = in.ReadyReplicas out.UnavailableReplicas = in.UnavailableReplicas @@ -283,11 +284,6 @@ func autoConvert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlan return nil } -// Convert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus is an autogenerated conversion function. -func Convert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(in *v1beta2.KThreesControlPlaneStatus, out *KThreesControlPlaneStatus, s conversion.Scope) error { - return autoConvert_v1beta2_KThreesControlPlaneStatus_To_v1beta1_KThreesControlPlaneStatus(in, out, s) -} - func autoConvert_v1beta1_LastRemediationStatus_To_v1beta2_LastRemediationStatus(in *LastRemediationStatus, out *v1beta2.LastRemediationStatus, s conversion.Scope) error { out.Machine = in.Machine out.Timestamp = in.Timestamp diff --git a/controlplane/api/v1beta2/kthreescontrolplane_types.go b/controlplane/api/v1beta2/kthreescontrolplane_types.go index cab51471..78ce9f88 100644 --- a/controlplane/api/v1beta2/kthreescontrolplane_types.go +++ b/controlplane/api/v1beta2/kthreescontrolplane_types.go @@ -171,6 +171,11 @@ type KThreesControlPlaneStatus struct { // +optional Replicas int32 `json:"replicas,omitempty"` + // Version represents the minimum Kubernetes version for the control plane machines + // in the cluster. + // +optional + Version *string `json:"version,omitempty"` + // Total number of non-terminated machines targeted by this control plane // that have the desired template spec. // +optional diff --git a/controlplane/api/v1beta2/zz_generated.deepcopy.go b/controlplane/api/v1beta2/zz_generated.deepcopy.go index 078d9630..fd5048f7 100644 --- a/controlplane/api/v1beta2/zz_generated.deepcopy.go +++ b/controlplane/api/v1beta2/zz_generated.deepcopy.go @@ -151,6 +151,11 @@ func (in *KThreesControlPlaneSpec) DeepCopy() *KThreesControlPlaneSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *KThreesControlPlaneStatus) DeepCopyInto(out *KThreesControlPlaneStatus) { *out = *in + if in.Version != nil { + in, out := &in.Version, &out.Version + *out = new(string) + **out = **in + } if in.FailureMessage != nil { in, out := &in.FailureMessage, &out.FailureMessage *out = new(string) diff --git a/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_kthreescontrolplanes.yaml b/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_kthreescontrolplanes.yaml index 7b4392e6..c0488d0a 100644 --- a/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_kthreescontrolplanes.yaml +++ b/controlplane/config/crd/bases/controlplane.cluster.x-k8s.io_kthreescontrolplanes.yaml @@ -1111,6 +1111,11 @@ spec: that have the desired template spec. format: int32 type: integer + version: + description: |- + Version represents the minimum Kubernetes version for the control plane machines + in the cluster. + type: string type: object type: object served: true diff --git a/controlplane/controllers/kthreescontrolplane_controller.go b/controlplane/controllers/kthreescontrolplane_controller.go index 18795670..3fce3d37 100644 --- a/controlplane/controllers/kthreescontrolplane_controller.go +++ b/controlplane/controllers/kthreescontrolplane_controller.go @@ -49,6 +49,7 @@ import ( controlplanev1 "github.com/k3s-io/cluster-api-k3s/controlplane/api/v1beta2" k3s "github.com/k3s-io/cluster-api-k3s/pkg/k3s" "github.com/k3s-io/cluster-api-k3s/pkg/kubeconfig" + "github.com/k3s-io/cluster-api-k3s/pkg/machinefilters" "github.com/k3s-io/cluster-api-k3s/pkg/secret" "github.com/k3s-io/cluster-api-k3s/pkg/token" ) @@ -159,13 +160,23 @@ func (r *KThreesControlPlaneReconciler) Reconcile(ctx context.Context, req ctrl. err = kerrors.NewAggregate([]error{err, patchErr}) } - // TODO: remove this as soon as we have a proper remote cluster cache in place. - // Make KCP to requeue in case status is not ready, so we can check for node status without waiting for a full resync (by default 10 minutes). - // Only requeue if we are not going in exponential backoff due to error, or if we are not already re-queueing, or if the object has a deletion timestamp. - if err == nil && !res.Requeue && !(res.RequeueAfter > 0) && kcp.ObjectMeta.DeletionTimestamp.IsZero() { + // Only requeue if there is no error, Requeue or RequeueAfter and the object does not have a deletion timestamp. + if err == nil && res.IsZero() && kcp.ObjectMeta.DeletionTimestamp.IsZero() { + // Make KCP requeue in case node status is not ready, so we can check for node status without waiting for a full + // resync (by default 10 minutes). + // The alternative solution would be to watch the control plane nodes in the Cluster - similar to how the + // MachineSet and MachineHealthCheck controllers watch the nodes under their control. if !kcp.Status.Ready { res = ctrl.Result{RequeueAfter: 20 * time.Second} } + + // Make KCP requeue if ControlPlaneComponentsHealthyCondition is false so we can check for control plane component + // status without waiting for a full resync (by default 10 minutes). + // Otherwise this condition can lead to a delay in provisioning MachineDeployments when MachineSet preflight checks are enabled. + // The alternative solution to this requeue would be watching the relevant pods inside each workload cluster which would be very expensive. + if conditions.IsFalse(kcp, controlplanev1.ControlPlaneComponentsHealthyCondition) { + res = ctrl.Result{RequeueAfter: 20 * time.Second} + } } return res, err @@ -364,6 +375,12 @@ func (r *KThreesControlPlaneReconciler) updateStatus(ctx context.Context, kcp *c return nil } + machinesWithAgentHealthy := controlPlane.Machines.Filter(machinefilters.AgentHealthy()) + lowestVersion := machinesWithAgentHealthy.LowestVersion() + if lowestVersion != nil { + controlPlane.KCP.Status.Version = lowestVersion + } + switch { // We are scaling up case replicas < desiredReplicas: diff --git a/pkg/machinefilters/machine_filters.go b/pkg/machinefilters/machine_filters.go index 2e069a30..53853186 100644 --- a/pkg/machinefilters/machine_filters.go +++ b/pkg/machinefilters/machine_filters.go @@ -22,6 +22,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/collections" + "sigs.k8s.io/cluster-api/util/conditions" bootstrapv1 "github.com/k3s-io/cluster-api-k3s/bootstrap/api/v1beta2" controlplanev1 "github.com/k3s-io/cluster-api-k3s/controlplane/api/v1beta2" @@ -112,3 +113,14 @@ func MatchesKThreesBootstrapConfig(machineConfigs map[string]*bootstrapv1.KThree return reflect.DeepEqual(&machineConfig.Spec, kcpConfig) } } + +// AgentHealthy returns a filter to find all machines that have an AgentHealthy +// set to true. +func AgentHealthy() Func { + return func(machine *clusterv1.Machine) bool { + if machine == nil { + return false + } + return conditions.IsTrue(machine, controlplanev1.MachineAgentHealthyCondition) + } +}