Skip to content

Commit

Permalink
Update backend group name with a prefix (#2730)
Browse files Browse the repository at this point in the history
Update backend group name with a prefix 

Problem: User want to configure split client upstream in a namespace starting with a digit without running into any issues.

Solution: Updated backend group name with a prefix "group_". Hence, we update split client variable names, distribution names and upstream names.
  • Loading branch information
salonichf5 authored Nov 5, 2024
1 parent fed4239 commit a7c6977
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 24 deletions.
4 changes: 2 additions & 2 deletions internal/mode/static/nginx/config/servers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,7 @@ func TestCreateServers(t *testing.T) {
},
{
Path: "/_ngf-internal-rule1-route0",
ProxyPass: "http://$test__route1_rule1$request_uri",
ProxyPass: "http://$group_test__route1_rule1$request_uri",
ProxySetHeaders: httpBaseHeaders,
Type: http.InternalLocationType,
Includes: internalIncludes,
Expand Down Expand Up @@ -2810,7 +2810,7 @@ func TestCreateProxyPass(t *testing.T) {
},
},
{
expected: "http://$ns1__bg_rule0$request_uri",
expected: "http://$group_ns1__bg_rule0$request_uri",
grp: dataplane.BackendGroup{
Source: types.NamespacedName{Namespace: "ns1", Name: "bg"},
Backends: []dataplane.Backend{
Expand Down
16 changes: 8 additions & 8 deletions internal/mode/static/nginx/config/split_clients_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ func TestExecuteSplitClients(t *testing.T) {
bg3,
},
expStrings: []string{
"split_clients $request_id $test__hr_rule0",
"split_clients $request_id $test__hr_rule1",
"split_clients $request_id $group_test__hr_rule0",
"split_clients $request_id $group_test__hr_rule1",
"50.00% test1;",
"50.00% test2;",
"50.00% test3;",
Expand All @@ -74,7 +74,7 @@ func TestExecuteSplitClients(t *testing.T) {
},
},
expStrings: []string{
"split_clients $request_id $test__zero_percent_rule0",
"split_clients $request_id $group_test__zero_percent_rule0",
"100.00% non-zero;",
"# 0.00% zero;",
},
Expand Down Expand Up @@ -188,7 +188,7 @@ func TestCreateSplitClients(t *testing.T) {
},
expSplitClients: []http.SplitClient{
{
VariableName: "test__hr_one_split_rule0",
VariableName: "group_test__hr_one_split_rule0",
Distributions: []http.SplitClientDistribution{
{
Percent: "50.00",
Expand All @@ -201,7 +201,7 @@ func TestCreateSplitClients(t *testing.T) {
},
},
{
VariableName: "test__hr_two_splits_rule0",
VariableName: "group_test__hr_two_splits_rule0",
Distributions: []http.SplitClientDistribution{
{
Percent: "50.00",
Expand All @@ -214,7 +214,7 @@ func TestCreateSplitClients(t *testing.T) {
},
},
{
VariableName: "test__hr_two_splits_rule1",
VariableName: "group_test__hr_two_splits_rule1",
Distributions: []http.SplitClientDistribution{
{
Percent: "33.33",
Expand Down Expand Up @@ -661,7 +661,7 @@ func TestBackendGroupName(t *testing.T) {
Weight: 1,
},
},
expName: "test__hr_rule0",
expName: "group_test__hr_rule0",
},
{
msg: "multiple invalid backends",
Expand All @@ -677,7 +677,7 @@ func TestBackendGroupName(t *testing.T) {
Weight: 1,
},
},
expName: "test__hr_rule0",
expName: "group_test__hr_rule0",
},
}

Expand Down
36 changes: 23 additions & 13 deletions internal/mode/static/state/dataplane/configuration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2992,20 +2992,21 @@ func TestBuildUpstreams(t *testing.T) {
g.Expect(upstreams).To(ConsistOf(expUpstreams))
}

func TestBuildBackendGroups(t *testing.T) {
t.Parallel()
createBackendGroup := func(name string, ruleIdx int, backendNames ...string) BackendGroup {
backends := make([]Backend, len(backendNames))
for i, name := range backendNames {
backends[i] = Backend{UpstreamName: name}
}
func createBackendGroup(name string, ruleIdx int, backendNames ...string) BackendGroup {
backends := make([]Backend, len(backendNames))
for i, name := range backendNames {
backends[i] = Backend{UpstreamName: name}
}

return BackendGroup{
Source: types.NamespacedName{Namespace: "test", Name: name},
RuleIdx: ruleIdx,
Backends: backends,
}
return BackendGroup{
Source: types.NamespacedName{Namespace: "test", Name: name},
RuleIdx: ruleIdx,
Backends: backends,
}
}

func TestBuildBackendGroups(t *testing.T) {
t.Parallel()

hr1Group0 := createBackendGroup("hr1", 0, "foo", "bar")

Expand Down Expand Up @@ -3061,10 +3062,19 @@ func TestBuildBackendGroups(t *testing.T) {
g := NewWithT(t)

result := buildBackendGroups(servers)

g.Expect(result).To(ConsistOf(expGroups))
}

func TestBackendGroupName(t *testing.T) {
t.Parallel()
backendGroup := createBackendGroup("route1", 2, "foo", "bar")

expectedGroupName := "group_test__route1_rule2"

g := NewWithT(t)
g.Expect(backendGroup.Name()).To(Equal(expectedGroupName))
}

func TestHostnameMoreSpecific(t *testing.T) {
t.Parallel()
tests := []struct {
Expand Down
4 changes: 3 additions & 1 deletion internal/mode/static/state/dataplane/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,11 +267,13 @@ type BackendGroup struct {

// Name returns the name of the backend group.
// This name must be unique across all HTTPRoutes and all rules within the same HTTPRoute.
// It is prefixed with `group_` for cases when namespace name starts with a digit. Variable names
// in nginx configuration cannot start with a digit.
// The RuleIdx is used to make the name unique across all rules within the same HTTPRoute.
// The RuleIdx may change for a given rule if an update is made to the HTTPRoute, but it will always match the index
// of the rule in the stored HTTPRoute.
func (bg *BackendGroup) Name() string {
return fmt.Sprintf("%s__%s_rule%d", bg.Source.Namespace, bg.Source.Name, bg.RuleIdx)
return fmt.Sprintf("group_%s__%s_rule%d", bg.Source.Namespace, bg.Source.Name, bg.RuleIdx)
}

// Backend represents a Backend for a routing rule.
Expand Down
2 changes: 2 additions & 0 deletions site/content/how-to/monitoring/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -294,13 +294,15 @@ Verify that the port number (for example, `8080`) matches the port number you ha
### Common errors

{{< bootstrap-table "table table-striped table-bordered" >}}

| Problem Area | Symptom | Troubleshooting Method | Common Cause |
|------------------------------|----------------------------------------|---------------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------|
| Startup | NGINX Gateway Fabric fails to start. | Check logs for _nginx_ and _nginx-gateway_ containers. | Readiness probe failed. |
| Resources not configured | Status missing on resources. | Check referenced resources. | Referenced resources do not belong to NGINX Gateway Fabric. |
| NGINX errors | Reload failures on NGINX | Fix permissions for control plane. | Security context not configured. |
| Usage reporting | Errors logs related to usage reporting | Enable usage reporting. Refer to [Usage Reporting]({{< relref "installation/usage-reporting.md" >}}) | Usage reporting disabled. |
| Client Settings | Request entity too large error | Adjust client settings. Refer to [Client Settings Policy]({{< relref "../traffic-management/client-settings.md" >}}) | Payload is greater than the [`client_max_body_size`](https://nginx.org/en/docs/http/ngx_http_core_module.html#client_max_body_size) value.|

{{< /bootstrap-table >}}

##### NGINX fails to reload
Expand Down

0 comments on commit a7c6977

Please sign in to comment.