Skip to content
This repository was archived by the owner on Apr 30, 2025. It is now read-only.

Conversation

@mkocher
Copy link
Member

@mkocher mkocher commented Jul 20, 2021

  • A short explanation of the proposed change:
    Adding support for protocol in route.registrar messages, and having the connections between gorouter and the backend use http2 if it is set in the registration message.

See routing-release issue for more context: [cloudfoundry/routing-release#200]

  • An explanation of the use cases your change solves
    This is most important for cases where we need end to end http2 traffic to an app, e.g. supporting grpc endpoints with app running on CF.

  • Instructions to functionally test the behavior change using operator interfaces (BOSH manifest, logs, curl, and metrics)

This functionality is easiest to test with the other pieces of this change. To test in isolation deploy the branch of diego with the necessary executor changes, push an app which supports h2 and then publish a message on the nats bus manually with the appropriate protocol:

nats pub 'router.register' '{"host":"10.244.0.138","port":61004,"protocol":"http2","tls_port":61006,"uris":["h2c.mud-rabbit.capi.land"],"app":"dd33ca1b-829e-4eb4-9d8e-052c550ab0b8","private_instance_id":"bd460be2-5184-41ba-635b-9abb","private_instance_index":"0","server_cert_domain_san":"bd460be2-5184-41ba-635b-9abb","tags":{"app_id":"dd33ca1b-829e-4eb4-9d8e-052c550ab0b8","app_name":"h2c","component":"route-emitter","instance_id":"0","organization_id":"36f14cc0-5097-4f30-b2bb-14668cb085cd","organization_name":"org","process_id":"dd33ca1b-829e-4eb4-9d8e-052c550ab0b8","process_instance_id":"bd460be2-5184-41ba-635b-9abb","process_type":"web","source_id":"dd33ca1b-829e-4eb4-9d8e-052c550ab0b8","space_id":"dc467951-2878-4243-9f76-fd61573f4120","space_name":"cadet"}}'

  • Expected result after the change

Make a request to the app and see that h2 is working. We instrumented the sample app to tell us which protocol was being used.

  • Current result before the change

Traffic will be over http1.1 or will 500 if the app only supports h2.

  • Links to any other associated PRs

TBD

  • I have viewed signed and have submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using scripts/run-unit-tests-in-docker from routing-release.

  • (Optional) I have run Routing Acceptance Tests and Routing Smoke Tests on bosh lite

  • (Optional) I have run CF Acceptance Tests on bosh lite

mkocher and others added 3 commits July 20, 2021 21:53
- store it in the internal routing table
- message format addition is "protocol":"http2"

[cloudfoundry/routing-release#200]

Co-authored-by: Maria Shaldybin <[email protected]>
Co-authored-by: Greg Cobb <[email protected]>
- Doing this as a separate commit because it touches a lot of lines
- Move createCertAndAddCA into test helper
- h2 function will be used soon

[cloudfoundry/routing-release#200]

Co-authored-by: Matthew Kocher <[email protected]>
Co-authored-by: Maria Shaldybin <[email protected]>
- Because Gorouter's round tripper has a custom TLS config, it will not
attempt HTTP/2 by default. Setting ForceAttemptHTTP2, will cause it to
negotiate up to HTTP/2 with compatible backends.

[cloudfoundry/routing-release#200]

Co-authored-by: Maria Shaldybin <[email protected]>
Co-authored-by: Greg Cobb <[email protected]>
@cf-gitbot
Copy link

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

@Gerg Gerg marked this pull request as ready for review July 27, 2021 21:00
@Gerg
Copy link
Member

Gerg commented Aug 9, 2021

Once this PR is merged, #290 can be moved out of draft. It is branched off of this PR, so the diff currently includes these commits.

@ameowlia
Copy link
Member

ameowlia commented Aug 11, 2021

TODO

  • for logging, log "http" as the protocol, when the field protocol is empty.
  • in pool_test, test when protocol field is empty and when protocol field is "http"

ameowlia and others added 2 commits August 11, 2021 19:43
- subscriber will not set empty protocol

Co-authored-by: Michael Oleske <[email protected]>
@ameowlia ameowlia merged commit 4213a64 into main Aug 11, 2021
@ameowlia ameowlia deleted the http2_backend_support branch August 11, 2021 19:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants