NE-1790: Enable Dynamic Configuration Manager feature gate #1159

alebedev87 · 2024-10-22T18:33:19Z

Bumped openshift/api
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.

openshift-ci-robot · 2024-10-22T18:33:23Z

@alebedev87: This pull request references NE-1790 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

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 hard reseting pool size to 0.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

pkg/manifests/assets/router/deployment.yaml

pkg/operator/controller/ingress/deployment.go

Miciah · 2024-10-22T22:48:41Z

pkg/operator/controller/ingress/deployment_test.go

+
+	// Set max dynamic server when DCM feature gate is on.
+	ic.Spec.UnsupportedConfigOverrides = runtime.RawExtension{
+		Raw: []byte(`{"maxDynamicServers":"7"}`),
+	}
+	deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, true)
+	if err != nil {
+		t.Fatalf("invalid router Deployment: %v", err)
+	}
+	tests = []envData{
+		{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
+		{"ROUTER_MAX_DYNAMIC_SERVERS", true, "7"},
+	}
+	if err := checkDeploymentEnvironment(t, deployment, tests); err != nil {
+		t.Error(err)
+	}
+
+	// Set max dynamic server to a wrong value when DCM feature gate is on.
+	ic.Spec.UnsupportedConfigOverrides = runtime.RawExtension{
+		Raw: []byte(`{"maxDynamicServers":"wrong"}`),
+	}
+	deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, true)
+	if err != nil {
+		t.Fatalf("invalid router Deployment: %v", err)
+	}
+	tests = []envData{
+		{"ROUTER_HAPROXY_CONFIG_MANAGER", true, "true"},
+		{"ROUTER_MAX_DYNAMIC_SERVERS", true, "1"},
+	}
+	if err := checkDeploymentEnvironment(t, deployment, tests); err != nil {
+		t.Error(err)
+	}
+
+	// Opt out from DCM via unsupportedConfigOverride.
+	ic.Spec.UnsupportedConfigOverrides = runtime.RawExtension{
+		Raw: []byte(`{"dynamicConfigManager":"false", "maxDynamicServers":"7"}`),
+	}
+	deployment, err = desiredRouterDeployment(ic, ingressControllerImage, ingressConfig, infraConfig, apiConfig, networkConfig, proxyNeeded, false, nil, clusterProxyConfig, false, true)
+	if err != nil {
+		t.Fatalf("invalid router Deployment: %v", err)
+	}
+	tests = []envData{
+		{"ROUTER_HAPROXY_CONFIG_MANAGER", false, ""},
+		{"ROUTER_MAX_DYNAMIC_SERVERS", false, "0"},
+	}
+	if err := checkDeploymentEnvironment(t, deployment, tests); err != nil {
+		t.Error(err)
+	}


Long ago, we had just one gigantic TestDesiredRouterDeployment unit test. This test became difficult to modify when we needed to expand test coverage, and so in 2849fcf, Candace started splitting that gigantic test into multiple tests. We should continue in that direction: more, smaller, focused tests rather than fewer, larger, "all the things!" tests.

What do you think about defining a new TestDesiredRouterDeploymentDynamicConfigManager test, perhaps using a list of test cases à la TestDesiredRouterDeploymentDefaultPlacement?

I agree, we need a dedicated test. I thought of a test for all unsupportedConfigOverrides, will start it with DCM and maxDynamicServers stetting, the other ones can be migrated later.

Upd: This test already exists: TestUnsupportedConfigOverride.

What do you think about defining a new TestDesiredRouterDeploymentDynamicConfigManager test, perhaps using a list of test cases à la TestDesiredRouterDeploymentDefaultPlacement?

Upd: This test already exists: TestUnsupportedConfigOverride.

At the end, I think the best way would be to make it a dedicated unit test indeed. Working on it.

TestDesiredRouterDeploymentDynamicConfigManager unit test added.

Miciah · 2024-10-22T22:50:15Z

test/e2e/operator_test.go

-		{"contstats", "contStats", "ROUTER_HAPROXY_CONTSTATS"},
+		{"dynamic-config-manager", "dynamicConfigManager", "ROUTER_HAPROXY_CONFIG_MANAGER", "true", "false"},
+		{"contstats", "contStats", "ROUTER_HAPROXY_CONTSTATS", "", "true"},
+		{"max-dynamic-servers", "maxDynamicServers", "ROUTER_MAX_DYNAMIC_SERVERS", "2", "7"},


Should defaultValue for ROUTER_MAX_DYNAMIC_SERVERS be "1"?

