Skip to content

Update Graylog2/go-gelf vendoring. Fixes #35613#35765

Merged
thaJeztah merged 2 commits into
moby:masterfrom
ghislainbourgeois:35613-update-go-gelf-v2-for-bugfix
Dec 12, 2017
Merged

Update Graylog2/go-gelf vendoring. Fixes #35613#35765
thaJeztah merged 2 commits into
moby:masterfrom
ghislainbourgeois:35613-update-go-gelf-v2-for-bugfix

Conversation

@ghislainbourgeois
Copy link
Copy Markdown
Contributor

@ghislainbourgeois ghislainbourgeois commented Dec 11, 2017

- What I did

Updated the vendoring of Graylog2/go-gelf to get the latest bug fix. Fixes #35613.

- How I did it

Simply fetch the changes with vndr.

- How to verify it

  1. start something to imitate log server ex: "nc -l -p 12000"
  2. start something to imitate log server ex: "nc -l -p 12000"
  3. start container "docker run --rm -it --log-driver gelf --log-opt gelf-address=tcp://127.0.0.1:12000 debian bash"
  4. type something in the container's terminal (generate some logs)
  5. stop the nc process
  6. type something again in the container's terminal (generate some more logs)

Validate that the docker daemon does not crash.

- Description for the changelog

Fixed daemon crash when using the GELF log driver over TCP when the GELF server goes down.

- A picture of a cute animal (not mandatory but encouraged)

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 11, 2017

We just noticed that the previous pull request updated the vendoring to use a branch, not a tag; #34758 (comment)

Given that the vendor.conf expects the vendoring to be reproducible, can you;

  • add a commit that updates the vendor.conf to refer to the commit that was previously used (i.e., matching what was vendored in Add TCP support for GELF log driver #34758 (comment))
  • then in your current commit, update the vendor.conf to 4143646226541087117ff2f83334ea48b3201841 (current tip of the v2 branch)

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 11, 2017

We'll then cherry-pick/backport the first commit to the currently supported Docker releases (17.09.x, 17.12)

@ghislainbourgeois
Copy link
Copy Markdown
Contributor Author

ghislainbourgeois commented Dec 11, 2017

Sure, just fixed it.

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 11, 2017

Thanks!

ping @anusha-ragunathan @vieux PTAL

@anusha-ragunathan
Copy link
Copy Markdown
Contributor

anusha-ragunathan commented Dec 11, 2017

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 12, 2017

Hm, possibly flaky test on Windows;

22:02:25 ----------------------------------------------------------------------
22:02:25 FAIL: docker_api_logs_test.go:152: DockerSuite.TestLogsAPIUntil
22:02:25 
22:02:25 docker_api_logs_test.go:184:
22:02:25     c.Assert(logsString, checker.Not(checker.Contains), "log3", check.Commentf("unexpected log message returned, until=%v", until))
22:02:25 ... obtained string = "" +
22:02:25 ...     "2017-12-11T22:02:27.571445400Z log1\n" +
22:02:25 ...     "2017-12-11T22:02:27.571657700Z log2\n" +
22:02:25 ...     "2017-12-11T22:02:27.571657700Z log3\n"
22:02:25 ... substring string = "log3"
22:02:25 ... unexpected log message returned, until=2017-12-11T22:02:27.5716577Z
22:02:25

vieux
vieux approved these changes Dec 12, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@vieux vieux left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 12, 2017

Same test is failing again, so there may be an actual issue;

02:20:01 ----------------------------------------------------------------------
02:20:01 FAIL: docker_api_logs_test.go:152: DockerSuite.TestLogsAPIUntil
02:20:01 
02:20:01 docker_api_logs_test.go:184:
02:20:01     c.Assert(logsString, checker.Not(checker.Contains), "log3", check.Commentf("unexpected log message returned, until=%v", until))
02:20:01 ... obtained string = "" +
02:20:01 ...     "2017-12-12T02:20:00.938221900Z log1\n" +
02:20:01 ...     "2017-12-12T02:20:00.938221900Z log2\n" +
02:20:01 ...     "2017-12-12T02:20:00.938221900Z log3\n"
02:20:01 ... substring string = "log3"
02:20:01 ... unexpected log message returned, until=2017-12-12T02:20:00.9382219Z
02:20:01

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 12, 2017

I think the failing test may be introduced by / related to ee594dc (#35745); looking at the output for the failing test, I see that all three log-entries have exactly the same timestamp, so because of that, the --until probably doesn't filter

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 12, 2017

Opened #35771 to fix the test

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Dec 12, 2017

All green now 👍

@thaJeztah thaJeztah merged commit e48e938 into moby:master Dec 12, 2017
@dnephin dnephin mentioned this pull request Dec 12, 2017
Merged
@thaJeztah thaJeztah mentioned this pull request Dec 13, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@vieux vieux vieux approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/logging status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

docker crashes when using gelf log driver in tcp mode and logging server becomes unavailable

6 participants

@ghislainbourgeois @thaJeztah @anusha-ragunathan @vieux @vdemeester @GordonTheTurtle