Skip to content

Commit

Permalink
enable warm duration through runtime param (#377)
Browse files Browse the repository at this point in the history 
enable warm duration through runtime param

Signed-off-by: Shriram Sharma <[email protected]>
  • Loading branch information
@shriramsharma
shriramsharma authored Jan 16, 2025
1 parent b642331 commit 8226882
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 11 deletions.
1 change: 1 addition & 0 deletions admiral/cmd/admiral/cmd/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ func GetRootCmd(args []string) *cobra.Command {
// Flags pertaining to VS based routing
rootCmd.PersistentFlags().BoolVar(&params.EnableVSRouting, "enable_vs_routing", false, "Enable/Disable VS Based Routing")
rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingEnabledClusters, "vs_routing_enabled_clusters", []string{}, "The source clusters to enable VS based routing on")
rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingSlowStartEnabledClusters, "vs_routing_slow_start_enabled_clusters", []string{}, "The source clusters to where VS routing is enabled and require slow start")
rootCmd.PersistentFlags().StringSliceVar(&params.VSRoutingGateways, "vs_routing_gateways", []string{}, "The PASSTHROUGH gateways to use for VS based routing")
rootCmd.PersistentFlags().StringSliceVar(&params.IngressVSExportToNamespaces, "ingress_vs_export_to_namespaces", []string{"istio-system"}, "List of namespaces where the ingress VS should be exported")
rootCmd.PersistentFlags().StringVar(&params.IngressLBPolicy, "ingress_lb_policy", "round_robin", "loadbalancer policy for ingress destination rule (round_robin/random/passthrough/least_request)")
Expand Down
7 changes: 6 additions & 1 deletion admiral/pkg/clusters/virtualservice_routing.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -776,13 +776,18 @@ func addUpdateDestinationRuleForSourceIngress(
LocalityLbSetting: &networkingV1Alpha3.LocalityLoadBalancerSetting{
Enabled: &wrappers.BoolValue{Value: false},
},
WarmupDurationSecs: &duration.Duration{Seconds: common.GetDefaultWarmupDurationSecs()},
},
Tls: &networkingV1Alpha3.ClientTLSSettings{
SubjectAltNames: []string{san},
},
},
}

if common.IsSlowStartEnabledForCluster(sourceCluster) {
drObj.TrafficPolicy.LoadBalancer.WarmupDurationSecs =
&duration.Duration{Seconds: common.GetDefaultWarmupDurationSecs()}
}

drName := fmt.Sprintf("%s-routing-dr", name)

newDR := createDestinationRuleSkeleton(drObj, drName, util.IstioSystemNamespace)
Expand Down
59 changes: 54 additions & 5 deletions admiral/pkg/clusters/virtualservice_routing_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2378,10 +2378,11 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
}