Yes, it should. ROUTER_HAPROXY_CONFIG_MANAGER should have false default value too. These tests failed in CI:

--- FAIL: TestAll/parallel/TestUnsupportedConfigOverride (306.33s) --- FAIL: TestAll/parallel/TestUnsupportedConfigOverride/dynamic-config-manager (111.43s) --- PASS: TestAll/parallel/TestUnsupportedConfigOverride/contstats (83.49s) --- FAIL: TestAll/parallel/TestUnsupportedConfigOverride/max-dynamic-servers (111.41s)

Miciah · 2024-10-22T22:52:09Z

test/e2e/operator_test.go

@@ -3424,10 +3424,11 @@ func TestUnsupportedConfigOverride(t *testing.T) {
 	t.Parallel()

 	var tests = []struct {
-		name, unsupportedConfigOverride, env string
+		name, unsupportedConfigOverride, env, defaultValue, value string


I think this would make the test easier to follow:

Suggested change

name, unsupportedConfigOverride, env, defaultValue, value string

name, unsupportedConfigOverride, env, initialValue, updatedValue string

I kept defaultValue as I think it matches the idea of this variable - expected default value when no overrides are given. Also, I changed value to updateValue and added expectedValue because for the max dynamic servers setting the value after the updated may be different from the update value.

openshift-ci-robot · 2024-10-25T13:30:26Z

@alebedev87: This pull request references NE-1790 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

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 hard reseting pool size to 0.

Better to be merged after #1156.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-10-25T16:29:45Z

@alebedev87: This pull request references NE-1790 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

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 hard reseting (via flag) the pool size to 0.
Configuring via environment variable is not possible as it's lower
bound to 1.

Added a dedicated unit test for desiredRouterDeployemnt with DCM
feature gate.

Better to be merged after #1156.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-10-25T16:30:11Z

@alebedev87: This pull request references NE-1790 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

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 hard reseting (via flag) the pool size to 0. Configuring via environment variable is not possible as it's lower bound to 1.

Added a dedicated unit test for desiredRouterDeployemnt with DCM feature gate.

Better to be merged after #1156.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

alebedev87 · 2024-10-25T16:35:45Z

@Miciah : I made the changes you requested, can you please have another look?
Note that I will wait for #1156 to merge before removing WIP in the title.

alebedev87 · 2024-10-28T11:07:33Z

        --- FAIL: TestAll/parallel/TestManagedDNSToUnmanagedDNSIngressController (222.23s)
        --- FAIL: TestAll/parallel/TestAWSLBSubnets (365.36s)

/test e2e-aws-operator-techpreview

alebedev87 · 2024-10-28T11:08:50Z

/test e2e-aws-ovn-single-node

alebedev87 · 2024-10-28T11:19:03Z

/testwith openshift/cluster-ingress-operator/master/e2e-aws-operator-techpreview openshift/origin/29180

openshift-ci · 2024-10-28T11:19:13Z

@alebedev87, testwith: Error processing request. ERROR:

could not determine job runs: invalid format for additional PR: openshift/origin/29180

alebedev87 · 2024-10-28T11:19:35Z

/testwith openshift/cluster-ingress-operator/master/e2e-aws-operator-techpreview openshift/origin#29180

alebedev87 · 2024-10-28T12:25:52Z

/testwith e2e-aws-ovn-techpreview openshift/origin#29180

openshift-ci · 2024-10-28T12:25:57Z

@alebedev87, testwith: Error processing request. ERROR:

could not determine job runs: requested job is invalid. needs to be formatted like: <org>/<repo>/<branch>/<variant?>/<job>. instead it was: e2e-aws-ovn-techpreview

alebedev87 · 2024-10-28T12:32:15Z

/testwith openshift/cluster-ingress-operator/master/e2e-aws-ovn-techpreview openshift/origin#29180

openshift-ci-robot · 2024-11-04T09:59:46Z

@alebedev87: This pull request references NE-1790 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.18.0" version, but no target version was set.

In response to this:

Bumped to k8s.io v0.31.1 and openshift/api

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 hard reseting (via flag) the pool size to 0. Configuring via environment variable is not possible as it's lower bound to 1.

Added a dedicated unit test for desiredRouterDeployemnt with DCM feature gate.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

alebedev87 · 2024-11-04T10:00:35Z

Added a commit for the k8s and openshift api bumps.

alebedev87 · 2024-11-04T13:50:22Z

/retest

- 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.

alebedev87 · 2024-11-20T16:30:13Z

As discussed during the PR scrub, removed the upper bound limit for the unsupportedConfigOverrides.maxDynamicServers. Some customers wanted to play with hundreds of dynamic slots.

rfredette · 2024-11-20T16:50:11Z

/lgtm

alebedev87 · 2024-11-20T20:27:22Z

/retest

alebedev87 · 2024-11-21T06:45:54Z

/retest

openshift-ci · 2024-11-21T08:52:59Z

@alebedev87: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

alebedev87 · 2024-11-21T18:04:29Z

/label acknowledge-critical-fixes-only

This PR does not introduce any change to the default featureset of OpenShift.

candita · 2024-11-21T18:08:40Z

pkg/operator/controller/ingress/deployment.go

@@ -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)


nit: if there is anything else to change, I would suggest refactoring the desiredRouterDeployment signature to pass in r.config instead of three different parameters in r.config.

Yes, this can be done in a separate PR.

candita · 2024-11-21T18:10:37Z

pkg/operator/controller/ingress/deployment.go

@@ -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.


Suggested change

// The default value is 1, with a maximum of 10.

// The default value is 1.

candita · 2024-11-21T18:13:21Z

pkg/operator/controller/ingress/deployment.go

+		// 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


nit: When this becomes suppported, we should add a link to an OpenShift repo.

candita · 2024-11-21T18:30:58Z

pkg/operator/controller/ingress/deployment_test.go

+				{"ROUTER_MAX_DYNAMIC_SERVERS", true, "100"},
+				{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", true, "0"},
+			},
+		},


