Skip to content

Commit

Permalink
Merge pull request oracle#398 in OKE/oci-cloud-controller-manager fro…
Browse files Browse the repository at this point in the history
…m task/OKE-21439-1.24 to release-1.24

* commit '43e496764d380967c94c785c64d2718d896966e3':
  Make lb flex shape value formats consistent for all lb shape related annotations
  • Loading branch information
vbhargav875 committed Oct 12, 2022
2 parents cc65220 + 43e4967 commit af9f006
Show file tree
Hide file tree
Showing 4 changed files with 118 additions and 29 deletions.
10 changes: 4 additions & 6 deletions pkg/cloudprovider/providers/oci/load_balancer_spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -1055,15 +1055,13 @@ func getLBShape(svc *v1.Service) (string, *int, *int, error) {
)
}

flexShapeMinMbps, err := strconv.Atoi(flexMinS)
flexShapeMinMbps, err := parseFlexibleShapeBandwidth(flexMinS, ServiceAnnotationLoadBalancerShapeFlexMin)
if err != nil {
return "", nil, nil, errors.Wrap(err,
fmt.Sprintf("The annotation %s should contain only integer value", ServiceAnnotationLoadBalancerShapeFlexMin))
return "", nil, nil, err
}
flexShapeMaxMbps, err = strconv.Atoi(flexMaxS)
flexShapeMaxMbps, err = parseFlexibleShapeBandwidth(flexMaxS, ServiceAnnotationLoadBalancerShapeFlexMax)
if err != nil {
return "", nil, nil, errors.Wrap(err,
fmt.Sprintf("The annotation %s should contain only integer value", ServiceAnnotationLoadBalancerShapeFlexMax))
return "", nil, nil, err
}
if flexShapeMinMbps < 10 {
flexShapeMinMbps = 10
Expand Down
23 changes: 0 additions & 23 deletions pkg/cloudprovider/providers/oci/load_balancer_spec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2200,29 +2200,6 @@ func TestNewLBSpecFailure(t *testing.T) {
},
expectedErrMsg: "error parsing service annotation: service.beta.kubernetes.io/oci-load-balancer-shape=flexible requires service.beta.kubernetes.io/oci-load-balancer-shape-flex-min and service.beta.kubernetes.io/oci-load-balancer-shape-flex-max to be set",
},
"invalid flex shape non int min/max": {
defaultSubnetOne: "one",
defaultSubnetTwo: "two",
service: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "kube-system",
Name: "testservice",
UID: "test-uid",
Annotations: map[string]string{
ServiceAnnotationLoadBalancerShape: "flexible",
ServiceAnnotationLoadBalancerShapeFlexMin: "10Mbps",
ServiceAnnotationLoadBalancerShapeFlexMax: "100Mbps",
},
},
Spec: v1.ServiceSpec{
SessionAffinity: v1.ServiceAffinityNone,
Ports: []v1.ServicePort{
{Protocol: v1.ProtocolTCP},
},
},
},
expectedErrMsg: `The annotation service.beta.kubernetes.io/oci-load-balancer-shape-flex-min should contain only integer value: strconv.Atoi: parsing "10Mbps": invalid syntax`,
},
"invalid loadbalancer policy": {
defaultSubnetOne: "one",
defaultSubnetTwo: "two",
Expand Down
18 changes: 18 additions & 0 deletions pkg/cloudprovider/providers/oci/load_balancer_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"os"
"regexp"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -62,6 +63,8 @@ const (
Delete = "delete"
)

const nonAlphanumericRegexExpression = "[^a-zA-Z0-9]+"

// Action that should take place on the resource.
type Action interface {
Type() ActionType
Expand Down Expand Up @@ -691,3 +694,18 @@ func getMetric(lbtype string, metricType string) string {
}
return ""
}

func parseFlexibleShapeBandwidth(shape, annotation string) (int, error) {
reg, _ := regexp.Compile(nonAlphanumericRegexExpression)
processedString := reg.ReplaceAllString(shape, "")
if strings.HasSuffix(processedString, "Mbps") {
processedString = strings.TrimSuffix(processedString, "Mbps")
} else if strings.HasSuffix(processedString, "mbps") {
processedString = strings.TrimSuffix(processedString, "mbps")
}
parsedIntFlexibleShape, err := strconv.Atoi(processedString)
if err != nil {
return 0, fmt.Errorf("invalid format for %s annotation : %v", annotation, shape)
}
return parsedIntFlexibleShape, nil
}
96 changes: 96 additions & 0 deletions pkg/cloudprovider/providers/oci/load_balancer_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1898,3 +1898,99 @@ func TestHasLoadBalancerNetworkSecurityGroupsChanged(t *testing.T) {
})
}
}