admiralParams := common.AdmiralParams{
SANPrefix: "test-san-prefix",
IngressVSExportToNamespaces: []string{"istio-system"},
EnableVSRouting: true,
VSRoutingEnabledClusters: []string{"cluster-1"},
SANPrefix: "test-san-prefix",
IngressVSExportToNamespaces: []string{"istio-system"},
EnableVSRouting: true,
VSRoutingEnabledClusters: []string{"cluster-1", "cluster-2"},
VSRoutingSlowStartEnabledClusters: []string{"cluster-1"},
}
common.ResetSync()
common.InitializeConfig(admiralParams)
Expand All @@ -2401,6 +2402,7 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
testCases := []struct {
name string
istioClient *istioFake.Clientset
drName string
sourceClusterToDRHosts map[string]map[string]string
sourceIdentity string
expectedError error
Expand All @@ -2410,6 +2412,7 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
name: "Given a empty sourceIdentity " +
"When addUpdateDestinationRuleForSourceIngress is invoked, " +
"Then it should return an error",
drName: "test-ns.svc.cluster.local-routing-dr",
sourceClusterToDRHosts: map[string]map[string]string{
"cluster-1": {
"test-ns.svc.cluster.local": "*.test-ns.svc.cluster.local",
Expand All @@ -2422,6 +2425,7 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
name: "Given a valid sourceClusterToDRHosts " +
"When addUpdateDestinationRuleForSourceIngress is invoked, " +
"Then it should create the destination rules",
drName: "test-ns.svc.cluster.local-routing-dr",
sourceIdentity: "test-identity",
sourceClusterToDRHosts: map[string]map[string]string{
"cluster-1": {
Expand Down Expand Up @@ -2459,6 +2463,7 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
name: "Given a valid sourceClusterToDRHosts " +
"When addUpdateDestinationRuleForSourceIngress is invoked, " +
"Then it should create the destination rules",
drName: "test-ns.svc.cluster.local-routing-dr",
sourceIdentity: "test-identity",
sourceClusterToDRHosts: map[string]map[string]string{
"cluster-1": {
Expand Down Expand Up @@ -2492,6 +2497,44 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
},
},
},
{
name: "Given a valid sourceClusterToDRHosts " +
"When addUpdateDestinationRuleForSourceIngress is invoked," +
"And the cluster is not enabled with slow start " +
"Then it should create the destination rules without warmupDurationSecs",
drName: "test-ns2.svc.cluster.local-routing-dr",
sourceIdentity: "test-identity",
sourceClusterToDRHosts: map[string]map[string]string{
"cluster-2": {
"test-ns2.svc.cluster.local": "*.test-ns2.svc.cluster.local",
},
},
istioClient: istioClientWithExistingDR,
expectedError: nil,
expectedDestinationRules: &apiNetworkingV1Alpha3.DestinationRule{
ObjectMeta: metaV1.ObjectMeta{
Name: "test-ns2.svc.cluster.local-routing-dr",
Namespace: util.IstioSystemNamespace,
},
Spec: networkingV1Alpha3.DestinationRule{
Host: "*.test-ns2.svc.cluster.local",
ExportTo: []string{util.IstioSystemNamespace},
TrafficPolicy: &networkingV1Alpha3.TrafficPolicy{
LoadBalancer: &networkingV1Alpha3.LoadBalancerSettings{
LbPolicy: &networkingV1Alpha3.LoadBalancerSettings_Simple{
Simple: networkingV1Alpha3.LoadBalancerSettings_ROUND_ROBIN,
},
LocalityLbSetting: &networkingV1Alpha3.LocalityLoadBalancerSetting{
Enabled: &wrappers.BoolValue{Value: false},
},
},
Tls: &networkingV1Alpha3.ClientTLSSettings{
SubjectAltNames: []string{"spiffe://test-san-prefix/test-identity"},
},
},
},
},
},
}

for _, tc := range testCases {
Expand All @@ -2500,8 +2543,14 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
ClusterID: "cluster-1",
DestinationRuleController: &istio.DestinationRuleController{},
}
rc2 := &RemoteController{
ClusterID: "cluster-2",
DestinationRuleController: &istio.DestinationRuleController{},
}
rc.DestinationRuleController.IstioClient = tc.istioClient
rc2.DestinationRuleController.IstioClient = tc.istioClient
rr.PutRemoteController("cluster-1", rc)
rr.PutRemoteController("cluster-2", rc2)

err := addUpdateDestinationRuleForSourceIngress(
context.Background(),
Expand All @@ -2514,7 +2563,7 @@ func TestAaddUpdateDestinationRuleForSourceIngress(t *testing.T) {
require.Equal(t, tc.expectedError.Error(), err.Error())
} else {
actualDR, err := tc.istioClient.NetworkingV1alpha3().DestinationRules(util.IstioSystemNamespace).
Get(context.Background(), "test-ns.svc.cluster.local-routing-dr", metaV1.GetOptions{})
Get(context.Background(), tc.drName, metaV1.GetOptions{})
require.Nil(t, err)
require.Equal(t, tc.expectedDestinationRules.Spec.Host, actualDR.Spec.Host)
require.Equal(t, tc.expectedDestinationRules.Spec.TrafficPolicy, actualDR.Spec.TrafficPolicy)
Expand Down
17 changes: 17 additions & 0 deletions admiral/pkg/controller/common/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,23 @@ func DoVSRoutingForCluster(cluster string) bool {
return false
}

func IsSlowStartEnabledForCluster(cluster string) bool {
wrapper.RLock()
defer wrapper.RUnlock()
if !wrapper.params.EnableVSRouting {
return false
}
for _, c := range wrapper.params.VSRoutingSlowStartEnabledClusters {
if c == "*" {
return true
}
if c == cluster {
return true
}
}
return false
}

func DoRoutingPolicyForCluster(cluster string) bool {
wrapper.RLock()
defer wrapper.RUnlock()
Expand Down
70 changes: 70 additions & 0 deletions admiral/pkg/controller/common/config_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,76 @@ func TestDoVSRoutingForCluster(t *testing.T) {

}

func TestIsSlowStartEnabledForCluster(t *testing.T) {
p := AdmiralParams{}

testCases := []struct {
name string
cluster string
enableVSRouting bool
enableVSRoutingSlowStartForCluster []string
expected bool
}{
{
name: "Given enableVSRouting is false, enableVSRoutingSlowStartForCluster is empty" +
"When IsSlowStartEnabledForCluster is called" +
"Then it should return false",
cluster: "cluster1",
enableVSRouting: false,
enableVSRoutingSlowStartForCluster: []string{},
expected: false,
},
{
name: "Given enableVSRouting is true, enableVSRoutingSlowStartForCluster is empty" +
"When IsSlowStartEnabledForCluster is called" +
"Then it should return false",
cluster: "cluster1",
enableVSRouting: true,
enableVSRoutingSlowStartForCluster: []string{},
expected: false,
},
{
name: "Given enableVSRouting is true, and given cluster doesn't exists in the list" +
"When IsSlowStartEnabledForCluster is called" +
"Then it should return false",
cluster: "cluster2",
enableVSRouting: true,
enableVSRoutingSlowStartForCluster: []string{"cluster1"},
expected: false,
},
{
name: "Given enableVSRouting is true, and given cluster does exists in the list" +
"When IsSlowStartEnabledForCluster is called" +
"Then it should return true",
cluster: "cluster1",
enableVSRouting: true,
enableVSRoutingSlowStartForCluster: []string{"cluster1"},
expected: true,
},
{
name: "Given enableVSRouting is true, and all slow start is enabled in all clusters using '*'" +
"When IsSlowStartEnabledForCluster is called" +
"Then it should return false",
cluster: "cluster1",
enableVSRouting: true,
enableVSRoutingSlowStartForCluster: []string{"*"},
expected: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
p.EnableVSRouting = tc.enableVSRouting
p.VSRoutingSlowStartEnabledClusters = tc.enableVSRoutingSlowStartForCluster
ResetSync()
InitializeConfig(p)

assert.Equal(t, tc.expected, IsSlowStartEnabledForCluster(tc.cluster))
})
}

}

func TestConfigManagement(t *testing.T) {
setupForConfigTests()

Expand Down
11 changes: 6 additions & 5 deletions admiral/pkg/controller/common/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,12 @@ type AdmiralParams struct {
OperatorSecretFilterTags string

// VS Based Routing
EnableVSRouting bool
VSRoutingGateways []string
IngressVSExportToNamespaces []string
IngressLBPolicy string
VSRoutingEnabledClusters []string
EnableVSRouting bool
VSRoutingGateways []string
IngressVSExportToNamespaces []string
IngressLBPolicy string
VSRoutingEnabledClusters []string
VSRoutingSlowStartEnabledClusters []string

//Client discovery (types requiring mesh egress only)
EnableClientDiscovery bool
Expand Down

0 comments on commit 8226882

Please sign in to comment.