Can you add a test to see what happens when unsupportedConfigOverrides.DynamicConfigManager is an empty string? Or if it is malformatted (like "100K") or over the HAProxy limit for numbers (not sure if that applies here).

featuregate-enabled-default-values and featuregate-enabled-max-servers tests don't set unsupportedConfigOverrides.dynamicConfigManager which results into an "" after the unmarshaling.

Sorry, I meant when the max dynamic servers is set to "" or something like "100K" or over the HAProxy limit for numbers.

candita · 2024-11-21T18:37:02Z

pkg/operator/controller/ingress/deployment.go

+		// 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"`


This should probably be an int, not a string. Users may be confused that they'd have to specify 10000 as "10000" here.

Well, I just followed the same pattern as for the other overrides. Two of them (dynamicConfigManager,contStats) could be bools because bool is a valid JSON type.

True, but I think we have precedent for string-based bools. Choosing the string type for numbers allows malformatting, like "100K", which will fail later.

candita · 2024-11-21T18:43:52Z

pkg/operator/controller/ingress/deployment_test.go

+		{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
+		{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},


nit: these constants both have variable names.

For unit tests, I try to abstain from using the constants which are used in the code. This avoids repeating mistakes made in a code (typo etc.) and brings an additional check.

candita · 2024-11-21T18:47:01Z

pkg/operator/controller/ingress/deployment_test.go

@@ -1109,6 +1115,115 @@ func TestDesiredRouterDeploymentClientTLS(t *testing.T) {
 	}
 }

+// TestDesiredRouterDeploymentDynamicConfigManager verifies that desiredRouterDeployment
+// returns the expected deployment when DCM featuregate is involved.


nit:

Suggested change

// returns the expected deployment when DCM featuregate is involved.

// returns the expected deployment when the DCM featuregate is enabled.

alebedev87 · 2024-11-21T20:03:01Z

@candita: I would prefer to follow up on the comment changes (1, 2) in a separate PR (along with this enhacement) unless there are some blocking remarks.

candita · 2024-11-21T21:43:04Z

pkg/operator/controller/ingress/deployment.go

+	RouterHAProxyMaxDynamicServers             = "ROUTER_MAX_DYNAMIC_SERVERS"
+	RouterHAProxyMaxDynamicServersDefaultValue = 1
+
+	RouterHAProxyBlueprintRoutePoolSize = "ROUTER_BLUEPRINT_ROUTE_POOL_SIZE"


These two env variables (ROUTER_MAX_DYNAMIC_SERVERS and ROUTER_BLUEPRINT_ROUTE_POOL_SIZE) are not found in our haproxy.config template yet and I don't see a PR that adds them. Does that matter?

They are not used in the config template, they are only for the configuration of the router's DCM behavior.

Ok, but it's strange that they'd need to be env variables in that case.

The operator usually uses environment variables rather than command-line switches for openshift-router, for which reason I asked Andrey to use environment variables in this case too (#1159 (comment)).

candita · 2024-11-21T21:57:41Z

pkg/operator/controller/ingress/deployment.go

@@ -84,6 +84,11 @@ const (

 	RouterHAProxyConfigManager = "ROUTER_HAPROXY_CONFIG_MANAGER"

+	RouterHAProxyMaxDynamicServers             = "ROUTER_MAX_DYNAMIC_SERVERS"


Does there need to be any change for this in the flags https://github.com/openshift/router/blob/master/pkg/cmd/infra/router/template.go#L185?

No. Why? This flag accepts an environment varible as an alternative way of using it. This one is limited to 1 as minimal value and defaults to 5. I set the envvar explicitly to 1 so no default value is used.

I noticed that you changed the minimum for BlueprintRoutePoolSize, and wondered why the default didn't need to change for MaxDynamicServers.

In order to deliver an MVP in 4.18, we decided to turn off blueprint routes and pre-allocate 1 dynamic server per backend. openshift/enhancements#1687 explains the rationale.

candita · 2024-11-21T23:06:19Z

test/e2e/operator_test.go

+		{"dynamic-config-manager", "dynamicConfigManager", "ROUTER_HAPROXY_CONFIG_MANAGER", dcmDefaultValue, dcmUpdateValue, dcmExpectedValue},
+		{"contstats", "contStats", "ROUTER_HAPROXY_CONTSTATS", "", "true", "true"},
+		{"max-dynamic-servers", "maxDynamicServers", "ROUTER_MAX_DYNAMIC_SERVERS", maxServersDefaultValue, maxServersUpdateValue, maxServersExpectedValue},


nit: We do have variable names for many of these hard-coded values.

#1159 (comment)

Also, there's no test for the route blueprint default.

Also, there's no test for the route blueprint default.

Right, the test was made the way that only 1 variable is expected to be checked for a given config override. I didn't have a goal to fully cover all configuration permutations in this e2e test, I just wanted to adapt the test to work with two new config overrides. The route blueprint configurations are well covered in the unit tests (TestDesiredRouterDeploymentDynamicConfigManager).

candita · 2024-11-21T23:15:47Z

test/e2e/operator_test.go

+	if dcmEnabled, err := isFeatureGateEnabled(features.FeatureGateIngressControllerDynamicConfigurationManager); err != nil {
+		t.Fatalf("failed to get dynamic config manager feature gate: %v", err)
+	} else if dcmEnabled {
+		dcmDefaultValue = "true"


You may want to log something here to let us know this was run on a techpreview cluster.

Suggested change

dcmDefaultValue = "true"

t.Logf("DynamicConfigurationManager feature gate is enabled for this test")

dcmDefaultValue = "true"

candita · 2024-11-21T23:34:54Z

test/e2e/operator_test.go

-			if err := waitForDeploymentEnvVar(t, kclient, deployment, 30*time.Second, tt.env, ""); err != nil {
-				t.Fatalf("expected initial deployment not to set %s=true: %v", tt.env, err)
+			if err := waitForDeploymentEnvVar(t, kclient, deployment, 30*time.Second, tt.env, tt.defaultValue); err != nil {
+				t.Fatalf("expected initial deployment not to set %s=%s: %v", tt.env, tt.defaultValue, err)


Even if the feature gate is enabled, we don't expect to see the max servers set to 1, the default?

Correct, ROUTER_MAX_DYNAMIC_SERVERS is only set if the dynamic config manager is enabled:

cluster-ingress-operator/pkg/operator/controller/ingress/deployment.go

Lines 594 to 611 in 9532e78

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),

})

candita · 2024-11-21T23:53:44Z

@alebedev87 All of my review comments can be handled in a followup PR unless you see something that has a greater impact than I assumed.

/approve

openshift-ci · 2024-11-21T23:53:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: candita

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [candita]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-bot · 2024-11-22T02:44:23Z

[ART PR BUILD NOTIFIER]

Distgit: ose-cluster-ingress-operator
This PR has been included in build ose-cluster-ingress-operator-container-v4.19.0-202411220208.p0.gfb5c5d3.assembly.stream.el9.
All builds following this will include this PR.

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Oct 22, 2024

alebedev87 changed the title ~~NE-1790: Enable Dynamic Configuration Manager feature gate~~ [WIP] NE-1790: Enable Dynamic Configuration Manager feature gate Oct 22, 2024

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 22, 2024

openshift-ci bot requested review from frobware and grzpiotrowski October 22, 2024 18:34

Miciah reviewed Oct 22, 2024

View reviewed changes

alebedev87 force-pushed the dcm-featuregate branch from 7717e17 to 8f6f109 Compare October 24, 2024 15:54

alebedev87 force-pushed the dcm-featuregate branch 2 times, most recently from 9f1b7d2 to a42eff9 Compare October 25, 2024 16:29

alebedev87 force-pushed the dcm-featuregate branch from a42eff9 to 677207b Compare November 4, 2024 09:56

alebedev87 changed the title ~~[WIP] NE-1790: Enable Dynamic Configuration Manager feature gate~~ NE-1790: Enable Dynamic Configuration Manager feature gate Nov 4, 2024

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 4, 2024

alebedev87 force-pushed the dcm-featuregate branch from 677207b to 687a892 Compare November 4, 2024 10:06

alebedev87 force-pushed the dcm-featuregate branch from 687a892 to 88b7758 Compare November 5, 2024 14:25

alebedev87 force-pushed the dcm-featuregate branch from c9b8f39 to 9532e78 Compare November 20, 2024 16:28

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Nov 20, 2024

openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Nov 21, 2024

candita reviewed Nov 21, 2024

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 21, 2024

openshift-merge-bot bot merged commit fb5c5d3 into openshift:master Nov 21, 2024
18 checks passed

alebedev87 mentioned this pull request Dec 18, 2024

NE-1790: Follow up to enable Dynamic Configuration Manager feature gate #1174

Merged

	name, unsupportedConfigOverride, env, defaultValue, value string
	name, unsupportedConfigOverride, env, initialValue, updatedValue string

	// The default value is 1, with a maximum of 10.
	// The default value is 1.

		{"ROUTER_MAX_DYNAMIC_SERVERS", false, ""},
		{"ROUTER_BLUEPRINT_ROUTE_POOL_SIZE", false, ""},

	// returns the expected deployment when DCM featuregate is involved.
	// returns the expected deployment when the DCM featuregate is enabled.

		@@ -84,6 +84,11 @@ const (

		RouterHAProxyConfigManager = "ROUTER_HAPROXY_CONFIG_MANAGER"

		RouterHAProxyMaxDynamicServers = "ROUTER_MAX_DYNAMIC_SERVERS"

	dcmDefaultValue = "true"
	t.Logf("DynamicConfigurationManager feature gate is enabled for this test")
	dcmDefaultValue = "true"

	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),
	})

NE-1790: Enable Dynamic Configuration Manager feature gate #1159

NE-1790: Enable Dynamic Configuration Manager feature gate #1159

Conversation

alebedev87 commented Oct 22, 2024 • edited Loading

openshift-ci-robot commented Oct 22, 2024 • edited by openshift-ci bot Loading

Choose a reason for hiding this comment

alebedev87 Oct 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci-robot commented Oct 25, 2024 • edited by openshift-ci bot Loading

openshift-ci-robot commented Oct 25, 2024 • edited by openshift-ci bot Loading

openshift-ci-robot commented Oct 25, 2024 • edited by openshift-ci bot Loading

alebedev87 commented Oct 25, 2024

alebedev87 commented Oct 28, 2024

alebedev87 commented Oct 28, 2024

alebedev87 commented Oct 28, 2024

openshift-ci bot commented Oct 28, 2024

alebedev87 commented Oct 28, 2024

alebedev87 commented Oct 28, 2024

openshift-ci bot commented Oct 28, 2024

alebedev87 commented Oct 28, 2024

openshift-ci-robot commented Nov 4, 2024 • edited by openshift-ci bot Loading

alebedev87 commented Nov 4, 2024

alebedev87 commented Nov 4, 2024

alebedev87 commented Nov 20, 2024 • edited Loading

rfredette commented Nov 20, 2024

alebedev87 commented Nov 20, 2024

alebedev87 commented Nov 21, 2024

openshift-ci bot commented Nov 21, 2024

alebedev87 commented Nov 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

candita Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

candita Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 commented Nov 21, 2024

candita Nov 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Miciah Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

candita commented Nov 21, 2024

openshift-ci bot commented Nov 21, 2024

openshift-bot commented Nov 22, 2024

alebedev87 commented Oct 22, 2024 •

edited

Loading

openshift-ci-robot commented Oct 22, 2024 •

edited by openshift-ci bot

Loading

alebedev87 Oct 23, 2024 •

edited

Loading

openshift-ci-robot commented Oct 25, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 25, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Oct 25, 2024 •

edited by openshift-ci bot

Loading

openshift-ci-robot commented Nov 4, 2024 •

edited by openshift-ci bot

Loading

alebedev87 commented Nov 20, 2024 •

edited

Loading

candita Nov 21, 2024 •

edited

Loading

candita Nov 21, 2024 •

edited

Loading

candita Nov 21, 2024 •

edited

Loading

Miciah Nov 22, 2024 •

edited

Loading

alebedev87 Dec 18, 2024 •

edited

Loading