Skip to content

Commit

Permalink
remove roll back
Browse files Browse the repository at this point in the history
Signed-off-by: Kuromesi <[email protected]>
  • Loading branch information
Kuromesi committed Sep 12, 2023
1 parent 520ae57 commit d6046aa
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 67 deletions.
5 changes: 3 additions & 2 deletions pkg/controller/trafficrouting/trafficrouting_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
74 changes: 9 additions & 65 deletions pkg/trafficrouting/network/custom/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit d6046aa

Please sign in to comment.