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

HTTP Traffic Splitting #261

Merged
merged 1 commit into from
Oct 24, 2022
Merged

HTTP Traffic Splitting #261

merged 1 commit into from
Oct 24, 2022

Conversation

kate-osborn
Copy link
Contributor

This PR adds support for all the traffic splitting scenarios in the Gateway API HTTP traffic splitting guide. With this PR, NKG now supports multiple backendRefs and the backendRef.Weight field. I also added a basic traffic splitting example to our examples directory.

Traffic splitting in the data plane is implemented using the nginx split_clients directive. A split_clients directive is generated for every attached HTTPRoute rule that has multiple backendRefs defined.

If all the backendRefs are valid and their total weight is greater than zero, the split_clients directive will look similar to this:

split_clients $request_id $default_cafe_route_rule0 {
    80.00% default_coffee-v1_80;
    20.00% default_coffee-v2_80;
}

The variable name $default_cafe_route_rule0 is generated using the HTTPRoute nsname and the rule index.
The percentages must sum to 100.00. The values next to the percentages are the upstream names corresponding to the backendRef. When we support additional filters, we will need to change these values to the names of location blocks.

Edge cases:

  • If the number of backendRefs is greater than 1 and the sum of their weights equals zero, then we must return a 500 response code for all requests. In this case, we generate a split_clients block like this:

    split_clients $request_id $default_cafe_route_rule0 {
        100.00% invalid_backend_ref;
    }
    
  • If one or more of the backendRefs is invalid (e.g., unsupported Kind or cross-namespace), then we must return 500 codes for requests that would have otherwise been routed to an invalid backend. In this case, we will generate a split_clients block like this:

    split_clients $request_id $default_cafe_route_rule0 {
        80.00% default_coffee-v1_80; 
        20.00% invalid_backend_ref;
    }
    

    If there are more than one invalid backendRefs, each invalid backendRef will have an entry in the split_clients directive with its percentage.

  • If one backendRef has 100% of the total weight, we will generate a split_clients directive like this:

    split_clients $request_id $default_cafe_route_rule0 {
         10.00% default_coffee-v1_80; 
         # 0.00% invalid_backend_ref;
    }
    

    The 0.00% entry is commented out because it is an invalid nginx config. It's generated as a comment for debugging purposes.

In addition to adding support for traffic splitting, this PR also refactors the /nginx/config package in the following ways:

  • Moves all HTTP config types into a new http package.
  • Modifies config generation so that each template data type -- []http.Server, []http.Upstream, []http.SplitClient -- generates its own config. The Generate function is now only responsible for concatenating all the generated config into one byte array.
  • Replaces the templateExecutor component with a Template component that is a wrapper around a go template. Template is responsible for returning an executable template for each supported data type: []http.Server, []http.Upstream, []http.SplitClient.

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 13, 2022
internal/nginx/config/servers.go Outdated Show resolved Hide resolved
internal/nginx/config/servers.go Outdated Show resolved Hide resolved
internal/nginx/config/split_clients.go Outdated Show resolved Hide resolved
internal/nginx/config/split_clients_test.go Outdated Show resolved Hide resolved
internal/nginx/config/split_clients_test.go Outdated Show resolved Hide resolved
internal/state/backend_refs.go Outdated Show resolved Hide resolved
internal/state/backend_refs.go Outdated Show resolved Hide resolved
internal/state/backend_refs.go Outdated Show resolved Hide resolved
internal/state/backend_refs_test.go Outdated Show resolved Hide resolved
internal/state/configuration.go Outdated Show resolved Hide resolved
internal/nginx/config/split_clients.go Outdated Show resolved Hide resolved
internal/nginx/config/split_clients_test.go Outdated Show resolved Hide resolved
internal/state/backend_refs.go Outdated Show resolved Hide resolved
internal/nginx/config/servers.go Show resolved Hide resolved
Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kate-osborn please see my review

additionally, the link to the api-compatibiltiy doc from the main README became broken after the file rename.

internal/state/configuration.go Outdated Show resolved Hide resolved
internal/state/configuration.go Show resolved Hide resolved
internal/state/configuration_test.go Outdated Show resolved Hide resolved
internal/state/configuration_test.go Outdated Show resolved Hide resolved
@kate-osborn
Copy link
Contributor Author

Hi @kate-osborn please see my review

additionally, the link to the api-compatibiltiy doc from the main README became broken after the file rename.

Thanks for catching this. I updated the README link.

Copy link
Contributor

@pleshakov pleshakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants