From d6046aad7f88614764f859127a13005c2ee4f9db Mon Sep 17 00:00:00 2001 From: Kuromesi Date: Tue, 12 Sep 2023 22:57:15 +0800 Subject: [PATCH] remove roll back Signed-off-by: Kuromesi --- .../trafficrouting_controller.go | 5 +- pkg/trafficrouting/network/custom/custom.go | 74 +++---------------- 2 files changed, 12 insertions(+), 67 deletions(-) diff --git a/pkg/controller/trafficrouting/trafficrouting_controller.go b/pkg/controller/trafficrouting/trafficrouting_controller.go index eb62d3c2..441525b9 100644 --- a/pkg/controller/trafficrouting/trafficrouting_controller.go +++ b/pkg/controller/trafficrouting/trafficrouting_controller.go @@ -97,6 +97,7 @@ func (r *TrafficRoutingReconciler) Reconcile(ctx context.Context, req ctrl.Reque newStatus := tr.Status.DeepCopy() if newStatus.Phase == "" { newStatus.Phase = v1alpha1.TrafficRoutingPhaseInitial + newStatus.Message = "TrafficRouting is Initializing" } if !tr.DeletionTimestamp.IsZero() { newStatus.Phase = v1alpha1.TrafficRoutingPhaseTerminating @@ -124,8 +125,8 @@ func (r *TrafficRoutingReconciler) Reconcile(ctx context.Context, req ctrl.Reque case v1alpha1.TrafficRoutingPhaseFinalizing: done, err = r.trafficRoutingManager.FinalisingTrafficRouting(newTrafficRoutingContext(tr), false) if done { - newStatus.Phase = v1alpha1.TrafficRoutingPhaseHealthy - newStatus.Message = "TrafficRouting is Healthy" + newStatus.Phase = v1alpha1.TrafficRoutingPhaseInitial + newStatus.Message = "TrafficRouting is Initializing" } case v1alpha1.TrafficRoutingPhaseTerminating: done, err = r.trafficRoutingManager.FinalisingTrafficRouting(newTrafficRoutingContext(tr), false) diff --git a/pkg/trafficrouting/network/custom/custom.go b/pkg/trafficrouting/network/custom/custom.go index dfeb90b0..849a4cbf 100644 --- a/pkg/trafficrouting/network/custom/custom.go +++ b/pkg/trafficrouting/network/custom/custom.go @@ -86,7 +86,6 @@ func NewCustomController(client client.Client, conf Config) (network.NetworkProv } // when initializing, first check lua and get all custom providers, then store custom providers -// once fails to store a custom provider, roll back custom providers previously (assume error should not occur) func (r *customController) Initialize(ctx context.Context) error { customNetworkRefList := make([]*unstructured.Unstructured, 0) for _, ref := range r.conf.TrafficConf { @@ -110,11 +109,8 @@ func (r *customController) Initialize(ctx context.Context) error { err := r.storeObject(nObj) if err != nil { klog.Errorf("failed to store custom network provider %s(%s/%s): %s", customNetworkRefList[i].GetKind(), r.conf.RolloutNs, customNetworkRefList[i].GetName(), err.Error()) - klog.Errorf("roll back custom network providers previously") - r.rollBack(customNetworkRefList, i) return err } - customNetworkRefList[i] = nObj } return nil } @@ -124,26 +120,9 @@ func (r *customController) Initialize(ctx context.Context) error { func (r *customController) EnsureRoutes(ctx context.Context, strategy *rolloutv1alpha1.TrafficRoutingStrategy) (bool, error) { done := true // *strategy.Weight == 0 indicates traffic routing is doing finalising and tries to route whole traffic to stable service - // then restore custom network provider directly + // then directly do finalising if strategy.Weight != nil && *strategy.Weight == 0 { - for _, ref := range r.conf.TrafficConf { - obj := &unstructured.Unstructured{} - obj.SetAPIVersion(ref.APIVersion) - obj.SetKind(ref.Kind) - if err := r.Get(ctx, types.NamespacedName{Namespace: r.conf.RolloutNs, Name: ref.Name}, obj); err != nil { - klog.Errorf("failed to get %s(%s/%s) when routing whole traffic to stable service", ref.Kind, r.conf.RolloutNs, ref.Name) - return false, err - } - changed, err := r.restoreObject(obj, true) - if err != nil { - klog.Errorf("failed to restore %s(%s/%s) when routing whole traffic to stable service", ref.Kind, r.conf.RolloutNs, ref.Name) - return false, err - } - if changed { - done = false - } - } - return done, nil + return true, nil } var err error nSpecList := make([]Data, 0) @@ -184,13 +163,9 @@ func (r *customController) EnsureRoutes(ctx context.Context, strategy *rolloutv1 } if err = r.Update(context.TODO(), nObj); err != nil { klog.Errorf("failed to update custom network provider %s(%s/%s) from (%s) to (%s)", nObj.GetKind(), r.conf.RolloutNs, nObj.GetName(), util.DumpJSON(customNetworkRefList[i]), util.DumpJSON(nObj)) - // if fails to update, restore the previous CustomNetworkRefs - klog.Errorf("roll back custom network providers previously") - r.rollBack(customNetworkRefList, i) return false, err } klog.Infof("update custom network provider %s(%s/%s) from (%s) to (%s) success", nObj.GetKind(), r.conf.RolloutNs, nObj.GetName(), util.DumpJSON(customNetworkRefList[i]), util.DumpJSON(nObj)) - customNetworkRefList[i] = nObj done = false } return done, nil @@ -211,25 +186,13 @@ func (r *customController) Finalise(ctx context.Context) error { done = false continue } - if _, err := r.restoreObject(obj, false); err != nil { + if err := r.restoreObject(obj); err != nil { done = false klog.Errorf("failed to restore %s(%s/%s) when finalising: %s", ref.Kind, r.conf.RolloutNs, ref.Name, err.Error()) } } if !done { - return fmt.Errorf("finalising work is not done") - } - return nil -} - -// roll back custom network provider previous to failedIdx -func (r *customController) rollBack(customNetworkRefList []*unstructured.Unstructured, failedIdx int) error { - for j := 0; j < failedIdx; j++ { - if _, err := r.restoreObject(customNetworkRefList[j].DeepCopy(), true); err != nil { - klog.Errorf("failed to roll back custom network provider %s(%s/%s): %s", customNetworkRefList[j].GetKind(), r.conf.RolloutNs, customNetworkRefList[j].GetName(), err.Error()) - continue - } - klog.Infof("roll back custom network provider %s(%s/%s) success", customNetworkRefList[j].GetKind(), r.conf.RolloutNs, customNetworkRefList[j].GetName()) + return fmt.Errorf("finalising work for %s is not done", r.conf.Key) } return nil } @@ -262,44 +225,25 @@ func (r *customController) storeObject(obj *unstructured.Unstructured) error { return nil } -// restore an object from spec stored in OriginalSpecAnnotation, needStoreOriginalSpec indicates whether the original spec should be stored -// if needStoreOriginalSpec is false, indicates traffic routing is doing finalising -// if needStoreOriginalSpec is true, indicates traffic routing failed when ensuring routes and is rolling back or in finalising -func (r *customController) restoreObject(obj *unstructured.Unstructured, needStoreOriginalSpec bool) (bool, error) { - changed := false +// restore an object from spec stored in OriginalSpecAnnotation +func (r *customController) restoreObject(obj *unstructured.Unstructured) error { annotations := obj.GetAnnotations() if annotations == nil || annotations[OriginalSpecAnnotation] == "" { klog.Infof("OriginalSpecAnnotation not found in custom network provider %s(%s/%s)") - return changed, nil + return nil } oSpecStr := annotations[OriginalSpecAnnotation] var oSpec Data _ = json.Unmarshal([]byte(oSpecStr), &oSpec) - if needStoreOriginalSpec { - // when the traffic routing is rolling back, first check whether current spec equals to original spec, if equals, then just return - // this is not a concern when traffic routing is doing finalising, since the OriginalSpecAnnotation should be removed and network provider always need to be updated - delete(annotations, OriginalSpecAnnotation) - data := Data{ - Spec: obj.Object["spec"], - Labels: obj.GetLabels(), - Annotations: annotations, - } - cSpecStr := util.DumpJSON(data) - if oSpecStr == cSpecStr { - return changed, nil - } - oSpec.Annotations[OriginalSpecAnnotation] = oSpecStr - } obj.Object["spec"] = oSpec.Spec obj.SetAnnotations(oSpec.Annotations) obj.SetLabels(oSpec.Labels) if err := r.Update(context.TODO(), obj); err != nil { klog.Errorf("failed to restore object %s(%s/%s) from annotation(%s): %s", obj.GetKind(), r.conf.RolloutNs, obj.GetName(), OriginalSpecAnnotation, err.Error()) - return changed, err + return err } - changed = true klog.Infof("restore custom network provider %s(%s/%s) from annotation(%s) success", obj.GetKind(), obj.GetNamespace(), obj.GetName(), OriginalSpecAnnotation) - return changed, nil + return nil } func (r *customController) executeLuaForCanary(spec Data, strategy *rolloutv1alpha1.TrafficRoutingStrategy, luaScript string) (Data, error) {