func Test_parseFlexibleShapeBandwidth(t *testing.T) {
var testCases = []struct {
name string
annotation string
inputShape string
expectedOutputShape int
errMessage string
wantErr bool
}{
{
name: "Valid Flexible Shape Min - Only integer",
annotation: ServiceAnnotationLoadBalancerShapeFlexMin,
inputShape: "100",
expectedOutputShape: 100,
errMessage: "",
wantErr: false,
},
{
name: "Valid Flexible Shape Max - Only integer",
annotation: ServiceAnnotationLoadBalancerShapeFlexMax,
inputShape: "8000",
expectedOutputShape: 8000,
errMessage: "",
wantErr: false,
},
{
name: "Valid Flexible Shape Min - With Unit (Uppercase M)",
annotation: ServiceAnnotationLoadBalancerShapeFlexMin,
inputShape: "100Mbps",
expectedOutputShape: 100,
errMessage: "",
wantErr: false,
},
{
name: "Valid Flexible Shape Max - With Unit (Uppercase M)",
annotation: ServiceAnnotationLoadBalancerShapeFlexMax,
inputShape: "8000Mbps",
expectedOutputShape: 8000,
errMessage: "",
wantErr: false,
},
{
name: "Valid Flexible Shape Min - With Unit (Lowercase m)",
annotation: ServiceAnnotationLoadBalancerShapeFlexMin,
inputShape: "100mbps",
expectedOutputShape: 100,
errMessage: "",
wantErr: false,
},
{
name: "Valid Flexible Shape Max - With Unit (Lowercase m)",
annotation: ServiceAnnotationLoadBalancerShapeFlexMax,
inputShape: "8000mbps",
expectedOutputShape: 8000,
errMessage: "",
wantErr: false,
},
{
name: "Invalid Flexible Shape Min",
annotation: ServiceAnnotationLoadBalancerShapeFlexMin,
inputShape: "100kbps",
expectedOutputShape: 0,
errMessage: "invalid format for service.beta.kubernetes.io/oci-load-balancer-shape-flex-min annotation : 100kbps",
wantErr: true,
},
{
name: "Invalid Flexible Shape Max",
annotation: ServiceAnnotationLoadBalancerShapeFlexMax,
inputShape: "8000kbps",
expectedOutputShape: 0,
errMessage: "invalid format for service.beta.kubernetes.io/oci-load-balancer-shape-flex-max annotation : 8000kbps",
wantErr: true,
},
}

for _, tt := range testCases {
t.Run(tt.name, func(t *testing.T) {
parsedShape, err := parseFlexibleShapeBandwidth(tt.inputShape, tt.annotation)
if err != nil {
if !tt.wantErr {
t.Errorf("parseFlexibleShapeBandwidth() error = %v, wantErr = %v", err, tt.wantErr)
}
if !reflect.DeepEqual(err.Error(), tt.errMessage) {
t.Errorf("parseFlexibleShapeBandwidth() got errMessage = %v, want errMessage = %v", err.Error(), tt.errMessage)
}
}
if err == nil && tt.wantErr {
t.Errorf("parseFlexibleShapeBandwidth() error = %v, wantErr = %v", err, tt.wantErr)
}
if err == nil && parsedShape != tt.expectedOutputShape {
t.Errorf("parseFlexibleShapeBandwidth() got = %v, want = %v", parsedShape, tt.expectedOutputShape)
}
})
}
}

0 comments on commit af9f006

Please sign in to comment.