Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update backend group name with a prefix #2730

Merged
merged 3 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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")
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved

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

salonichf5 marked this conversation as resolved.
Show resolved Hide resolved
{{< /bootstrap-table >}}

##### NGINX fails to reload
Expand Down