Skip to content

Commit

Permalink
add support for the traffic weight > 100.
Browse files Browse the repository at this point in the history
Signed-off-by: Liming Liu <[email protected]>
  • Loading branch information
andyliuliming committed Dec 19, 2023
1 parent 0eff316 commit d5dd5d4
Show file tree
Hide file tree
Showing 11 changed files with 209 additions and 109 deletions.
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,7 @@ golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJ
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
golang.org/x/sync v0.4.0/go.mod h1:FU7BRWz2tNW+3quACPkgCx/L+uEAv1htQ0V83Z9Rj+Y=
golang.org/x/sys v0.0.0-20170830134202-bb24a47a89ea/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20180905080454-ebe1bf3edb33/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/rollout-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,9 @@ spec:
- name
type: object
type: array
maxTrafficWeight:
format: int32
type: integer
nginx:
properties:
additionalIngressAnnotations:
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,11 +379,14 @@ type RolloutTrafficRouting struct {
ManagedRoutes []MangedRoutes `json:"managedRoutes,omitempty" protobuf:"bytes,8,rep,name=managedRoutes"`
// Apisix holds specific configuration to use Apisix to route traffic
Apisix *ApisixTrafficRouting `json:"apisix,omitempty" protobuf:"bytes,9,opt,name=apisix"`

// +kubebuilder:validation:Schemaless
// +kubebuilder:pruning:PreserveUnknownFields
// +kubebuilder:validation:Type=object
// Plugins holds specific configuration that traffic router plugins can use for routing traffic
Plugins map[string]json.RawMessage `json:"plugins,omitempty" protobuf:"bytes,10,opt,name=plugins"`

MaxTrafficWeight *int32 `json:"maxTrafficWeight,omitempty" protobuf:"varint,11,opt,name=maxTrafficWeight"`
}

