From 5fe4b6fc14fb3fe784cc37d251c26c84cec71df7 Mon Sep 17 00:00:00 2001 From: yunbo Date: Tue, 13 Aug 2024 15:31:03 +0800 Subject: [PATCH] improve finalising logic for canary release Signed-off-by: yunbo --- api/v1beta1/rollout_types.go | 9 +- pkg/controller/rollout/rollout_canary.go | 132 +++++++++++---- pkg/controller/rollout/rollout_progressing.go | 4 + .../rollout/rollout_progressing_test.go | 157 +++++++++++++++++- pkg/trafficrouting/manager.go | 2 +- 5 files changed, 267 insertions(+), 37 deletions(-) diff --git a/api/v1beta1/rollout_types.go b/api/v1beta1/rollout_types.go index c0b862e7..b079bd1f 100644 --- a/api/v1beta1/rollout_types.go +++ b/api/v1beta1/rollout_types.go @@ -569,18 +569,17 @@ const ( // For Both BlueGreenStrategy and CanaryStrategy: // set workload.pause=false, set workload.partition=0 FinalisingStepTypeBatchRelease FinalisingStepType = "PatchBatchRelease" - //TODO - Currently, the next three steps are in the same function, FinalisingTrafficRouting - // we should try to separate the FinalisingStepTypeGateway and FinalisingStepTypeCanaryService - // with graceful time to prevent some potential issues - // Restore the stable Service (i.e. remove corresponding selector) - FinalisingStepTypeStableService FinalisingStepType = "RestoreStableService" + // Execute the FinalisingTrafficRouting function + FinalisingStepTypeTrafficRouting FinalisingStepType = "FinalisingTrafficRouting" // Restore the GatewayAPI/Ingress/Istio FinalisingStepTypeGateway FinalisingStepType = "RestoreGateway" // Delete Canary Service FinalisingStepTypeDeleteCanaryService FinalisingStepType = "DeleteCanaryService" // Delete Batch Release FinalisingStepTypeDeleteBR FinalisingStepType = "DeleteBatchRelease" + // All needed work done + FinalisingStepTypeEnd FinalisingStepType = "END" ) // +genclient diff --git a/pkg/controller/rollout/rollout_canary.go b/pkg/controller/rollout/rollout_canary.go index c56b550c..f8f7c659 100644 --- a/pkg/controller/rollout/rollout_canary.go +++ b/pkg/controller/rollout/rollout_canary.go @@ -363,43 +363,86 @@ func (m *canaryReleaseManager) doCanaryJump(c *RolloutContext) (jumped bool) { // cleanup after rollout is completed or finished func (m *canaryReleaseManager) doCanaryFinalising(c *RolloutContext) (bool, error) { + canaryStatus := c.NewStatus.CanaryStatus // when CanaryStatus is nil, which means canary action hasn't started yet, don't need doing cleanup - if c.NewStatus.CanaryStatus == nil { + if canaryStatus == nil { return true, nil } - // 1. rollout progressing complete, remove rollout progressing annotation in workload + // rollout progressing complete, remove rollout progressing annotation in workload err := m.removeRolloutProgressingAnnotation(c) if err != nil { return false, err } tr := newTrafficRoutingContext(c) - // 2. remove stable service the pod revision selector, so stable service will be selector all version pods. - done, err := m.trafficRoutingManager.FinalisingTrafficRouting(tr) - c.NewStatus.CanaryStatus.LastUpdateTime = tr.LastUpdateTime - if err != nil || !done { - return done, err - } - // 3. set workload.pause=false; set workload.partition=0 - done, err = m.finalizingBatchRelease(c) - if err != nil || !done { - return done, err - } - // 4. modify network api(ingress or gateway api) configuration, and route 100% traffic to stable pods. - done, err = m.trafficRoutingManager.FinalisingTrafficRouting(tr) - c.NewStatus.CanaryStatus.LastUpdateTime = tr.LastUpdateTime - if err != nil || !done { - return done, err - } - // 5. delete batchRelease crd - done, err = m.removeBatchRelease(c) - if err != nil { - klog.Errorf("rollout(%s/%s) Finalize batchRelease failed: %s", c.Rollout.Namespace, c.Rollout.Name, err.Error()) - return false, err - } else if !done { - return false, nil + // execute steps based on the predefined order for each reason + nextStep := nextTask(c.FinalizeReason, canaryStatus.FinalisingStep) + // if current step is empty, set it with the first step + // if current step is end, we just return + if len(canaryStatus.FinalisingStep) == 0 { + canaryStatus.FinalisingStep = nextStep + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + } else if canaryStatus.FinalisingStep == v1beta1.FinalisingStepTypeEnd { + klog.Infof("rollout(%s/%s) finalising process is already completed", c.Rollout.Namespace, c.Rollout.Name) + return true, nil } - klog.Infof("rollout(%s/%s) doCanaryFinalising success", c.Rollout.Namespace, c.Rollout.Name) - return true, nil + klog.Infof("rollout(%s/%s) Finalising Step is %s", c.Rollout.Namespace, c.Rollout.Name, canaryStatus.FinalisingStep) + // the steps. order is maitained by the nextStep + switch canaryStatus.FinalisingStep { + // call the FinalisingTrafficRouting function to: + // 1.restore stable service selector to select all pods + // 2.restore network api(ingress/ gateway api/ istio) configuration + // 3.delete canary service + case v1beta1.FinalisingStepTypeTrafficRouting: + done, err := m.trafficRoutingManager.FinalisingTrafficRouting(tr) + if err != nil || !done { + canaryStatus.LastUpdateTime = tr.LastUpdateTime + return done, err + } + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + canaryStatus.FinalisingStep = nextStep + if canaryStatus.FinalisingStep == v1beta1.FinalisingStepTypeEnd { + return true, nil + } + // set workload.pause=false; set workload.partition=0 + case v1beta1.FinalisingStepTypeBatchRelease: + done, err := m.finalizingBatchRelease(c) + if err != nil || !done { + return done, err + } + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + canaryStatus.FinalisingStep = nextStep + if canaryStatus.FinalisingStep == v1beta1.FinalisingStepTypeEnd { + return true, nil + } + // delete batchRelease + case v1beta1.FinalisingStepTypeDeleteBR: + done, err := m.removeBatchRelease(c) + if err != nil { + klog.Errorf("rollout(%s/%s) Finalize batchRelease failed: %s", c.Rollout.Namespace, c.Rollout.Name, err.Error()) + return false, err + } else if !done { + return false, nil + } + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + canaryStatus.FinalisingStep = nextStep + if canaryStatus.FinalisingStep == v1beta1.FinalisingStepTypeEnd { + return true, nil + } + // restore the gateway resources (ingress/gatewayAPI/Istio), that means + // only stable Service will accept the traffic + case v1beta1.FinalisingStepTypeGateway: + retry, err := m.trafficRoutingManager.RestoreGateway(tr) + if err != nil || retry { + return false, err + } + canaryStatus.LastUpdateTime = &metav1.Time{Time: time.Now()} + canaryStatus.FinalisingStep = nextStep + if canaryStatus.FinalisingStep == v1beta1.FinalisingStepTypeEnd { + return true, nil + } + } + + return false, nil } func (m *canaryReleaseManager) removeRolloutProgressingAnnotation(c *RolloutContext) error { @@ -601,3 +644,36 @@ func (m *canaryReleaseManager) syncBatchRelease(br *v1beta1.BatchRelease, canary } return nil } + +// calculate next task +func nextTask(reason string, currentTask v1beta1.FinalisingStepType) v1beta1.FinalisingStepType { + var taskSequence []v1beta1.FinalisingStepType + //REVIEW - should we consider more complex scenarios? + // like, user pauses Rollout and rollbacks the workload at the same time? + switch reason { + case v1beta1.FinaliseReasonRollback: // rollback + taskSequence = []v1beta1.FinalisingStepType{ + v1beta1.FinalisingStepTypeGateway, // route all traffic to stable version + v1beta1.FinalisingStepTypeBatchRelease, // scale up old, scale down new + v1beta1.FinalisingStepTypeDeleteBR, + v1beta1.FinalisingStepTypeTrafficRouting, // do cleaning works(restore stable Service, remove canary Service) + } + default: // others: success/paused/disabled + taskSequence = []v1beta1.FinalisingStepType{ + v1beta1.FinalisingStepTypeTrafficRouting, // remove selector of stable Service + v1beta1.FinalisingStepTypeBatchRelease, // scale up new, scale down old + v1beta1.FinalisingStepTypeDeleteBR, + } + } + // if currentTask is empty, return first task + if len(currentTask) == 0 { + return taskSequence[0] + } + // find next task + for i := range taskSequence { + if currentTask == taskSequence[i] && i < len(taskSequence)-1 { + return taskSequence[i+1] + } + } + return v1beta1.FinalisingStepTypeEnd +} diff --git a/pkg/controller/rollout/rollout_progressing.go b/pkg/controller/rollout/rollout_progressing.go index 7cfabf8d..d47b5ada 100644 --- a/pkg/controller/rollout/rollout_progressing.go +++ b/pkg/controller/rollout/rollout_progressing.go @@ -45,6 +45,8 @@ type RolloutContext struct { RecheckTime *time.Time // wait stable workload pods ready WaitReady bool + // finalising reason + FinalizeReason string } // parameter1 retryReconcile, parameter2 error @@ -116,6 +118,7 @@ func (r *RolloutReconciler) reconcileRolloutProgressing(rollout *v1beta1.Rollout klog.Infof("rollout(%s/%s) is Progressing, and in reason(%s)", rollout.Namespace, rollout.Name, cond.Reason) var done bool rolloutContext.WaitReady = true + rolloutContext.FinalizeReason = v1beta1.FinaliseReasonSuccess done, err = r.doFinalising(rolloutContext) if err != nil { return nil, err @@ -140,6 +143,7 @@ func (r *RolloutReconciler) reconcileRolloutProgressing(rollout *v1beta1.Rollout case v1alpha1.ProgressingReasonCancelling: klog.Infof("rollout(%s/%s) is Progressing, and in reason(%s)", rollout.Namespace, rollout.Name, cond.Reason) var done bool + rolloutContext.FinalizeReason = v1beta1.FinaliseReasonRollback done, err = r.doFinalising(rolloutContext) if err != nil { return nil, err diff --git a/pkg/controller/rollout/rollout_progressing_test.go b/pkg/controller/rollout/rollout_progressing_test.go index 15098490..311afa81 100644 --- a/pkg/controller/rollout/rollout_progressing_test.go +++ b/pkg/controller/rollout/rollout_progressing_test.go @@ -175,7 +175,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { }, }, { - name: "ReconcileRolloutProgressing rolling -> finalizing", + name: "ReconcileRolloutProgressing rolling -> finalizing", // call FinalisingTrafficRouting to restore stable Service getObj: func() ([]*apps.Deployment, []*apps.ReplicaSet) { dep1 := deploymentDemo.DeepCopy() dep2 := deploymentDemo.DeepCopy() @@ -235,7 +235,146 @@ func TestReconcileRolloutProgressing(t *testing.T) { }, }, { - name: "ReconcileRolloutProgressing finalizing1", + name: "ReconcileRolloutProgressing finalizing1", // call FinalisingTrafficRouting to remove canary service + getObj: func() ([]*apps.Deployment, []*apps.ReplicaSet) { + dep1 := deploymentDemo.DeepCopy() + delete(dep1.Annotations, util.InRolloutProgressingAnnotation) + dep1.Status = apps.DeploymentStatus{ + ObservedGeneration: 2, + Replicas: 10, + UpdatedReplicas: 5, + ReadyReplicas: 10, + AvailableReplicas: 10, + } + rs1 := rsDemo.DeepCopy() + return []*apps.Deployment{dep1}, []*apps.ReplicaSet{rs1} + }, + getNetwork: func() ([]*corev1.Service, []*netv1.Ingress) { + s1 := demoService.DeepCopy() + s1.Spec.Selector[apps.DefaultDeploymentUniqueLabelKey] = "podtemplatehash-v1" + s2 := demoService.DeepCopy() + s2.Name = s1.Name + "-canary" + return []*corev1.Service{s1, s2}, []*netv1.Ingress{demoIngress.DeepCopy()} + }, + getRollout: func() (*v1beta1.Rollout, *v1beta1.BatchRelease, *v1alpha1.TrafficRouting) { + obj := rolloutDemo.DeepCopy() + obj.Annotations[v1alpha1.TrafficRoutingAnnotation] = "tr-demo" + obj.Status.CanaryStatus.ObservedWorkloadGeneration = 2 + obj.Status.CanaryStatus.RolloutHash = "f55bvd874d5f2fzvw46bv966x4bwbdv4wx6bd9f7b46ww788954b8z8w29b7wxfd" + obj.Status.CanaryStatus.StableRevision = "pod-template-hash-v1" + obj.Status.CanaryStatus.CanaryRevision = "6f8cc56547" + obj.Status.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" + obj.Status.CanaryStatus.CurrentStepIndex = 4 + obj.Status.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + cond := util.GetRolloutCondition(obj.Status, v1beta1.RolloutConditionProgressing) + cond.Reason = v1alpha1.ProgressingReasonFinalising + cond.Status = corev1.ConditionTrue + util.SetRolloutCondition(&obj.Status, *cond) + br := batchDemo.DeepCopy() + br.Spec.ReleasePlan.Batches = []v1beta1.ReleaseBatch{ + { + CanaryReplicas: intstr.FromInt(1), + }, + } + tr := demoTR.DeepCopy() + tr.Finalizers = []string{util.ProgressingRolloutFinalizer(rolloutDemo.Name)} + return obj, br, tr + }, + expectStatus: func() *v1beta1.RolloutStatus { + s := rolloutDemo.Status.DeepCopy() + s.CanaryStatus.ObservedWorkloadGeneration = 2 + s.CanaryStatus.RolloutHash = "f55bvd874d5f2fzvw46bv966x4bwbdv4wx6bd9f7b46ww788954b8z8w29b7wxfd" + s.CanaryStatus.StableRevision = "pod-template-hash-v1" + s.CanaryStatus.CanaryRevision = "6f8cc56547" + s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" + s.CanaryStatus.CurrentStepIndex = 4 + s.CanaryStatus.NextStepIndex = 0 + s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeTrafficRouting + s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) + cond.Reason = v1alpha1.ProgressingReasonFinalising + cond.Status = corev1.ConditionTrue + util.SetRolloutCondition(s, *cond) + s.CurrentStepIndex = s.CanaryStatus.CurrentStepIndex + s.CurrentStepState = s.CanaryStatus.CurrentStepState + return s + }, + expectTr: func() *v1alpha1.TrafficRouting { + tr := demoTR.DeepCopy() + return tr + }, + }, + { + name: "ReconcileRolloutProgressing finalizing2", // go to the next finalising step + getObj: func() ([]*apps.Deployment, []*apps.ReplicaSet) { + dep1 := deploymentDemo.DeepCopy() + delete(dep1.Annotations, util.InRolloutProgressingAnnotation) + dep1.Status = apps.DeploymentStatus{ + ObservedGeneration: 2, + Replicas: 10, + UpdatedReplicas: 5, + ReadyReplicas: 10, + AvailableReplicas: 10, + } + rs1 := rsDemo.DeepCopy() + return []*apps.Deployment{dep1}, []*apps.ReplicaSet{rs1} + }, + getNetwork: func() ([]*corev1.Service, []*netv1.Ingress) { + return []*corev1.Service{demoService.DeepCopy()}, []*netv1.Ingress{demoIngress.DeepCopy()} + }, + getRollout: func() (*v1beta1.Rollout, *v1beta1.BatchRelease, *v1alpha1.TrafficRouting) { + obj := rolloutDemo.DeepCopy() + obj.Annotations[v1alpha1.TrafficRoutingAnnotation] = "tr-demo" + obj.Status.CanaryStatus.ObservedWorkloadGeneration = 2 + obj.Status.CanaryStatus.RolloutHash = "f55bvd874d5f2fzvw46bv966x4bwbdv4wx6bd9f7b46ww788954b8z8w29b7wxfd" + obj.Status.CanaryStatus.StableRevision = "pod-template-hash-v1" + obj.Status.CanaryStatus.CanaryRevision = "6f8cc56547" + obj.Status.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" + obj.Status.CanaryStatus.CurrentStepIndex = 4 + obj.Status.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + // given that the selector of stable Service is removed + // we will go on it the next step, i.e. patch BatchRelease + obj.Status.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeTrafficRouting + cond := util.GetRolloutCondition(obj.Status, v1beta1.RolloutConditionProgressing) + cond.Reason = v1alpha1.ProgressingReasonFinalising + cond.Status = corev1.ConditionTrue + util.SetRolloutCondition(&obj.Status, *cond) + br := batchDemo.DeepCopy() + br.Spec.ReleasePlan.Batches = []v1beta1.ReleaseBatch{ + { + CanaryReplicas: intstr.FromInt(1), + }, + } + tr := demoTR.DeepCopy() + tr.Finalizers = []string{util.ProgressingRolloutFinalizer(rolloutDemo.Name)} + return obj, br, tr + }, + expectStatus: func() *v1beta1.RolloutStatus { + s := rolloutDemo.Status.DeepCopy() + s.CanaryStatus.ObservedWorkloadGeneration = 2 + s.CanaryStatus.RolloutHash = "f55bvd874d5f2fzvw46bv966x4bwbdv4wx6bd9f7b46ww788954b8z8w29b7wxfd" + s.CanaryStatus.StableRevision = "pod-template-hash-v1" + s.CanaryStatus.CanaryRevision = "6f8cc56547" + s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" + s.CanaryStatus.CurrentStepIndex = 4 + s.CanaryStatus.NextStepIndex = 0 + s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeBatchRelease + s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) + cond.Reason = v1alpha1.ProgressingReasonFinalising + cond.Status = corev1.ConditionTrue + util.SetRolloutCondition(s, *cond) + s.CurrentStepIndex = s.CanaryStatus.CurrentStepIndex + s.CurrentStepState = s.CanaryStatus.CurrentStepState + return s + }, + expectTr: func() *v1alpha1.TrafficRouting { + tr := demoTR.DeepCopy() + return tr + }, + }, + { + name: "ReconcileRolloutProgressing finalizing3", getObj: func() ([]*apps.Deployment, []*apps.ReplicaSet) { dep1 := deploymentDemo.DeepCopy() delete(dep1.Annotations, util.InRolloutProgressingAnnotation) @@ -262,6 +401,9 @@ func TestReconcileRolloutProgressing(t *testing.T) { obj.Status.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" obj.Status.CanaryStatus.CurrentStepIndex = 4 obj.Status.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + // because the batchRelease hasn't completed (ie. br.Status.Phase is not completed), + // it will take more than one reconciles to go on to the next step + obj.Status.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeBatchRelease cond := util.GetRolloutCondition(obj.Status, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonFinalising cond.Status = corev1.ConditionTrue @@ -285,6 +427,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 4 s.CanaryStatus.NextStepIndex = 0 + s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeBatchRelease s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted cond := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonFinalising @@ -300,7 +443,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { }, }, { - name: "ReconcileRolloutProgressing finalizing2", + name: "ReconcileRolloutProgressing finalizing4", getObj: func() ([]*apps.Deployment, []*apps.ReplicaSet) { dep1 := deploymentDemo.DeepCopy() delete(dep1.Annotations, util.InRolloutProgressingAnnotation) @@ -326,6 +469,9 @@ func TestReconcileRolloutProgressing(t *testing.T) { obj.Status.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" obj.Status.CanaryStatus.CurrentStepIndex = 4 obj.Status.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + // the batchRelease has completed (ie. br.Status.Phase is completed), + // we expect the finalizing step to be next step, i.e. deleteBatchRelease + obj.Status.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeBatchRelease cond := util.GetRolloutCondition(obj.Status, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonFinalising cond.Status = corev1.ConditionTrue @@ -348,6 +494,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" s.CanaryStatus.CurrentStepIndex = 4 s.CanaryStatus.NextStepIndex = 0 + s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeDeleteBR s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted cond2 := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond2.Reason = v1alpha1.ProgressingReasonFinalising @@ -385,6 +532,9 @@ func TestReconcileRolloutProgressing(t *testing.T) { obj.Status.CanaryStatus.PodTemplateHash = "pod-template-hash-v2" obj.Status.CanaryStatus.CurrentStepIndex = 4 obj.Status.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + // deleteBatchRelease is the last step, and it won't wait a grace time + // after this step, this release should be succeeded + obj.Status.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeDeleteBR cond := util.GetRolloutCondition(obj.Status, v1beta1.RolloutConditionProgressing) cond.Reason = v1alpha1.ProgressingReasonFinalising cond.Status = corev1.ConditionTrue @@ -401,6 +551,7 @@ func TestReconcileRolloutProgressing(t *testing.T) { s.CanaryStatus.CurrentStepIndex = 4 s.CanaryStatus.NextStepIndex = 0 s.CanaryStatus.CurrentStepState = v1beta1.CanaryStepStateCompleted + s.CanaryStatus.FinalisingStep = v1beta1.FinalisingStepTypeEnd cond2 := util.GetRolloutCondition(*s, v1beta1.RolloutConditionProgressing) cond2.Reason = v1alpha1.ProgressingReasonCompleted cond2.Status = corev1.ConditionFalse diff --git a/pkg/trafficrouting/manager.go b/pkg/trafficrouting/manager.go index a1fe2813..8e4cb0bc 100644 --- a/pkg/trafficrouting/manager.go +++ b/pkg/trafficrouting/manager.go @@ -205,7 +205,7 @@ func (m *Manager) FinalisingTrafficRouting(c *TrafficRoutingContext) (bool, erro return true, nil } klog.Infof("%s start finalising traffic routing", c.Key) - // remove stable service the pod revision selector, so stable service will be selector all version pods. + // remove stable service the pod revision selector, so stable service will select pods of all versions. if retry, err := m.RestoreStableService(c); err != nil || retry { return false, err }