Skip to content

Commit 4ebbf69

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #1174 from alebedev87/ne-1790-followup
NE-1790: Follow up to enable Dynamic Configuration Manager feature gate
2 parents dbdff16 + 9735e68 commit 4ebbf69

File tree

3 files changed

+35
-23
lines changed

3 files changed

+35
-23
lines changed

‎pkg/operator/controller/ingress/deployment.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func (r *reconciler) ensureRouterDeployment(ci *operatorv1.IngressController, in
126126
if err != nil {
127127
return false, nil, fmt.Errorf("failed to determine if proxy protocol is needed for ingresscontroller %s/%s: %v", ci.Namespace, ci.Name, err)
128128
}
129-
desired, err := desiredRouterDeployment(ci, r.config.IngressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, haveClientCAConfigmap, clientCAConfigmap, clusterProxyConfig, r.config.RouteExternalCertificateEnabled, r.config.IngressControllerDCMEnabled)
129+
desired, err := desiredRouterDeployment(ci, &r.config, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, haveClientCAConfigmap, clientCAConfigmap, clusterProxyConfig)
130130
if err != nil {
131131
return haveDepl, current, fmt.Errorf("failed to build router deployment: %v", err)
132132
}
@@ -248,7 +248,7 @@ func headerValues(values []operatorv1.IngressControllerHTTPHeader) string {
248248
}
249249

250250
// desiredRouterDeployment returns the desired router deployment.
251-
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) {
251+
func desiredRouterDeployment(ci *operatorv1.IngressController, config *Config, ingressConfig *configv1.Ingress, infraConfig *configv1.Infrastructure, apiConfig *configv1.APIServer, networkConfig *configv1.Network, proxyNeeded bool, haveClientCAConfigmap bool, clientCAConfigmap *corev1.ConfigMap, clusterProxyConfig *configv1.Proxy) (*appsv1.Deployment, error) {
252252
deployment := manifests.RouterDeployment()
253253
name := controller.RouterDeploymentName(ci)
254254
deployment.Name = name.Name
@@ -530,7 +530,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
530530
ContStats string `json:"contStats"`
531531
// maxDynamicServers specifies the number of dynamic server slots that will be added to each route.
532532
// This setting can only be configured if dynamicConfigManager is `true`.
533-
// The default value is 1, with a maximum of 10.
533+
// The default value is 1.
534534
// Dynamic server slots help to avoid reloads during endpoint scale-ups. The more dynamic servers
535535
// added, the fewer reloads required when scaling up.
536536
// Note, however, that dynamic servers consume memory even when not enabled.
@@ -582,7 +582,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
582582
}
583583

584584
enableDCM := false
585-
if ingressControllerDCMEnabled {
585+
if config.IngressControllerDCMEnabled {
586586
enableDCM = true
587587
}
588588
dynamicConfigOverride := unsupportedConfigOverrides.DynamicConfigManager
@@ -733,7 +733,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
733733
env = append(env, corev1.EnvVar{Name: "ROUTE_LABELS", Value: routeSelector.String()})
734734
}
735735

736-
deployment.Spec.Template.Spec.Containers[0].Image = ingressControllerImage
736+
deployment.Spec.Template.Spec.Containers[0].Image = config.IngressControllerImage
737737
deployment.Spec.Template.Spec.DNSPolicy = corev1.DNSClusterFirst
738738

739739
var (
@@ -853,7 +853,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
853853
syslogContainer := corev1.Container{
854854
Name: operatorv1.ContainerLoggingSidecarContainerName,
855855
// The ingresscontroller image has rsyslog built in.
856-
Image: ingressControllerImage,
856+
Image: config.IngressControllerImage,
857857
Command: []string{
858858
"/sbin/rsyslogd", "-n",
859859
// TODO: Once we have rsyslog 8.32 or later,
@@ -1179,7 +1179,7 @@ func desiredRouterDeployment(ci *operatorv1.IngressController, ingressController
11791179
}
11801180

11811181
// Add router external certificate environment variable.
1182-
if routeExternalCertificateEnabled {
1182+
if config.RouteExternalCertificateEnabled {
11831183
env = append(env,
11841184
corev1.EnvVar{Name: "ROUTER_ENABLE_EXTERNAL_CERTIFICATE", Value: "true"},
11851185
)

‎pkg/operator/controller/ingress/deployment_test.go‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ func TestTuningOptions(t *testing.T) {
174174
ic.Spec.TuningOptions.HealthCheckInterval = &metav1.Duration{Duration: 15 * time.Second}
175175
ic.Spec.TuningOptions.ReloadInterval = metav1.Duration{Duration: 30 * time.Second}
176176

177-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
177+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
178178
if err != nil {
179179
t.Fatalf("invalid router Deployment: %v", err)
180180
}
@@ -203,7 +203,7 @@ func TestTuningOptions(t *testing.T) {
203203
func TestClusterProxy(t *testing.T) {
204204
ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)
205205

206-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
206+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
207207
if err != nil {
208208
t.Fatalf("invalid router Deployment: %v", err)
209209
}
@@ -227,7 +227,7 @@ func TestClusterProxy(t *testing.T) {
227227
clusterProxyConfig.Status.HTTPSProxy = "bar"
228228
clusterProxyConfig.Status.NoProxy = "baz"
229229

230-
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
230+
deployment, err = desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
231231
if err != nil {
232232
t.Fatalf("invalid router Deployment: %v", err)
233233
}
@@ -392,7 +392,7 @@ func getRouterDeploymentComponents(t *testing.T) (*operatorv1.IngressController,
392392
func Test_desiredRouterDeployment(t *testing.T) {
393393
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)
394394

395-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
395+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
396396
if err != nil {
397397
t.Fatalf("invalid router Deployment: %v", err)
398398
}
@@ -546,7 +546,7 @@ func checkProbes(t *testing.T, container *corev1.Container, useLocalhost bool) {
546546
func TestDesiredRouterDeploymentSpecTemplate(t *testing.T) {
547547
ic, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, clusterProxyConfig := getRouterDeploymentComponents(t)
548548

549-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
549+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
550550
if err != nil {
551551
t.Fatalf("invalid router Deployment: %v", err)
552552
}
@@ -687,7 +687,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
687687
if err != nil {
688688
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
689689
}
690-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
690+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
691691
if err != nil {
692692
t.Fatalf("invalid router Deployment: %v", err)
693693
}
@@ -785,7 +785,7 @@ func TestDesiredRouterDeploymentSpecAndNetwork(t *testing.T) {
785785
if err != nil {
786786
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
787787
}
788-
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
788+
deployment, err = desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
789789
if err != nil {
790790
t.Fatalf("invalid router Deployment: %v", err)
791791
}
@@ -895,7 +895,7 @@ func TestDesiredRouterDeploymentVariety(t *testing.T) {
895895
if err != nil {
896896
t.Errorf("failed to determine infrastructure platform status for ingresscontroller %s/%s: %v", ic.Namespace, ic.Name, err)
897897
}
898-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
898+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
899899
if err != nil {
900900
t.Fatalf("invalid router Deployment: %v", err)
901901
}
@@ -1013,7 +1013,7 @@ func TestDesiredRouterDeploymentHostNetworkNil(t *testing.T) {
10131013
if err != nil {
10141014
t.Fatal(err)
10151015
}
1016-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, false)
1016+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig)
10171017
if err != nil {
10181018
t.Fatal(err)
10191019
}
@@ -1044,7 +1044,7 @@ func TestDesiredRouterDeploymentSingleReplica(t *testing.T) {
10441044
},
10451045
}
10461046

1047-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
1047+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
10481048
if err != nil {
10491049
t.Fatalf("invalid router Deployment: %v", err)
10501050
}
@@ -1079,7 +1079,7 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
10791079
},
10801080
}
10811081
clientCAConfigmap := &corev1.ConfigMap{}
1082-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, true, clientCAConfigmap, clusterProxyConfig, false, false)
1082+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, true, clientCAConfigmap, clusterProxyConfig)
10831083
if err != nil {
10841084
t.Fatalf("invalid router deployment: %v", err)
10851085
}
@@ -1116,7 +1116,7 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
11161116
}
11171117

11181118
// TestDesiredRouterDeploymentDynamicConfigManager verifies that desiredRouterDeployment
1119-
// returns the expected deployment when DCM featuregate is involved.
1119+
// returns the expected deployment when DCM featuregate is enabled.
11201120
func TestDesiredRouterDeploymentDynamicConfigManager(t *testing.T) {
11211121
testCases := []struct {
11221122
name string
@@ -1153,6 +1153,16 @@ func TestDesiredRouterDeploymentDynamicConfigManager(t *testing.T) {
11531153
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
11541154
},
11551155
},
1156+
{
1157+
name: "featuregate-enabled-max-servers-invalid-value",
1158+
unsupportedConfigOverrides: `{"maxDynamicServers":"10K"}`,
1159+
dcmEnabled: true,
1160+
expectedEnv: []envData{
1161+
{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
1162+
{"ROUTER_MAX_DYNAMIC_SERVERS", true, "1"},
1163+
{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
1164+
},
1165+
},
11561166
{
11571167
name: "featuregate-enabled-dcm-override",
11581168
unsupportedConfigOverrides: `{"dynamicConfigManager":"false","maxDynamicServers":"7"}`,
@@ -1212,7 +1222,7 @@ func TestDesiredRouterDeploymentDynamicConfigManager(t *testing.T) {
12121222
},
12131223
}
12141224

1215-
deployment, err := desiredRouterDeployment(ic, "dummy", &configv1.Ingress{}, &configv1.Infrastructure{}, &configv1.APIServer{}, &configv1.Network{}, false, false, nil, &configv1.Proxy{}, false, tc.dcmEnabled)
1225+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage, IngressControllerDCMEnabled: tc.dcmEnabled}, &configv1.Ingress{}, &configv1.Infrastructure{}, &configv1.APIServer{}, &configv1.Network{}, false, false, nil, &configv1.Proxy{})
12161226
if err != nil {
12171227
t.Error(err)
12181228
}
@@ -2513,7 +2523,7 @@ func TestDesiredRouterDeploymentDefaultPlacement(t *testing.T) {
25132523
// This value does not matter in the context of this test, just use a dummy value
25142524
dummyProxyNeeded := true
25152525

2516-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, tc.ingressConfig, tc.infraConfig, apiConfig, networkConfig, dummyProxyNeeded, false, nil, &configv1.Proxy{}, false, false)
2526+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, tc.ingressConfig, tc.infraConfig, apiConfig, networkConfig, dummyProxyNeeded, false, nil, &configv1.Proxy{})
25172527
if err != nil {
25182528
t.Error(err)
25192529
}
@@ -2534,7 +2544,7 @@ func TestDesiredRouterDeploymentDefaultPlacement(t *testing.T) {
25342544
func TestDesiredRouterDeploymentRouterExternalCertificate(t *testing.T) {
25352545
ic, ingressConfig, infraConfig, apiConfig, networkConfig, _, clusterProxyConfig := getRouterDeploymentComponents(t)
25362546

2537-
deployment, err := desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, false, false)
2547+
deployment, err := desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
25382548
if err != nil {
25392549
t.Fatalf("invalid router Deployment: %v", err)
25402550
}
@@ -2548,7 +2558,7 @@ func TestDesiredRouterDeploymentRouterExternalCertificate(t *testing.T) {
25482558
t.Error(err)
25492559
}
25502560

2551-
deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig, true, false)
2561+
deployment, err = desiredRouterDeployment(ic, &Config{IngressControllerImage: ingressControllerImage, RouteExternalCertificateEnabled: true}, ingressConfig, infraConfig, apiConfig, networkConfig, false, false, nil, clusterProxyConfig)
25522562
if err != nil {
25532563
t.Fatalf("invalid router Deployment: %v", err)
25542564
}

‎test/e2e/operator_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3503,6 +3503,7 @@ func TestLoadBalancingAlgorithmUnsupportedConfigOverride(t *testing.T) {
35033503
// configures router pod replicas to use the:
35043504
// - dynamic config manager
35053505
// - contstats
3506+
// - max dynamic servers
35063507
// if the ingresscontroller is so configured using an unsupported config override.
35073508
func TestUnsupportedConfigOverride(t *testing.T) {
35083509
t.Parallel()
@@ -3519,6 +3520,7 @@ func TestUnsupportedConfigOverride(t *testing.T) {
35193520
if dcmEnabled, err := isFeatureGateEnabled(features.FeatureGateIngressControllerDynamicConfigurationManager); err != nil {
35203521
t.Fatalf("failed to get dynamic config manager feature gate: %v", err)
35213522
} else if dcmEnabled {
3523+
t.Logf("DynamicConfigurationManager feature gate is enabled for this test")
35223524
dcmDefaultValue = "true"
35233525
dcmUpdateValue = "false"
35243526
dcmExpectedValue = ""

0 commit comments

Comments
 (0)