type MangedRoutes struct {
Expand Down
20 changes: 16 additions & 4 deletions pkg/apis/rollouts/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,16 @@ import (

"github.com/argoproj/argo-rollouts/pkg/apis/rollouts/v1alpha1"
"github.com/argoproj/argo-rollouts/utils/defaults"
"github.com/argoproj/argo-rollouts/utils/weightutil"
)

const (
// Validate Spec constants

// MissingFieldMessage the message to indicate rollout is missing a field
MissingFieldMessage = "Rollout has missing field '%s'"
// InvalidSetWeightMessage indicates the setweight value needs to be between 0 and 100
InvalidSetWeightMessage = "SetWeight needs to be between 0 and 100"
// InvalidSetWeightMessage indicates the setweight value needs to be between 0 and max weight
InvalidSetWeightMessage = "SetWeight needs to be between 0 and %d"
// InvalidCanaryExperimentTemplateWeightWithoutTrafficRouting indicates experiment weight cannot be set without trafficRouting
InvalidCanaryExperimentTemplateWeightWithoutTrafficRouting = "Experiment template weight cannot be set unless TrafficRouting is enabled"
// InvalidSetCanaryScaleTrafficPolicy indicates that TrafficRouting, required for SetCanaryScale, is missing
Expand Down Expand Up @@ -72,6 +73,8 @@ const (
InvalidCanaryDynamicStableScale = "Canary dynamicStableScale can only be used with traffic routing"
// InvalidCanaryDynamicStableScaleWithScaleDownDelay indicates that canary.dynamicStableScale cannot be used with scaleDownDelaySeconds
InvalidCanaryDynamicStableScaleWithScaleDownDelay = "Canary dynamicStableScale cannot be used with scaleDownDelaySeconds"
// InvalidCanaryMaxWeightOnlySupportInNginxAndPlugins indicates that canary.maxTrafficWeight cannot be used
InvalidCanaryMaxWeightOnlySupportInNginxAndPlugins = "Canary maxTrafficWeight in traffic routing only support in Nginx and Plugins"
// InvalidPingPongProvidedMessage indicates that both ping and pong service must be set to use Ping-Pong feature
InvalidPingPongProvidedMessage = "Ping service and Pong service must to be set to use Ping-Pong feature"
// DuplicatedPingPongServicesMessage indicates that the rollout uses the same service for the ping and pong services
Expand Down Expand Up @@ -281,6 +284,12 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat
if canary.ScaleDownDelaySeconds != nil && canary.DynamicStableScale {
allErrs = append(allErrs, field.Invalid(fldPath.Child("dynamicStableScale"), canary.DynamicStableScale, InvalidCanaryDynamicStableScaleWithScaleDownDelay))
}
// only the nginx and plugin have this support for now
if canary.TrafficRouting.MaxTrafficWeight != nil {
if canary.TrafficRouting.Nginx == nil && len(canary.TrafficRouting.Plugins) == 0 {
allErrs = append(allErrs, field.Invalid(fldPath.Child("trafficRouting").Child("maxTrafficWeight"), canary.TrafficRouting.MaxTrafficWeight, InvalidCanaryMaxWeightOnlySupportInNginxAndPlugins))
}
}
}

for i, step := range canary.Steps {
Expand All @@ -292,8 +301,11 @@ func ValidateRolloutStrategyCanary(rollout *v1alpha1.Rollout, fldPath *field.Pat
step.Experiment == nil, step.Pause == nil, step.SetWeight == nil, step.Analysis == nil, step.SetCanaryScale == nil, step.SetHeaderRoute == nil, step.SetMirrorRoute == nil)
allErrs = append(allErrs, field.Invalid(stepFldPath, errVal, InvalidStepMessage))
}
if step.SetWeight != nil && (*step.SetWeight < 0 || *step.SetWeight > 100) {
allErrs = append(allErrs, field.Invalid(stepFldPath.Child("setWeight"), *canary.Steps[i].SetWeight, InvalidSetWeightMessage))

maxTrafficWeight := weightutil.MaxTrafficWeight(rollout)

if step.SetWeight != nil && (*step.SetWeight < 0 || *step.SetWeight > maxTrafficWeight) {
allErrs = append(allErrs, field.Invalid(stepFldPath.Child("setWeight"), *canary.Steps[i].SetWeight, fmt.Sprintf(InvalidSetWeightMessage, maxTrafficWeight)))
}
if step.Pause != nil && step.Pause.DurationSeconds() < 0 {
allErrs = append(allErrs, field.Invalid(stepFldPath.Child("pause").Child("duration"), step.Pause.DurationSeconds(), InvalidDurationMessage))
Expand Down
54 changes: 53 additions & 1 deletion pkg/apis/rollouts/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,59 @@ func TestValidateRolloutStrategyCanary(t *testing.T) {
invalidRo := ro.DeepCopy()
invalidRo.Spec.Strategy.Canary.Steps[0].SetWeight = &setWeight
allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath(""))
assert.Equal(t, InvalidSetWeightMessage, allErrs[0].Detail)
assert.Equal(t, fmt.Sprintf(InvalidSetWeightMessage, 100), allErrs[0].Detail)
})

t.Run("only nginx/plugins support max weight value", func(t *testing.T) {
anyWeight := int32(1)

type testCases struct {
trafficRouting *v1alpha1.RolloutTrafficRouting
expectError bool
expectedError string
}

testCasesList := []testCases{
{
trafficRouting: &v1alpha1.RolloutTrafficRouting{
ALB: &v1alpha1.ALBTrafficRouting{RootService: "root-service"},
MaxTrafficWeight: &anyWeight,
},
expectError: true,
expectedError: InvalidCanaryMaxWeightOnlySupportInNginxAndPlugins,
},
{
trafficRouting: &v1alpha1.RolloutTrafficRouting{
Nginx: &v1alpha1.NginxTrafficRouting{
StableIngress: "stable-ingress",
},
MaxTrafficWeight: &anyWeight,
},
expectError: false,
},
{
trafficRouting: &v1alpha1.RolloutTrafficRouting{
Plugins: map[string]json.RawMessage{
"anyplugin": []byte(`{"key": "value"}`),
},
MaxTrafficWeight: &anyWeight,
},
expectError: false,
},
}

for _, testCase := range testCasesList {
invalidRo := ro.DeepCopy()
invalidRo.Spec.Strategy.Canary.Steps[0].SetWeight = &anyWeight
invalidRo.Spec.Strategy.Canary.TrafficRouting = testCase.trafficRouting
allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath(""))
if !testCase.expectError {
assert.Empty(t, allErrs)
continue
}

assert.Equal(t, testCase.expectedError, allErrs[0].Detail)
}
})

t.Run("invalid duration set in paused step", func(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions pkg/kubectl-argo-rollouts/info/rollout_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/argoproj/argo-rollouts/utils/defaults"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
rolloututil "github.com/argoproj/argo-rollouts/utils/rollout"
"github.com/argoproj/argo-rollouts/utils/weightutil"
)

func NewRolloutInfo(
Expand Down Expand Up @@ -58,12 +59,12 @@ func NewRolloutInfo(
currentStep, _ := replicasetutil.GetCurrentCanaryStep(ro)

if currentStep == nil {
roInfo.ActualWeight = "100"
roInfo.ActualWeight = fmt.Sprintf("%d", weightutil.MaxTrafficWeight(ro))
} else if ro.Status.AvailableReplicas > 0 {
if ro.Spec.Strategy.Canary.TrafficRouting == nil {
for _, rs := range roInfo.ReplicaSets {
if rs.Canary {
roInfo.ActualWeight = fmt.Sprintf("%d", (rs.Available*100)/ro.Status.AvailableReplicas)
roInfo.ActualWeight = fmt.Sprintf("%d", (rs.Available*weightutil.MaxTrafficWeight(ro))/ro.Status.AvailableReplicas)
}
}
} else {
Expand Down
14 changes: 10 additions & 4 deletions rollout/trafficrouting.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/argoproj/argo-rollouts/utils/record"
replicasetutil "github.com/argoproj/argo-rollouts/utils/replicaset"
rolloututil "github.com/argoproj/argo-rollouts/utils/rollout"
"github.com/argoproj/argo-rollouts/utils/weightutil"
)

// NewTrafficRoutingReconciler identifies return the TrafficRouting Plugin that the rollout wants to modify
Expand Down Expand Up @@ -132,6 +133,7 @@ func (c *Controller) NewTrafficRoutingReconciler(roCtx *rolloutContext) ([]traff
return nil, nil
}

// this currently only be used in the canary strategy
func (c *rolloutContext) reconcileTrafficRouting() error {
reconcilers, err := c.newTrafficRoutingReconciler(c)
// a return here does ensure that all trafficReconcilers are healthy
Expand Down Expand Up @@ -199,7 +201,8 @@ func (c *rolloutContext) reconcileTrafficRouting() error {
// But we can only increase canary weight according to available replica counts of the canary.
// we will need to set the desiredWeight to 0 when the newRS is not available.
if c.rollout.Spec.Strategy.Canary.DynamicStableScale {
desiredWeight = (100 * c.newRS.Status.AvailableReplicas) / *c.rollout.Spec.Replicas
// TODO: handle total weight
desiredWeight = (weightutil.MaxTrafficWeight(c.rollout) * c.newRS.Status.AvailableReplicas) / *c.rollout.Spec.Replicas
} else if c.rollout.Status.Canary.Weights != nil {
desiredWeight = c.rollout.Status.Canary.Weights.Canary.Weight
}
Expand Down Expand Up @@ -227,7 +230,7 @@ func (c *rolloutContext) reconcileTrafficRouting() error {
desiredWeight = replicasetutil.GetCurrentSetWeight(c.rollout)
weightDestinations = append(weightDestinations, c.calculateWeightDestinationsFromExperiment()...)
} else {
desiredWeight = 100
desiredWeight = weightutil.MaxTrafficWeight(c.rollout)
}
}
// We need to check for revision > 1 because when we first install the rollout we run step 0 this prevents that.
Expand Down Expand Up @@ -301,7 +304,9 @@ func (c *rolloutContext) calculateDesiredWeightOnAbortOrStableRollback() int32 {
}
// When using dynamic stable scaling, we must dynamically decreasing the weight to the canary
// according to the availability of the stable (whatever it can support).
desiredWeight := 100 - ((100 * c.stableRS.Status.AvailableReplicas) / *c.rollout.Spec.Replicas)
// TODO: handle total weight
// c.rollout.Spec.Strategy.Canary.TrafficRouting.MaxTrafficWeight
desiredWeight := weightutil.MaxTrafficWeight(c.rollout) - ((weightutil.MaxTrafficWeight(c.rollout) * c.stableRS.Status.AvailableReplicas) / *c.rollout.Spec.Replicas)
if c.rollout.Status.Canary.Weights != nil {
// This ensures that if we are already at a lower weight, then we will not
// increase the weight because stable availability is flapping (e.g. pod restarts)
Expand Down Expand Up @@ -334,7 +339,8 @@ func calculateWeightStatus(ro *v1alpha1.Rollout, canaryHash, stableHash string,
ServiceName: ro.Spec.Strategy.Canary.CanaryService,
},
}
stableWeight := 100 - desiredWeight
// TODO: handle total weight
stableWeight := weightutil.MaxTrafficWeight(ro) - desiredWeight
for _, weightDest := range weightDestinations {
weights.Additional = append(weights.Additional, weightDest)
stableWeight -= weightDest.Weight
Expand Down
4 changes: 4 additions & 0 deletions rollout/trafficrouting/nginx/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,10 @@ func (r *Reconciler) buildLegacyCanaryIngress(stableIngress *extensionsv1beta1.I
desiredCanaryIngress.Annotations[fmt.Sprintf("%s/canary", annotationPrefix)] = "true"
desiredCanaryIngress.Annotations[fmt.Sprintf("%s/canary-weight", annotationPrefix)] = fmt.Sprintf("%d", desiredWeight)

if r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.MaxTrafficWeight != nil {
weightTotal := *r.cfg.Rollout.Spec.Strategy.Canary.TrafficRouting.MaxTrafficWeight
desiredCanaryIngress.Annotations[fmt.Sprintf("%s/canary-weight-total", annotationPrefix)] = fmt.Sprintf("%d", weightTotal)
}

Check warning on line 215 in rollout/trafficrouting/nginx/nginx.go

View check run for this annotation

Codecov / codecov/patch

rollout/trafficrouting/nginx/nginx.go#L213-L215

Added lines #L213 - L215 were not covered by tests
return ingressutil.NewLegacyIngress(desiredCanaryIngress), nil
}

Expand Down
Loading

0 comments on commit d5dd5d4

Please sign in to comment.