Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add finalizer to preventing HPA from being deleted before replicas synced #4084

Closed
wants to merge 1 commit into from
Closed

add finalizer to preventing HPA from being deleted before replicas synced #4084

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 107 additions & 21 deletions pkg/controllers/hpareplicassyncer/hpa_replicas_syncer_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/scale"
"k8s.io/client-go/util/retry"
"k8s.io/klog/v2"
controllerruntime "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/builder"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

"github.com/karmada-io/karmada/pkg/util"
)
Expand Down Expand Up @@ -72,13 +75,27 @@ func (r *HPAReplicasSyncer) Reconcile(ctx context.Context, req controllerruntime
return controllerruntime.Result{}, err
}

err = r.updateScaleIfNeed(ctx, workloadGR, scale.DeepCopy(), hpa)
synced := isReplicasSynced(scale, hpa)

if !hpa.DeletionTimestamp.IsZero() && synced {
return controllerruntime.Result{}, r.removeFinalizerIfNeed(ctx, hpa)
}

err = r.addFinalizerIfNeed(ctx, hpa)
if err != nil {
return controllerruntime.Result{}, err
}

// TODO(@lxtywypc): Add finalizer for HPA and remove them
// when the HPA is deleting and the replicas have been synced.
if !synced {
err = r.updateScale(ctx, workloadGR, scale, hpa)
if err != nil {
return controllerruntime.Result{}, err
}

// If it needs to sync, we had better to requeue once immediately to make sure
// the syncing successfully and the finalizer removed successfully.
return controllerruntime.Result{Requeue: true}, nil
}

return controllerruntime.Result{}, nil
}
Expand All @@ -102,6 +119,9 @@ func (r *HPAReplicasSyncer) getGroupResourceAndScaleForWorkloadFromHPA(ctx conte
if err != nil {
if apierrors.IsNotFound(err) {
// If the scale of workload is not found, skip processing.
klog.V(4).Infof("Scale of resource(kind=%s, %s/%s) not found, the resource might have been removed",
hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name)

return gr, nil, nil
}

Expand All @@ -114,32 +134,98 @@ func (r *HPAReplicasSyncer) getGroupResourceAndScaleForWorkloadFromHPA(ctx conte
return gr, scale, nil
}

// updateScaleIfNeed would update the scale of workload on fed-control plane
// if the replicas declared in the workload on karmada-control-plane does not match
// the actual replicas in member clusters effected by HPA.
func (r *HPAReplicasSyncer) updateScaleIfNeed(ctx context.Context, workloadGR schema.GroupResource, scale *autoscalingv1.Scale, hpa *autoscalingv2.HorizontalPodAutoscaler) error {
// If the scale of workload is not found, skip processing.
if scale == nil {
klog.V(4).Infof("Scale of resource(kind=%s, %s/%s) not found, the resource might have been removed, skip",
hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name)
// updateScale would update the scale of workload on fed-control plane
// with the actual replicas in member clusters effected by HPA.
func (r *HPAReplicasSyncer) updateScale(ctx context.Context, workloadGR schema.GroupResource, scale *autoscalingv1.Scale, hpa *autoscalingv2.HorizontalPodAutoscaler) error {
scaleCopy := scale.DeepCopy()

scaleCopy.Spec.Replicas = hpa.Status.DesiredReplicas
_, err := r.ScaleClient.Scales(hpa.Namespace).Update(ctx, workloadGR, scaleCopy, metav1.UpdateOptions{})
if err != nil {
klog.Errorf("Failed to try to sync scale for resource(kind=%s, %s/%s) from %d to %d: %v",
hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name, scale.Spec.Replicas, hpa.Status.CurrentReplicas, err)

return err
}

klog.V(4).Infof("Successfully synced scale for resource(kind=%s, %s/%s) from %d to %d",
hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name, scale.Spec.Replicas, hpa.Status.CurrentReplicas)

return nil
}

// removeFinalizerIfNeed tries to remove HPAReplicasSyncerFinalizer from HPA if the finalizer exists.
func (r *HPAReplicasSyncer) removeFinalizerIfNeed(ctx context.Context, hpa *autoscalingv2.HorizontalPodAutoscaler) error {
hpaCopy := hpa.DeepCopy()

err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
updated := controllerutil.RemoveFinalizer(hpaCopy, util.HPAReplicasSyncerFinalizer)
if !updated {
// If the hpa does not have the finalizer, return immediately without error.
return nil
}

updateErr := r.Client.Update(ctx, hpaCopy)
if updateErr == nil {
klog.V(4).Infof("Remove HPAReplicasSyncerFinalizer from horizontalpodautoscaler %s/%s successfully", hpa.Namespace, hpa.Name)

return nil
return nil
}

err := r.Client.Get(ctx, types.NamespacedName{Namespace: hpa.Namespace, Name: hpa.Name}, hpaCopy)
if err != nil {
return err
}

return updateErr
})
if err != nil {
klog.Errorf("Failed to remove HPAReplicasSyncerFinalizer from horizontalpodautoscaler %s/%s: %v", hpa.Namespace, hpa.Name, err)
}

if scale.Spec.Replicas != hpa.Status.DesiredReplicas {
oldReplicas := scale.Spec.Replicas
return err
}

// addFinalizerIfNeed tries to add HPAReplicasSyncerFinalizer to HPA if the finalizer does not exist.
func (r *HPAReplicasSyncer) addFinalizerIfNeed(ctx context.Context, hpa *autoscalingv2.HorizontalPodAutoscaler) error {
hpaCopy := hpa.DeepCopy()

err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
updated := controllerutil.AddFinalizer(hpaCopy, util.HPAReplicasSyncerFinalizer)
if !updated {
// If the hpa has had the finalizer, return immediately without error.
return nil
}

updateErr := r.Client.Update(ctx, hpaCopy)
if updateErr == nil {
klog.V(4).Infof("Add HPAReplicasSyncerFinalizer to horizontalpodautoscaler %s/%s successfully", hpa.Namespace, hpa.Name)

scale.Spec.Replicas = hpa.Status.DesiredReplicas
_, err := r.ScaleClient.Scales(hpa.Namespace).Update(ctx, workloadGR, scale, metav1.UpdateOptions{})
return nil
}

err := r.Client.Get(ctx, types.NamespacedName{Namespace: hpa.Namespace, Name: hpa.Name}, hpaCopy)
if err != nil {
klog.Errorf("Failed to try to sync scale for resource(kind=%s, %s/%s) from %d to %d: %v",
hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name, oldReplicas, hpa.Status.DesiredReplicas, err)
return err
}

klog.V(4).Infof("Successfully synced scale for resource(kind=%s, %s/%s) from %d to %d",
hpa.Spec.ScaleTargetRef.Kind, hpa.Namespace, hpa.Spec.ScaleTargetRef.Name, oldReplicas, hpa.Status.DesiredReplicas)
return updateErr
})
if err != nil {
klog.Errorf("Failed to add HPAReplicasSyncerFinalizer to horizontalpodautoscaler %s/%s: %v", hpa.Namespace, hpa.Name, err)
}

return err
}

// isReplicasSynced judges if the replicas of workload scale has been synced successfully.
func isReplicasSynced(scale *autoscalingv1.Scale, hpa *autoscalingv2.HorizontalPodAutoscaler) bool {
if scale == nil || hpa == nil {
// If the scale not found, the workload might has been removed;
// if the hpa not found, the workload is not managed by hpa;
// we treat these as replicas synced.
return true
}

return nil
return scale.Spec.Replicas == hpa.Status.DesiredReplicas
}
Loading