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

Fixes broken HTTP header and method for health checks. #3178

Merged
merged 2 commits into from
Jun 23, 2017

Conversation

slackpad
Copy link
Contributor

These didn't get plumbed through so were not getting applied. Also reordered them to match the order in the CheckType structure.

@slackpad slackpad added the type/bug Feature does not function as expected label Jun 22, 2017
@slackpad slackpad self-assigned this Jun 22, 2017
@slackpad slackpad requested a review from magiconair June 22, 2017 21:50
@slackpad
Copy link
Contributor Author

With this config:

$ cat header.json
{
  "check": {
    "id": "api",
    "name": "HTTP API on port 5000",
    "http": "http://localhost:5000/health",
    "method": "POST",
    "header": {"x-foo":["bar", "baz"]},
    "interval": "10s",
    "timeout": "1s"
  }
}

Before the fix:

$ nc -l 5000
GET /health HTTP/1.1
Host: localhost:5000
User-Agent: Consul Health Check
Accept: text/plain, text/*, */*
Accept-Encoding: gzip
Connection: close

After the fix:

$ nc -l 5000
POST /health HTTP/1.1
Host: localhost:5000
User-Agent: Consul Health Check
Content-Length: 0
Accept: text/plain, text/*, */*
x-foo: bar
x-foo: baz
Accept-Encoding: gzip
Connection: close

@slackpad
Copy link
Contributor Author

This is kind of a temporary fix, so opened #3179 to track getting rid of the multiple structures so we don't hit this issue again.

@slackpad slackpad merged commit e4b1168 into master Jun 23, 2017
@slackpad slackpad deleted the fix-header-method branch June 23, 2017 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant