Skip to content

Commit

Permalink
chore: add the max weight field.
Browse files Browse the repository at this point in the history
Signed-off-by: Liming Liu <[email protected]>
  • Loading branch information
Ubuntu authored and andyliuliming committed Jul 22, 2023
1 parent 45e549d commit 5c542f5
Show file tree
Hide file tree
Showing 10 changed files with 408 additions and 356 deletions.
720 changes: 375 additions & 345 deletions pkg/apis/rollouts/v1alpha1/generated.pb.go

Large diffs are not rendered by default.

3 changes: 3 additions & 0 deletions pkg/apis/rollouts/v1alpha1/generated.proto

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -622,6 +622,9 @@ type CanaryStep struct {
// SetMirrorRoutes Mirrors traffic that matches rules to a particular destination
// +optional
SetMirrorRoute *SetMirrorRoute `json:"setMirrorRoute,omitempty" protobuf:"bytes,8,opt,name=setMirrorRoute"`

// MaxWeight sets what is the max weight of the traffic, the newRS will receive SetWeight/MaxWeight traffic
MaxWeight *int32 `json:"maxWeight,omitempty" protobuf:"varint,9,opt,name=maxWeight"`
}

type SetMirrorRoute struct {
Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/rollouts/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ import (

const (
// Validate Spec constants

DefaultMaxWeight = int32(100)
// 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 = "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 @@ -293,8 +293,13 @@ 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))
maxWeight := DefaultMaxWeight
if step.MaxWeight != nil {
maxWeight = *step.MaxWeight
}
if step.SetWeight != nil && (*step.SetWeight < 0 || *step.SetWeight > maxWeight) {
errVal := fmt.Sprintf(InvalidSetWeightMessage, maxWeight)
allErrs = append(allErrs, field.Invalid(stepFldPath.Child("setWeight"), *canary.Steps[i].SetWeight, errVal))
}
if step.Pause != nil && step.Pause.DurationSeconds() < 0 {
allErrs = append(allErrs, field.Invalid(stepFldPath.Child("pause").Child("duration"), step.Pause.DurationSeconds(), InvalidDurationMessage))
Expand Down
13 changes: 11 additions & 2 deletions pkg/apis/rollouts/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,20 @@ func TestValidateRolloutStrategyCanary(t *testing.T) {
})

t.Run("invalid set weight value", func(t *testing.T) {
setWeight := int32(101)
setWeight := int32(DefaultMaxWeight + 1)

Check failure on line 237 in pkg/apis/rollouts/validation/validation_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code

unnecessary conversion (unconvert)
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, DefaultMaxWeight), allErrs[0].Detail)
})

t.Run("max weight works", func(t *testing.T) {
setWeight := int32(DefaultMaxWeight + 1)

Check failure on line 245 in pkg/apis/rollouts/validation/validation_test.go

View workflow job for this annotation

GitHub Actions / Lint Go code

unnecessary conversion (unconvert)
invalidRo := ro.DeepCopy()
invalidRo.Spec.Strategy.Canary.Steps[0].SetWeight = &setWeight
invalidRo.Spec.Strategy.Canary.Steps[0].MaxWeight = &setWeight
allErrs := ValidateRolloutStrategyCanary(invalidRo, field.NewPath(""))
assert.Equal(t, 0, len(allErrs))
})

t.Run("invalid duration set in paused step", func(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions rollout/trafficrouting/nginx/nginx.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ func (r *Reconciler) buildLegacyCanaryIngress(stableIngress *extensionsv1beta1.I
// Always set `canary` and `canary-weight` - `canary-by-header` and `canary-by-cookie`, if set, will always take precedence
desiredCanaryIngress.Annotations[fmt.Sprintf("%s/canary", annotationPrefix)] = "true"
desiredCanaryIngress.Annotations[fmt.Sprintf("%s/canary-weight", annotationPrefix)] = fmt.Sprintf("%d", desiredWeight)
desiredCanaryIngress.Annotations[fmt.Sprintf("%s/canary-weight-total", annotationPrefix)] = fmt.Sprintf("%d", desiredWeight)

return ingressutil.NewLegacyIngress(desiredCanaryIngress), nil
}
Expand Down
3 changes: 2 additions & 1 deletion rollout/trafficrouting/plugin/rpc/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ type UpdateHashArgs struct {
type SetWeightAndVerifyWeightArgs struct {
Rollout v1alpha1.Rollout
DesiredWeight int32
WeightTotal int32
AdditionalDestinations []v1alpha1.WeightDestination
}

Expand Down Expand Up @@ -201,7 +202,7 @@ func (s *TrafficRouterRPCServer) SetWeight(args interface{}, resp *types.RpcErro
if !ok {
return fmt.Errorf("invalid args %s", args)
}
*resp = s.Impl.SetWeight(&setWeigthArgs.Rollout, setWeigthArgs.DesiredWeight, setWeigthArgs.AdditionalDestinations)
*resp = s.Impl.SetWeight(&setWeigthArgs.Rollout, setWeigthArgs.DesiredWeight, setWeigthArgs.WeightTotal, setWeigthArgs.AdditionalDestinations)
return nil
}

Expand Down
4 changes: 2 additions & 2 deletions rollout/trafficrouting/plugin/rpc/rpc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func TestPlugin(t *testing.T) {
err = plugin.SetHeaderRoute(&ro, &v1alpha1.SetHeaderRoute{})
assert.Equal(t, "", err.Error())

err = plugin.SetWeight(&ro, 0, []v1alpha1.WeightDestination{})
err = plugin.SetWeight(&ro, 0, 100, []v1alpha1.WeightDestination{})
assert.Equal(t, "", err.Error())

b, err := plugin.VerifyWeight(&ro, 0, []v1alpha1.WeightDestination{})
Expand Down Expand Up @@ -136,7 +136,7 @@ func TestPluginClosedConnection(t *testing.T) {
err = plugin.SetHeaderRoute(&v1alpha1.Rollout{}, &v1alpha1.SetHeaderRoute{})
assert.Contains(t, err.Error(), expectedError)

err = plugin.SetWeight(&v1alpha1.Rollout{}, 0, []v1alpha1.WeightDestination{})
err = plugin.SetWeight(&v1alpha1.Rollout{}, 0, 100, []v1alpha1.WeightDestination{})
assert.Contains(t, err.Error(), expectedError)

_, err = plugin.VerifyWeight(&v1alpha1.Rollout{}, 0, []v1alpha1.WeightDestination{})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (r *RpcPlugin) canaryIngress(ro *v1alpha1.Rollout, stableIngress *ingressut
}

// SetWeight modifies Nginx Ingress resources to reach desired state
func (r *RpcPlugin) SetWeight(ro *v1alpha1.Rollout, desiredWeight int32, additionalDestinations []v1alpha1.WeightDestination) pluginTypes.RpcError {
func (r *RpcPlugin) SetWeight(ro *v1alpha1.Rollout, desiredWeight int32, weightTotal int32, additionalDestinations []v1alpha1.WeightDestination) pluginTypes.RpcError {
ctx := context.TODO()

s := v1alpha1.NginxTrafficRouting{}
Expand Down
2 changes: 1 addition & 1 deletion utils/plugin/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type RpcTrafficRoutingReconciler interface {
// UpdateHash informs a traffic routing reconciler about new canary, stable, and additionalDestination(s) pod hashes
UpdateHash(rollout *v1alpha1.Rollout, canaryHash, stableHash string, additionalDestinations []v1alpha1.WeightDestination) RpcError
// SetWeight sets the canary weight to the desired weight
SetWeight(rollout *v1alpha1.Rollout, desiredWeight int32, additionalDestinations []v1alpha1.WeightDestination) RpcError
SetWeight(rollout *v1alpha1.Rollout, desiredWeight int32, weightTotal int32, additionalDestinations []v1alpha1.WeightDestination) RpcError
// SetHeaderRoute sets the header routing step
SetHeaderRoute(rollout *v1alpha1.Rollout, setHeaderRoute *v1alpha1.SetHeaderRoute) RpcError
// SetMirrorRoute sets up the traffic router to mirror traffic to a service
Expand Down

0 comments on commit 5c542f5

Please sign in to comment.