diff --git a/pkg/trafficrouting/network/composite.go b/pkg/trafficrouting/network/composite.go index afe50fbb..357e0c6a 100644 --- a/pkg/trafficrouting/network/composite.go +++ b/pkg/trafficrouting/network/composite.go @@ -19,7 +19,7 @@ package network import ( "context" - "go.uber.org/multierr" + "k8s.io/apimachinery/pkg/util/validation/field" "github.com/openkruise/rollouts/api/v1beta1" ) @@ -32,18 +32,18 @@ var ( type CompositeController []NetworkProvider func (c CompositeController) Initialize(ctx context.Context) error { - var err error for _, provider := range c { - err = multierr.Append(err, provider.Initialize(ctx)) + if err := provider.Initialize(ctx); err != nil { + return err + } } - return err + return nil } func (c CompositeController) EnsureRoutes(ctx context.Context, strategy *v1beta1.TrafficRoutingStrategy) (bool, error) { done := true for _, provider := range c { - innerDone, innerErr := provider.EnsureRoutes(ctx, strategy) - if innerErr != nil { + if innerDone, innerErr := provider.EnsureRoutes(ctx, strategy); innerErr != nil { return false, innerErr } else if !innerDone { done = false @@ -52,10 +52,15 @@ func (c CompositeController) EnsureRoutes(ctx context.Context, strategy *v1beta1 return done, nil } -func (c CompositeController) Finalise(ctx context.Context) error { - var err error +func (c CompositeController) Finalise(ctx context.Context) (bool, error) { + modified := false + errList := field.ErrorList{} for _, provider := range c { - err = multierr.Append(err, provider.Finalise(ctx)) + if updated, innerErr := provider.Finalise(ctx); innerErr != nil { + errList = append(errList, field.InternalError(field.NewPath("FinalizeChildNetworkProvider"), innerErr)) + } else if updated { + modified = true + } } - return err + return modified, errList.ToAggregate() } diff --git a/pkg/trafficrouting/network/interface.go b/pkg/trafficrouting/network/interface.go index 28acdf8e..0910a790 100644 --- a/pkg/trafficrouting/network/interface.go +++ b/pkg/trafficrouting/network/interface.go @@ -27,12 +27,11 @@ type NetworkProvider interface { // Initialize only determine if the network resources(ingress & gateway api) exist. // If error is nil, then the network resources exist. Initialize(ctx context.Context) error - // EnsureRoutes check and set canary weight and matches. - // weight indicates percentage of traffic to canary service, and range of values[0,100] - // matches indicates A/B Testing release for headers, cookies - // 1. check if canary has been set desired weight. - // 2. If not, set canary desired weight - // When the first set weight is returned false, mainly to give the provider some time to process, only when again ensure, will return true + // EnsureRoutes check and set routes, e.g. canary weight and match conditions. + // 1. Canary weight specifies the relative proportion of traffic to be forwarded to the canary service within the range of [0,100] + // 2. Match conditions indicates rules to be satisfied for A/B testing scenarios, such as header, cookie, queryParams etc. + // Return true if and only if the route resources have been correctly updated and does not change in this round of reconciliation. + // Otherwise, return false to wait for the eventual consistency. EnsureRoutes(ctx context.Context, strategy *v1beta1.TrafficRoutingStrategy) (bool, error) // Finalise will do some cleanup work after the canary rollout complete, such as delete canary ingress. // if error is nil, the return bool value means if the resources are modified