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

Fix using http2 between TO and TM when setting proxy#3927

Merged
ocket8888 merged 2 commits into
apache:masterfrom
mhoppa:bug/to_tm_http2
Sep 25, 2019
Merged

Fix using http2 between TO and TM when setting proxy#3927
ocket8888 merged 2 commits into
apache:masterfrom
mhoppa:bug/to_tm_http2

Conversation

@mhoppa

@mhoppa mhoppa commented Sep 17, 2019

Copy link
Copy Markdown
Contributor

What does this PR (Pull Request) do?

If TM has an https proxy set that is used by TO to connect it will fail as golang uses http2 be default in that case. This PR overrides that behavior based on the net godocs https://golang.org/src/net/http/doc.go.

There is not documentation changes as it is a bug fix and there is no tests as we dont have TM set up for our API tests

Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Setup TM proxy with HTTPs ensure endpoint cdns/capacity work

If this is a bug fix, what versions of Traffic Control are affected?

The following criteria are ALL met by this PR

  • This PR does not include tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

@asf-ci

asf-ci commented Sep 17, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4268/
Test PASSed.

@mhoppa mhoppa changed the title Fix using http2 between TO and TM when setting proxy WIP - Fix using http2 between TO and TM when setting proxy Sep 17, 2019
@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops bug something isn't working as intended Traffic Monitor related to Traffic Monitor labels Sep 18, 2019
@mitchell852
mitchell852 requested a review from rob05c September 18, 2019 16:42
@mitchell852

Copy link
Copy Markdown
Member

is there not a good way to do this globally? i'm just worried that other PRs will sneak thru without this type of change applied and generate a lot of bugs. i.e. #2305

@mhoppa mhoppa changed the title WIP - Fix using http2 between TO and TM when setting proxy Fix using http2 between TO and TM when setting proxy Sep 20, 2019
@mhoppa

mhoppa commented Sep 20, 2019

Copy link
Copy Markdown
Contributor Author

Agreed with you @mitchell852. One question is what services besides TM/TR does the TO talk to? If we put getting http clients for each of them into a common method we can enforce it a little better as https://github.com/apache/trafficcontrol/pull/2305/files#diff-6e647a5b8d470f399423ff712efb0050R40 does.

As we talked we can set it globally per https://golang.org/src/net/http/doc.go via GODEBUG=http2client=0 # disable HTTP/2 client support but I think we have agreed that feels a little cumbersome.

@ocket8888

Copy link
Copy Markdown
Contributor

It talks to TV and TS and a mail server (sort of and sometimes)

@asf-ci

asf-ci commented Sep 25, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4311/
Test FAILed.

@mhoppa

mhoppa commented Sep 25, 2019

Copy link
Copy Markdown
Contributor Author

retest this please

@ocket8888 ocket8888 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Compiles, works, fixes the issue.

@ocket8888
ocket8888 merged commit 56096b5 into apache:master Sep 25, 2019
@asf-ci

asf-ci commented Sep 25, 2019

Copy link
Copy Markdown
Contributor

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/4312/
Test PASSed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended Traffic Monitor related to Traffic Monitor Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TP: internal server error on dashboard page

4 participants