Skip to content

Commit

Permalink
NE-1790: Enable Dynamic Configuration Manager feature gate
Browse files Browse the repository at this point in the history
- Enabled Dynamic Configuration Manager if corresponding feature is on.
- Added a new unsupported config override option `maxDynamicServers`,
  with a default value of 1, which is used only if the feature gate is on.
- Disabled DCM route blueprints by setting the pool size to 0.
- Added a dedicated unit test for desiredRouterDeployemnt with DCM
  feature gate.
  • Loading branch information
alebedev87 committed Nov 20, 2024
1 parent 04e2c24 commit 9532e78
Show file tree
Hide file tree
Showing 5 changed files with 202 additions and 26 deletions.
1 change: 1 addition & 0 deletions pkg/operator/controller/ingress/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ type Config struct {
RouteExternalCertificateEnabled bool
IngressControllerLBSubnetsAWSEnabled bool
IngressControllerEIPAllocationsAWSEnabled bool
IngressControllerDCMEnabled bool
}

// reconciler handles the actual ingress reconciliation logic in response to
Expand Down
43 changes: 40 additions & 3 deletions pkg/operator/controller/ingress/deployment.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ const (

RouterHAProxyConfigManager = "ROUTER_HAPROXY_CONFIG_MANAGER"

RouterHAProxyMaxDynamicServers = "ROUTER_MAX_DYNAMIC_SERVERS"
RouterHAProxyMaxDynamicServersDefaultValue = 1

RouterHAProxyBlueprintRoutePoolSize = "ROUTER_BLUEPRINT_ROUTE_POOL_SIZE"

RouterHAProxyContstats = "ROUTER_HAPROXY_CONTSTATS"

RouterHAProxyThreadsEnvName = "ROUTER_THREADS"
Expand Down Expand Up @@ -121,7 +126,7 @@ func (r *reconciler) ensureRouterDeployment(ci *operatorv1.IngressController, in
if err != nil {
return false, nil, fmt.Errorf("failed to determine if proxy protocol is needed for ingresscontroller %s/%s: %v", ci.Namespace, ci.Name, err)
}
desired, err := desiredRouterDeployment(ci, r.config.IngressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, haveClientCAConfigmap, clientCAConfigmap, clusterProxyConfig, r.config.RouteExternalCertificateEnabled)
desired, err := desiredRouterDeployment(ci, r.config.IngressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, haveClientCAConfigmap, clientCAConfigmap, clusterProxyConfig, r.config.RouteExternalCertificateEnabled, r.config.IngressControllerDCMEnabled)
if err != nil {
return haveDepl, current, fmt.Errorf("failed to build router deployment: %v", err)
}
Expand Down Expand Up @@ -243,7 +248,7 @@ func headerValues(values []operatorv1.IngressControllerHTTPHeader) string {
}

// desiredRouterDeployment returns the desired router deployment.
func desiredRouterDeployment(ci *operatorv1.IngressController, ingressControllerImage string, ingressConfig *configv1.Ingress, infraConfig *configv1.Infrastructure, apiConfig *configv1.APIServer, networkConfig *configv1.Network, proxyNeeded bool, haveClientCAConfigmap bool, clientCAConfigmap *corev1.ConfigMap, clusterProxyConfig *configv1.Proxy, routeExternalCertificateEnabled bool) (*appsv1.Deployment, error) {
func desiredRouterDeployment(ci *operatorv1.IngressController, ingressControllerImage string, ingressConfig *configv1.Ingress, infraConfig *configv1.Infrastructure, apiConfig *configv1.APIServer, networkConfig *configv1.Network, proxyNeeded bool, haveClientCAConfigmap bool, clientCAConfigmap *corev1.ConfigMap, clusterProxyConfig *configv1.Proxy, routeExternalCertificateEnabled, ingressControllerDCMEnabled bool) (*appsv1.Deployment, error) {
deployment := manifests.RouterDeployment()
name := controller.RouterDeploymentName(ci)
deployment.Name = name.Name
Expand Down Expand Up @@ -523,6 +528,15 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
LoadBalancingAlgorithm string `json:"loadBalancingAlgorithm"`
DynamicConfigManager string `json:"dynamicConfigManager"`
ContStats string `json:"contStats"`
// maxDynamicServers specifies the number of dynamic server slots that will be added to each route.
// This setting can only be configured if dynamicConfigManager is `true`.
// The default value is 1, with a maximum of 10.
// Dynamic server slots help to avoid reloads during endpoint scale-ups. The more dynamic servers
// added, the fewer reloads required when scaling up.
// Note, however, that dynamic servers consume memory even when not enabled.
// Use this analysis of the memory usage to assess the impact of different numbers of servers:
// https://gist.github.com/frobware/2b527ce3f040797909eff482a4776e0b
MaxDynamicServers string `json:"maxDynamicServers"`
}
if len(ci.Spec.UnsupportedConfigOverrides.Raw) > 0 {
if err := json.Unmarshal(ci.Spec.UnsupportedConfigOverrides.Raw, &unsupportedConfigOverrides); err != nil {
Expand Down Expand Up @@ -567,13 +581,36 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
})
}

enableDCM := false
if ingressControllerDCMEnabled {
enableDCM = true
}
dynamicConfigOverride := unsupportedConfigOverrides.DynamicConfigManager
if v, err := strconv.ParseBool(dynamicConfigOverride); err == nil && v {
if v, err := strconv.ParseBool(dynamicConfigOverride); err == nil {
// Config override can still be used to opt out from DCM.
enableDCM = v
}

if enableDCM {
env = append(env, corev1.EnvVar{
Name: RouterHAProxyConfigManager,
Value: "true",
}, corev1.EnvVar{
Name: RouterHAProxyBlueprintRoutePoolSize,
Value: "0",
})

// Max dynamic servers override is only taken into account when DCM is enabled.
routerHAProxyMaxDynamicServersValue := RouterHAProxyMaxDynamicServersDefaultValue
if v, err := strconv.Atoi(unsupportedConfigOverrides.MaxDynamicServers); err == nil && v > 0 {
routerHAProxyMaxDynamicServersValue = v
}
env = append(env, corev1.EnvVar{
Name: RouterHAProxyMaxDynamicServers,
Value: strconv.Itoa(routerHAProxyMaxDynamicServersValue),
})
}

contStats := unsupportedConfigOverrides.ContStats
if v, err := strconv.ParseBool(contStats); err == nil && v {
env = append(env, corev1.EnvVar{
Expand Down
143 changes: 129 additions & 14 deletions pkg/operator/controller/ingress/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ func TestTuningOptions(t *testing.T) {
ic.Spec.TuningOptions.HealthCheckInterval = &metav1.Duration{Duration: 15 * time.Second}
ic.Spec.TuningOptions.ReloadInterval = metav1.Duration{Duration: 30 * time.Second}

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestTuningOptions(t *testing.T) {
func TestClusterProxy(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand All @@ -227,7 +227,7 @@ func TestClusterProxy(t *testing.T) {
clusterProxyConfig.Status.HTTPSProxy = "bar"
clusterProxyConfig.Status.NoProxy = "baz"

deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -392,7 +392,7 @@ func getRouterDeploymentComponents(t *testing.T) (*operatorv1.IngressController,
func Test_desiredRouterDeployment(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -447,6 +447,8 @@ func Test_desiredRouterDeployment(t *testing.T) {
{"ROUTER_ERRORFILE_503", false, ""},
{"ROUTER_ERRORFILE_404", false, ""},
{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},
{"ROUTER_H1_CASE_ADJUST", false, ""},
{"ROUTER_INSPECT_DELAY", false, ""},
{"ROUTER_IP_V4_V6_MODE", false, ""},
Expand Down Expand Up @@ -544,7 +546,7 @@ func checkProbes(t *testing.T, container *corev1.Container, useLocalhost bool) {
func TestDesiredRouterDeploymentSpecTemplate(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -685,7 +687,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
if err != nil {
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -728,6 +730,8 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
tests := []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
{"ROUTER_LOAD_BALANCE_ALGORITHM", true, "leastconn"},
{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},
{"ROUTER_TCP_BALANCE_SCHEME", true, "source"},
{"ROUTER_MAX_CONNECTIONS", true, "auto"},
{RouterReloadIntervalEnvName, true, "5s"},
Expand Down Expand Up @@ -781,7 +785,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
if err != nil {
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
}
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand All @@ -796,6 +800,8 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
tests = []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
{"ROUTER_LOAD_BALANCE_ALGORITHM", true, "random"},
{"ROUTER_MAX_DYNAMIC_SERVERS", true, "1"},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
{"ROUTER_TCP_BALANCE_SCHEME", true, "source"},
{"ROUTER_MAX_CONNECTIONS", true, "40000"},
{RouterReloadIntervalEnvName, true, "5s"},
Expand Down Expand Up @@ -889,7 +895,7 @@ func TestDesiredRouterDeploymentVariety(t *testing.T) {
if err != nil {
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -1007,7 +1013,7 @@ func TestDesiredRouterDeploymentHostNetworkNil(t *testing.T) {
if err != nil {
t.Fatal(err)
}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -1038,7 +1044,7 @@ func TestDesiredRouterDeploymentSingleReplica(t *testing.T) {
},
}

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down Expand Up @@ -1073,7 +1079,7 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
},
}
clientCAConfigmap := &corev1.ConfigMap{}
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, true, clientCAConfigmap, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, true, clientCAConfigmap, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router deployment: %v", err)
}
Expand Down Expand Up @@ -1109,6 +1115,115 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
}
}

// TestDesiredRouterDeploymentDynamicConfigManager verifies that desiredRouterDeployment
// returns the expected deployment when DCM featuregate is involved.
func TestDesiredRouterDeploymentDynamicConfigManager(t *testing.T) {
testCases := []struct {
name string
unsupportedConfigOverrides string
dcmEnabled bool
expectedEnv []envData
}{
{
name: "featuregate-enabled-default-values",
dcmEnabled: true,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
{"ROUTER_MAX_DYNAMIC_SERVERS", true, "1"},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
},
},
{
name: "featuregate-enabled-max-servers",
unsupportedConfigOverrides: `{"maxDynamicServers":"7"}`,
dcmEnabled: true,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
{"ROUTER_MAX_DYNAMIC_SERVERS", true, "7"},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
},
},
{
name: "featuregate-enabled-max-servers-lower-limit",
unsupportedConfigOverrides: `{"maxDynamicServers":"0"}`,
dcmEnabled: true,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
{"ROUTER_MAX_DYNAMIC_SERVERS", true, "1"},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
},
},
{
name: "featuregate-enabled-dcm-override",
unsupportedConfigOverrides: `{"dynamicConfigManager":"false","maxDynamicServers":"7"}`,
dcmEnabled: true,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},
},
},
{
name: "featuregate-disabled-default-values",
dcmEnabled: false,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},
},
},
{
name: "featuregate-disabled-max-servers",
unsupportedConfigOverrides: `{"maxDynamicServers":"7"}`,
dcmEnabled: false,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},
},
},
{
name: "featuregate-disabled-dcm-override",
unsupportedConfigOverrides: `{"dynamicConfigManager":"true","maxDynamicServers":"100"}`,
dcmEnabled: false,
expectedEnv: []envData{
{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
{"ROUTER_MAX_DYNAMIC_SERVERS", true, "100"},
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
},
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
ic := &operatorv1.IngressController{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
},
Spec: operatorv1.IngressControllerSpec{
UnsupportedConfigOverrides: runtime.RawExtension{
Raw: []byte(tc.unsupportedConfigOverrides),
},
},
// The status does not matter in the context of this test, just use a dummy value.
Status: operatorv1.IngressControllerStatus{
EndpointPublishingStrategy: &operatorv1.EndpointPublishingStrategy{
Type: operatorv1.PrivateStrategyType,
},
},
}

deployment, err := desiredRouterDeployment(ic, "dummy", &configv1.Ingress{}, &configv1.Infrastructure{}, &configv1.APIServer{}, &configv1.Network{}, false, false, nil, &configv1.Proxy{}, false, tc.dcmEnabled)
if err != nil {
t.Error(err)
}

if err := checkDeploymentEnvironment(t, deployment, tc.expectedEnv); err != nil {
t.Error(err)
}
})
}
}

func checkContainerPort(t *testing.T, d *appsv1.Deployment, portName string, port int32) {
t.Helper()
for _, p := range d.Spec.Template.Spec.Containers[0].Ports {
Expand Down Expand Up @@ -2398,7 +2513,7 @@ func TestDesiredRouterDeploymentDefaultPlacement(t *testing.T) {
// This value does not matter in the context of this test, just use a dummy value
dummyProxyNeeded := true

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, tc.ingressConfig, tc.infraConfig, apiConfig, networkConfig, dummyProxyNeeded, false, nil, &configv1.Proxy{}, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, tc.ingressConfig, tc.infraConfig, apiConfig, networkConfig, dummyProxyNeeded, false, nil, &configv1.Proxy{}, false, false)
if err != nil {
t.Error(err)
}
Expand All @@ -2419,7 +2534,7 @@ func TestDesiredRouterDeploymentDefaultPlacement(t *testing.T) {
func TestDesiredRouterDeploymentRouterExternalCertificate(t *testing.T) {
ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)

deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false)
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand All @@ -2433,7 +2548,7 @@ func TestDesiredRouterDeploymentRouterExternalCertificate(t *testing.T) {
t.Error(err)
}

deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, true)
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, true, false)
if err != nil {
t.Fatalf("invalid router Deployment: %v", err)
}
Expand Down
Loading

0 comments on commit 9532e78

Please sign in to comment.