Skip to content

Commit

Permalink
Fixes #104
Browse files Browse the repository at this point in the history 
exposed outlierDetection configuration through GTP

Signed-off-by: Shriram Sharma <shriram_sharma@intuit.com>
  • Loading branch information
@shriramsharma
shriramsharma committed May 13, 2022
1 parent 35de918 commit 05f3325
Show file tree
Hide file tree
Showing 5 changed files with 212 additions and 65 deletions.
130 changes: 103 additions & 27 deletions admiral/pkg/apis/admiral/model/globalrouting.pb.go
View file

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

13 changes: 13 additions & 0 deletions admiral/pkg/apis/admiral/model/globalrouting.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,19 @@ message TrafficPolicy {
//REQUIRED: dnsPrefix that will be prefixed for the service names being generated with this traffic policy
//Ex: dnsPrefix = west => generated service name = west.stage.servicename.global
string dnsPrefix = 4;

message FailoverConfiguration {
//REQUIRED: Minimum duration of time in seconds, the endpoint will be ejected
int64 base_ejection_time = 1;
//REQUIRED: No. of consecutive failures in specified interval after which the endpoint will be ejected
uint32 consecutive_gateway_errors = 2;
//REQUIRED: Time interval between ejection sweep analysis
int64 interval = 3;
}

//OPTIONAL: to configure the outlierDetection in DestinationRule
FailoverConfiguration failover_configuration = 5;

}

message TrafficGroup {
Expand Down
16 changes: 14 additions & 2 deletions admiral/pkg/clusters/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,21 @@ func getDestinationRule(se *v1alpha32.ServiceEntry, locality string, gtpTrafficP
func getOutlierDetection(se *v1alpha32.ServiceEntry, locality string, gtpTrafficPolicy *model.TrafficPolicy) *v1alpha32.OutlierDetection {

outlierDetection := &v1alpha32.OutlierDetection{
BaseEjectionTime: &types.Duration{Seconds: 300},
BaseEjectionTime: &types.Duration{Seconds: 300},
ConsecutiveGatewayErrors: &types.UInt32Value{Value: uint32(50)},
Interval: &types.Duration{Seconds: 60},
Interval: &types.Duration{Seconds: 60},
}

if gtpTrafficPolicy != nil && gtpTrafficPolicy.FailoverConfiguration != nil {
outlierDetection.BaseEjectionTime = &types.Duration{
Seconds: gtpTrafficPolicy.FailoverConfiguration.BaseEjectionTime,
}
outlierDetection.ConsecutiveGatewayErrors = &types.UInt32Value{
Value: gtpTrafficPolicy.FailoverConfiguration.ConsecutiveGatewayErrors,
}
outlierDetection.Interval = &types.Duration{
Seconds: gtpTrafficPolicy.FailoverConfiguration.Interval,
}
}

//Scenario 1: Only one endpoint present and is local service (ends in svc.cluster.local) - no outlier detection (optimize this for headless services in future?)
Expand Down
116 changes: 81 additions & 35 deletions admiral/pkg/clusters/handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,15 +175,14 @@ func TestIgnoreIstioResource(t *testing.T) {
func TestGetDestinationRule(t *testing.T) {
//Do setup here
outlierDetection := &v1alpha3.OutlierDetection{
BaseEjectionTime: &types.Duration{Seconds: 300},
BaseEjectionTime: &types.Duration{Seconds: 300},
ConsecutiveGatewayErrors: &types.UInt32Value{Value: 50},
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 100,
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 100,
}
mTLS := &v1alpha3.TrafficPolicy{Tls: &v1alpha3.TLSSettings{Mode: v1alpha3.TLSSettings_ISTIO_MUTUAL}, OutlierDetection: outlierDetection}


se := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"},Endpoints:[]*v1alpha3.ServiceEntry_Endpoint{
se := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"}, Endpoints: []*v1alpha3.ServiceEntry_Endpoint{
{Address: "east.com", Locality: "us-east-2"}, {Address: "west.com", Locality: "us-west-2"},
}}
noGtpDr := v1alpha3.DestinationRule{
Expand Down Expand Up @@ -298,17 +297,17 @@ func TestGetDestinationRule(t *testing.T) {
func TestGetOutlierDetection(t *testing.T) {
//Do setup here
outlierDetection := &v1alpha3.OutlierDetection{
BaseEjectionTime: &types.Duration{Seconds: 300},
BaseEjectionTime: &types.Duration{Seconds: 300},
ConsecutiveGatewayErrors: &types.UInt32Value{Value: 50},
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 100,
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 100,
}

outlierDetectionOneHostRemote := &v1alpha3.OutlierDetection{
BaseEjectionTime: &types.Duration{Seconds: 300},
BaseEjectionTime: &types.Duration{Seconds: 300},
ConsecutiveGatewayErrors: &types.UInt32Value{Value: 50},
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 34,
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 34,
}

topologyGTPPolicy := &model.TrafficPolicy{
Expand All @@ -321,58 +320,105 @@ func TestGetOutlierDetection(t *testing.T) {
},
}

se := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"},Endpoints:[]*v1alpha3.ServiceEntry_Endpoint{
se := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"}, Endpoints: []*v1alpha3.ServiceEntry_Endpoint{
{Address: "east.com", Locality: "us-east-2"}, {Address: "west.com", Locality: "us-west-2"},
}}

seOneHostRemote := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"},Endpoints:[]*v1alpha3.ServiceEntry_Endpoint{
seOneHostRemote := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"}, Endpoints: []*v1alpha3.ServiceEntry_Endpoint{
{Address: "east.com", Locality: "us-east-2"},
}}

seOneHostLocal := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"},Endpoints:[]*v1alpha3.ServiceEntry_Endpoint{
seOneHostLocal := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"}, Endpoints: []*v1alpha3.ServiceEntry_Endpoint{
{Address: "hello.ns.svc.cluster.local", Locality: "us-east-2"},
}}

