Skip to content

Commit

Permalink
[WRR] remove percentage (#953)
Browse files Browse the repository at this point in the history 
closes #930

remove percentage, using portions instead. see #930

Signed-off-by: kuritka <kuritka@gmail.com>

Signed-off-by: kuritka <kuritka@gmail.com>
  • Loading branch information
@kuritka
kuritka authored Oct 6, 2022
1 parent f76e9a6 commit 1c8cc73
Show file tree
Hide file tree
Showing 10 changed files with 66 additions and 140 deletions.
32 changes: 2 additions & 30 deletions api/v1beta1/gslb_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,6 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic
*/

import (
"strconv"
"strings"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

Expand All @@ -33,8 +30,8 @@ import (
type Strategy struct {
// Load balancing strategy type:(roundRobin|failover)
Type string `json:"type"`
// roundrobin and in the future consistent may (but also may not) contain a weight
Weight Weight `json:"weight,omitempty"`
// Weight is defined by map region:weight
Weight map[string]int `json:"weight,omitempty"`
// Primary Geo Tag. Valid for failover strategy only
PrimaryGeoTag string `json:"primaryGeoTag,omitempty"`
// Defines DNS record TTL in seconds
Expand Down Expand Up @@ -103,28 +100,3 @@ func (h HealthStatus) String() string {
func init() {
SchemeBuilder.Register(&Gslb{}, &GslbList{})
}

type Percentage string

type Weight map[string]Percentage

func (p Percentage) String() string {
return string(p)
}

func (p Percentage) IsEmpty() bool {
return string(p) == ""
}

func (p Percentage) TryParse() (v int, err error) {
return strconv.Atoi(strings.TrimSuffix(strings.ReplaceAll(p.String(), " ", ""), "%"))
}

func (p Percentage) Int() int {
v, _ := p.TryParse()
return v
}

func (w Weight) IsEmpty() bool {
return len(w) == 0
}
23 changes: 1 addition & 22 deletions api/v1beta1/zz_generated.deepcopy.go
View file

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

5 changes: 2 additions & 3 deletions chart/k8gb/crd/k8gb.absa.oss_gslbs.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,8 @@ spec:
type: string
weight:
additionalProperties:
type: string
description: roundrobin and in the future consistent may (but
also may not) contain a weight
type: integer
description: Weight is defined by map region:weight
type: object
required:
- type
Expand Down
14 changes: 2 additions & 12 deletions controllers/depresolver/depresolver_spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,29 +69,19 @@ func (dr *DependencyResolver) validateSpec(strategy k8gbv1beta1.Strategy) (err e
if err != nil {
return
}
if !strategy.Weight.IsEmpty() {
if len(strategy.Weight) != 0 {
if strategy.Type != RoundRobinStrategy {
return fmt.Errorf(`weight is allowed only for roundRobin strategy`)
}
summary := 0
for k, w := range strategy.Weight {
var wi int
err = field("weight", k).isNotEmpty().matchRegexp(geoTagRegex).err
if err != nil {
return err
}
wi, err = w.TryParse()
if err != nil {
return fmt.Errorf("weight invalid value %s", w.String())
}
err = field("weight", wi).isHigherOrEqualToZero().isLessOrEqualTo(100).err
err = field("weight", w).isHigherOrEqualToZero().isLessOrEqualTo(1000).err
if err != nil {
return err
}
summary += wi
}
if summary != 100 {
return fmt.Errorf("weight the sum of the weights must be equal to 100")
}
}
return nil
Expand Down
80 changes: 33 additions & 47 deletions controllers/depresolver/depresolver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ func TestResolveSpecWithFilledFields(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, 35, gslb.Spec.Strategy.DNSTtlSeconds)
assert.Equal(t, 305, gslb.Spec.Strategy.SplitBrainThresholdSeconds)
assert.False(t, gslb.Spec.Strategy.Weight.IsEmpty())
assert.Equal(t, 15, gslb.Spec.Strategy.Weight["za"].Int())
assert.Equal(t, 25, gslb.Spec.Strategy.Weight["eu"].Int())
assert.Equal(t, 60, gslb.Spec.Strategy.Weight["us"].Int())
assert.False(t, len(gslb.Spec.Strategy.Weight) == 0)
assert.Equal(t, 15, gslb.Spec.Strategy.Weight["za"])
assert.Equal(t, 25, gslb.Spec.Strategy.Weight["eu"])
assert.Equal(t, 60, gslb.Spec.Strategy.Weight["us"])
assert.Equal(t, 3, len(gslb.Spec.Strategy.Weight))
}

Expand All @@ -115,7 +115,7 @@ func TestResolveSpecWithoutFields(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, predefinedStrategy.DNSTtlSeconds, gslb.Spec.Strategy.DNSTtlSeconds)
assert.Equal(t, predefinedStrategy.SplitBrainThresholdSeconds, gslb.Spec.Strategy.SplitBrainThresholdSeconds)
assert.True(t, gslb.Spec.Strategy.Weight.IsEmpty())
assert.True(t, len(gslb.Spec.Strategy.Weight) == 0)
}

func TestResolveSpecWithZeroSplitBrain(t *testing.T) {
Expand Down Expand Up @@ -181,55 +181,41 @@ func TestResolveSpecWithType(t *testing.T) {
}

func TestResolveSpecWithWeightCornerCases(t *testing.T) {
type test struct {
strategy string
weight k8gbv1beta1.Weight
expectederr bool
empty bool
ignoreSumValidation bool
}
tests := []test{
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "-20%"}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"eu": "-20"}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"za": "+20"}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": " 20% "}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "20%"}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "20"}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "100%"}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "101"}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "101%"}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "0"}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "0%"}, false, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": ""}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "%"}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us": "blah"}, true, false, true},
{FailoverStrategy, map[string]k8gbv1beta1.Percentage{"us": "10%"}, true, false, true},
{GeoStrategy, map[string]k8gbv1beta1.Percentage{"us": "0%"}, true, false, true},
{GeoStrategy, nil, false, true, true},
{RoundRobinStrategy, nil, false, true, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"": ""}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"": "20%"}, true, false, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{}, false, true, true},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us-1": "100%", "us-2": "0%"}, false, false, false},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us-1": "100%", "us-2": "100%"}, true, false, false},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us-1": "0%", "us-2": "0%"}, true, false, false},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us-1": "50 %", "eu": "20%", "za": "30"}, false, false, false},
{RoundRobinStrategy, map[string]k8gbv1beta1.Percentage{"us-1": "50 %", "0": "20%", "-": "-"}, true, false, false},
tests := []struct {
strategy string
weight map[string]int
err bool
empty bool
}{
{RoundRobinStrategy, map[string]int{"us": -20}, true, false},
{RoundRobinStrategy, map[string]int{"za": 2}, false, false},
{RoundRobinStrategy, map[string]int{"us": 1000}, false, false},
{RoundRobinStrategy, map[string]int{"us": 0}, false, false},

{GeoStrategy, map[string]int{"za": 20}, true, false},
{GeoStrategy, map[string]int{"us": 0}, true, false},
{GeoStrategy, map[string]int{}, false, true},
{GeoStrategy, nil, false, true},

{RoundRobinStrategy, map[string]int{}, false, true},
{RoundRobinStrategy, map[string]int{"us-1": 100, "us-2": 0}, false, false},
{RoundRobinStrategy, map[string]int{"us-1": 100, "us-2": 100}, false, false},
{RoundRobinStrategy, map[string]int{"us-1": 10000, "us-2": 100}, true, false},
{RoundRobinStrategy, map[string]int{"us-1": 0, "us-2": 0}, false, false},
{RoundRobinStrategy, map[string]int{"us-1": 50, "eu": 20, "za": 0}, false, false},
}
// arrange
cl, gslb := getTestContext("./testdata/filled_omitempty.yaml")
resolver := NewDependencyResolver()
// act
for _, tt := range tests {
t.Run(fmt.Sprintf("weight_%s_%s", tt.strategy, tt.weight), func(t *testing.T) {
gslb.Spec.Strategy.Weight = tt.weight
gslb.Spec.Strategy.Type = tt.strategy
for _, test := range tests {
t.Run(fmt.Sprintf("weight_%s_%v", test.strategy, test.weight), func(t *testing.T) {
gslb.Spec.Strategy.Weight = test.weight
gslb.Spec.Strategy.Type = test.strategy
err := resolver.ResolveGslbSpec(context.TODO(), gslb, cl)
// assert
assert.Equal(t, tt.empty, gslb.Spec.Strategy.Weight.IsEmpty())
if !(tt.ignoreSumValidation && err != nil && strings.Contains(err.Error(), `the sum of the weights must be equal to 100`)) {
assert.True(t, (err != nil) == tt.expectederr)
}
assert.Equal(t, test.empty, len(gslb.Spec.Strategy.Weight) == 0)
assert.True(t, (err != nil) == test.err)
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions controllers/depresolver/testdata/filled_omitempty.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ spec:
splitBrainThresholdSeconds: 305
dnsTtlSeconds: 35
weight:
us: 60%
eu: 25%
za: 15%
us: 60
eu: 25
za: 15

2 changes: 1 addition & 1 deletion controllers/dnsupdate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func (r *GslbReconciler) getLabels(gslb *k8gbv1beta1.Gslb, targets assistant.Tar
continue
}
for i, ip := range t.IPs {
l := fmt.Sprintf("weight-%s-%v-%v", k, i, v.Int())
l := fmt.Sprintf("weight-%s-%v-%v", k, i, v)
labels[l] = ip
}
}
Expand Down
36 changes: 18 additions & 18 deletions controllers/weight_test.go → controllers/gslb_controller_weight_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ Generated by GoLic, for more details see: https://github.com/AbsaOSS/golic
func TestWeight(t *testing.T) {
// arrange
type wrr struct {
weight string
weight int
targets []string
}
var tests = []struct {
Expand All @@ -51,9 +51,9 @@ func TestWeight(t *testing.T) {
name: "eu35-us50-za15",
injectWeights: true,
data: map[string]wrr{
"eu": {weight: "35%", targets: []string{"10.10.0.1", "10.10.0.2"}},
"us": {weight: "50%", targets: []string{"10.0.0.1", "10.0.0.2"}},
"za": {weight: "15%", targets: []string{"10.22.0.1", "10.22.0.2", "10.22.1.1"}},
"eu": {weight: 35, targets: []string{"10.10.0.1", "10.10.0.2"}},
"us": {weight: 50, targets: []string{"10.0.0.1", "10.0.0.2"}},
"za": {weight: 15, targets: []string{"10.22.0.1", "10.22.0.2", "10.22.1.1"}},
},
expectedLabels: map[string]string{
"strategy": depresolver.RoundRobinStrategy,
Expand All @@ -70,9 +70,9 @@ func TestWeight(t *testing.T) {
name: "eu100-us0-za0",
injectWeights: true,
data: map[string]wrr{
"eu": {weight: "100%", targets: []string{"10.10.0.1", "10.10.0.2"}},
"us": {weight: "0%", targets: []string{"10.0.0.1", "10.0.0.2"}},
"za": {weight: "0%", targets: []string{"10.22.0.1", "10.22.0.2", "10.22.1.1"}},
"eu": {weight: 100, targets: []string{"10.10.0.1", "10.10.0.2"}},
"us": {weight: 0, targets: []string{"10.0.0.1", "10.0.0.2"}},
"za": {weight: 0, targets: []string{"10.22.0.1", "10.22.0.2", "10.22.1.1"}},
},
expectedLabels: map[string]string{
"strategy": depresolver.RoundRobinStrategy,
Expand All @@ -89,9 +89,9 @@ func TestWeight(t *testing.T) {
name: "weights-without-external-targets",
injectWeights: true,
data: map[string]wrr{
"eu": {weight: "25%", targets: []string{}},
"us": {weight: "75%", targets: []string{}},
"za": {weight: "0%", targets: []string{}},
"eu": {weight: 25, targets: []string{}},
"us": {weight: 75, targets: []string{}},
"za": {weight: 0, targets: []string{}},
},
expectedLabels: map[string]string{
"strategy": depresolver.RoundRobinStrategy,
Expand All @@ -101,9 +101,9 @@ func TestWeight(t *testing.T) {
name: "weights-without-external-targets",
injectWeights: true,
data: map[string]wrr{
"eu": {weight: "25%", targets: []string{}},
"us": {weight: "75%", targets: []string{}},
"za": {weight: "0%", targets: []string{}},
"eu": {weight: 25, targets: []string{}},
"us": {weight: 75, targets: []string{}},
"za": {weight: 0, targets: []string{}},
},
expectedLabels: map[string]string{
"strategy": depresolver.RoundRobinStrategy,
Expand All @@ -121,9 +121,9 @@ func TestWeight(t *testing.T) {
name: "no weights with external targets",
injectWeights: false,
data: map[string]wrr{
"eu": {weight: "100%", targets: []string{"10.10.0.1", "10.10.0.2"}},
"us": {weight: "0%", targets: []string{"10.0.0.1", "10.0.0.2"}},
"za": {weight: "0%", targets: []string{"10.22.0.1", "10.22.0.2", "10.22.1.1"}},
"eu": {weight: 100, targets: []string{"10.10.0.1", "10.10.0.2"}},
"us": {weight: 0, targets: []string{"10.0.0.1", "10.0.0.2"}},
"za": {weight: 0, targets: []string{"10.22.0.1", "10.22.0.2", "10.22.1.1"}},
},
expectedLabels: map[string]string{
"strategy": depresolver.RoundRobinStrategy,
Expand All @@ -147,9 +147,9 @@ func TestWeight(t *testing.T) {
if !test.injectWeights {
return nil
}
gslb.Spec.Strategy.Weight = make(map[string]k8gbv1beta1.Percentage, 0)
gslb.Spec.Strategy.Weight = make(map[string]int, 0)
for k, w := range test.data {
gslb.Spec.Strategy.Weight[k] = k8gbv1beta1.Percentage(w.weight)
gslb.Spec.Strategy.Weight[k] = w.weight
}
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions terratest/examples/roundrobin_weight1.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ spec:
type: roundRobin # Use a round robin load balancing strategy, when deciding which downstream clusters to route clients too
dnsTtlSeconds: 5
weight:
eu: 50%
us: 50%
eu: 5
us: 5
4 changes: 2 additions & 2 deletions terratest/test/k8gb_weight_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,8 @@ func TestWeightsExistsInLocalDNSEndpoint(t *testing.T) {
require.NoError(t, err, "missing endpoint", host)
// check all labels are correct
require.Equal(t, "roundRobin", ep.Labels["strategy"])
require.NotEqual(t, ep.Labels["weight-eu-0-50"], ep.Labels["weight-eu-1-50"])
require.NotEqual(t, ep.Labels["weight-us-0-50"], ep.Labels["weight-us-1-50"])
require.NotEqual(t, ep.Labels["weight-eu-0-5"], ep.Labels["weight-eu-1-5"])
require.NotEqual(t, ep.Labels["weight-us-0-5"], ep.Labels["weight-us-1-5"])
// check all targets are correct
for _, v := range epExternalEU.Targets {
require.True(t, Contains(v, ep.Targets))
Expand Down

0 comments on commit 1c8cc73

Please sign in to comment.