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.

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

refactor: use Update instead of Patch to delete annotation #60

Merged
merged 1 commit into from
Mar 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -154,5 +154,6 @@ replace (
k8s.io/pod-security-admission => k8s.io/pod-security-admission v0.22.2
k8s.io/sample-apiserver => k8s.io/sample-apiserver v0.22.2
k8s.io/system-validators => k8s.io/system-validators v1.5.0
k8s.io/utils => k8s.io/utils v0.0.0-20240102154912-e7106e64919e
sigs.k8s.io/controller-runtime => sigs.k8s.io/controller-runtime v0.10.3
)
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,6 @@ github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4k
github.com/soheilhy/cmux v0.1.5/go.mod h1:T7TcVDs9LWfQgPlPsdngu6I6QIoyIFZDDC6sNE1GqG0=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ=
github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTdifk=
github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
github.com/spf13/cobra v1.1.3/go.mod h1:pGADOWyqRD/YMrPZigI/zbliZ2wVD/23d+is3pSWzOo=
Expand Down Expand Up @@ -1034,8 +1033,6 @@ k8s.io/mount-utils v0.22.4-rc.0/go.mod h1:dHl6c2P60T5LHUnZxVslyly9EDCMzvhtISO5aY
k8s.io/pod-security-admission v0.22.2/go.mod h1:5FK/TIw6rySU522cZVueMcS/LPPovNHbsm1I1gLfVfU=
k8s.io/sample-apiserver v0.22.2/go.mod h1:h+/DIV5EmuNq4vfPr5TSXy9mIBVXXlPAKQMPbjPrlFM=
k8s.io/system-validators v1.5.0/go.mod h1:bPldcLgkIUK22ALflnsXk8pvkTEndYdNuaHH6gRrl0Q=
k8s.io/utils v0.0.0-20201110183641-67b214c5f920/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
k8s.io/utils v0.0.0-20210819203725-bdf08cb9a70a/go.mod h1:jPW/WVKK9YHAvNhRxK0md/EJ228hCsBRufyofKtW8HA=
k8s.io/utils v0.0.0-20240102154912-e7106e64919e h1:eQ/4ljkx21sObifjzXwlPKpdGLrCfRziVtos3ofG/sQ=
k8s.io/utils v0.0.0-20240102154912-e7106e64919e/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
kusionstack.io/kube-api v0.0.27 h1:7umLyoMmOdse0A8nXGccNpOdYu7B5sNPLoPxc/hmBIY=
Expand Down
3 changes: 0 additions & 3 deletions pkg/controllers/rolloutrun/executor/do_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,5 @@ func (r *Executor) doCommand(ctx *ExecutorContext) ctrl.Result {
}
}

// Regardless of the value, we need to delete the key.
delete(rolloutRun.Annotations, rolloutapis.AnnoManualCommandKey)

return ctrl.Result{Requeue: true}
}
45 changes: 27 additions & 18 deletions pkg/controllers/rolloutrun/rolloutrun_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,23 +129,18 @@ func (r *RolloutRunReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}

var result ctrl.Result
cmd := obj.Annotations[rollout.AnnoManualCommandKey]
result, err = r.syncRolloutRun(ctx, obj, newStatus, workloads)

if tempErr := r.cleanupAnnotation(ctx, obj); tempErr != nil {
logger.Error(tempErr, "failed to clean up annotation")
}

updateStatus := r.updateStatusOnly(ctx, obj, newStatus, workloads)
if updateStatus != nil {
logger.Error(err, "failed to update status")
logger.Error(updateStatus, "failed to update status")
return reconcile.Result{}, updateStatus
}

if _, exist := obj.Annotations[rollout.AnnoManualCommandKey]; !exist && len(cmd) > 0 {
patch := utils.DeleteAnnotation(types.JSONPatchType, rollout.AnnoManualCommandKey)
if patchError := r.Client.Patch(clusterinfo.ContextFed, obj, patch); patchError != nil {
logger.Error(err, "failed to patch status")
return reconcile.Result{}, patchError
}
}

if err != nil {
return reconcile.Result{}, err
}
Expand Down Expand Up @@ -183,6 +178,20 @@ func (r *RolloutRunReconciler) handleFinalizer(rolloutRun *rolloutv1alpha1.Rollo
return nil
}

func (r *RolloutRunReconciler) cleanupAnnotation(ctx context.Context, obj *rolloutv1alpha1.RolloutRun) error {
// delete manual command annotations from rollout
_, err := utils.UpdateOnConflict(clusterinfo.WithCluster(ctx, clusterinfo.Fed), r.Client, r.Client, obj, func() error {
delete(obj.Annotations, rollout.AnnoManualCommandKey)
return nil
})
if err != nil {
return err
}
key := utils.ObjectKeyString(obj)
r.rvExpectation.ExpectUpdate(key, obj.ResourceVersion) // nolint
return nil
}

func (r *RolloutRunReconciler) findTrafficTopology(ctx context.Context, obj *rolloutv1alpha1.RolloutRun) ([]rolloutv1alpha1.TrafficTopology, error) {
topologies := make([]rolloutv1alpha1.TrafficTopology, 0)
for _, name := range obj.Spec.TrafficTopologyRefs {
Expand Down Expand Up @@ -335,29 +344,29 @@ func (r *RolloutRunReconciler) syncWorkloadStatus(newStatus *rolloutv1alpha1.Rol
newStatus.TargetStatuses = workloadStatuses
}

func (r *RolloutRunReconciler) updateStatusOnly(ctx context.Context, instance *rolloutv1alpha1.RolloutRun, newStatus *rolloutv1alpha1.RolloutRunStatus, workloads *workload.Set) error {
func (r *RolloutRunReconciler) updateStatusOnly(ctx context.Context, obj *rolloutv1alpha1.RolloutRun, newStatus *rolloutv1alpha1.RolloutRunStatus, workloads *workload.Set) error {
// generate workload status
r.syncWorkloadStatus(newStatus, workloads)

if equality.Semantic.DeepEqual(instance.Status, *newStatus) {
if equality.Semantic.DeepEqual(obj.Status, *newStatus) {
// no change
return nil
}
key := utils.ObjectKeyString(instance)
key := utils.ObjectKeyString(obj)
now := metav1.Now()
newStatus.LastUpdateTime = &now
_, err := utils.UpdateOnConflict(clusterinfo.WithCluster(ctx, clusterinfo.Fed), r.Client, r.Client.Status(), instance, func() error {
instance.Status = *newStatus
instance.Status.ObservedGeneration = instance.Generation
_, err := utils.UpdateOnConflict(clusterinfo.WithCluster(ctx, clusterinfo.Fed), r.Client, r.Client.Status(), obj, func() error {
obj.Status = *newStatus
obj.Status.ObservedGeneration = obj.Generation
return nil
})
if err != nil {
r.Recorder.Eventf(instance, corev1.EventTypeWarning, "FailedUpdateStatus", "failed to update rolloutRun %q status: %v", key, err)
r.Recorder.Eventf(obj, corev1.EventTypeWarning, "FailedUpdateStatus", "failed to update rolloutRun %q status: %v", key, err)
r.Logger.Error(err, "failed to update rolloutRun status", "rolloutRun", key)
return err
}

r.Logger.V(2).Info("succeed to update rolloutRun status", "rolloutRun", key)
r.rvExpectation.ExpectUpdate(key, instance.ResourceVersion) // nolint
r.rvExpectation.ExpectUpdate(key, obj.ResourceVersion) // nolint
return nil
}
14 changes: 0 additions & 14 deletions pkg/utils/patch.go

This file was deleted.