seOneHostRemoteIp := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"},Endpoints:[]*v1alpha3.ServiceEntry_Endpoint{
seOneHostRemoteIp := &v1alpha3.ServiceEntry{Hosts: []string{"qa.myservice.global"}, Endpoints: []*v1alpha3.ServiceEntry_Endpoint{
{Address: "95.45.25.34", Locality: "us-east-2"},
}}

//Struct of test case info. Name is required.
testCases := []struct {
name string
se *v1alpha3.ServiceEntry
locality string
gtpPolicy *model.TrafficPolicy
name string
se *v1alpha3.ServiceEntry
locality string
gtpPolicy *model.TrafficPolicy
outlierDetection *v1alpha3.OutlierDetection
}{
{
name: "Should return nil for cluster local only endpoint",
se: seOneHostLocal,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
name: "Should return nil for cluster local only endpoint",
se: seOneHostLocal,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
outlierDetection: nil,
},
{
name: "Should return nil for one IP endpoint",
se: seOneHostRemoteIp,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
name: "Should return nil for one IP endpoint",
se: seOneHostRemoteIp,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
outlierDetection: nil,
},
{
name: "Should return 34% ejection for remote endpoint with one entry",
se: seOneHostRemote,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
name: "Should return 34% ejection for remote endpoint with one entry",
se: seOneHostRemote,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
outlierDetection: outlierDetectionOneHostRemote,
},
{
name: "Should return 100% ejection for two remote endpoints",
se: se,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
name: "Should return 100% ejection for two remote endpoints",
se: se,
locality: "uswest2",
gtpPolicy: topologyGTPPolicy,
outlierDetection: outlierDetection,
},
{
name: "Should use the default outlier detection if gtpPolicy is nil",
se: se,
locality: "uswest2",
gtpPolicy: nil,
outlierDetection: outlierDetection,
},
{
name: "Should use the default outlier detection if FailoverConfiguration is nil inside gtpPolicy",
se: se,
locality: "uswest2",
gtpPolicy: &model.TrafficPolicy{
LbType: model.TrafficPolicy_TOPOLOGY,
Target: []*model.TrafficGroup{
{
Region: "us-west-2",
Weight: 100,
},
},
},
outlierDetection: outlierDetection,
},
{
name: "Default outlier detection config should be overriden by the failover config specified in the TrafficPolicy",
se: se,
locality: "uswest2",
gtpPolicy: &model.TrafficPolicy{
LbType: model.TrafficPolicy_TOPOLOGY,
Target: []*model.TrafficGroup{
{
Region: "us-west-2",
Weight: 100,
},
},
FailoverConfiguration: &model.TrafficPolicy_FailoverConfiguration{
BaseEjectionTime: 600,
ConsecutiveGatewayErrors: 10,
Interval: 60,
},
},
outlierDetection: &v1alpha3.OutlierDetection{
BaseEjectionTime: &types.Duration{Seconds: 600},
ConsecutiveGatewayErrors: &types.UInt32Value{Value: 10},
Interval: &types.Duration{Seconds: 60},
MaxEjectionPercent: 100,
},
},
}

//Run the test for every provided case
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ require (
github.com/onsi/ginkgo v1.10.2 // indirect
github.com/onsi/gomega v1.7.0
github.com/prometheus/client_golang v1.5.0
github.com/prometheus/client_model v0.2.0 // indirect
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.9.1
github.com/sirupsen/logrus v1.4.2
github.com/spf13/cobra v0.0.5
Expand Down

0 comments on commit 05f3325

Please sign in to comment.