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

[20.10 backport] backport fluentd log driver async connect fix #43147

Merged
merged 3 commits into from Jan 20, 2022

Conversation

PettitWesley
Copy link
Contributor

@PettitWesley PettitWesley commented Jan 12, 2022

cherry picks of:

Currently, the fix for the issue is only merged into master. We want to get released sooner if possible.

This issue shows the impact for AWS customers: aws/amazon-ecs-agent#2787

And this comment in that issue shows how I tested both the original PR and this backport: aws/amazon-ecs-agent#2787 (comment)

@thaJeztah thaJeztah changed the title Backport fluentd log driver async connect fix to 20.10 [20.10 backport] backport fluentd log driver async connect fix Jan 12, 2022
@thaJeztah thaJeztah added this to the 20.10.13 milestone Jan 12, 2022
@thaJeztah
Copy link
Member

Thanks! Wondering if we should

  • cherry-pick the commit from vendor: github.com/fluent/fluent-logger-golang 1.6.1 #42418 as an intermediate commit in this PR as well (end result would be the same, but for a slightly cleaner history) looks like that one was not yet in the 20.10 branch, and there's some tickets linked to that that were resolved.
  • minor nit: we tend to cherry-pick commits with -X option (which adds a reference to the commit that was cherry-picked) (and the -s (sign-off) option, otherwise the DCO check may fail if the last line of the commit message is not a signed-off-by line)

let me also ping @samuelkarp for the above, and for review 👍

sparrc and others added 3 commits January 13, 2022 01:05
Updates the fluent logger library. Namely this fixes a couple places
where the library could panic when closing and writing to channels.

see fluent/fluent-logger-golang#93 and
fluent/fluent-logger-golang#95

closes moby#40829
closes moby#32567

Signed-off-by: Cam <gh@sparr.email>
(cherry picked from commit a6a98d6)
Signed-off-by: Wesley <wppttt@amazon.com>
Updates the fluent logger library to v1.8.0. Following PRs/commits were
merged since last bump:

* [Add callback for error handling when using
  async](fluent/fluent-logger-golang#97)
* [Fix panic when accessing unexported struct
  field](fluent/fluent-logger-golang#99)
* [Properly stop logger during (re)connect
  failure](fluent/fluent-logger-golang#82)
* [Support a TLS-enabled connection](fluent/fluent-logger-golang@e5d6aa1)

See https://github.com/fluent/fluent-logger-golang/compare/v1.6.1..v1.8.0

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
(cherry picked from commit e24d61b)
Signed-off-by: Wesley <wppttt@amazon.com>
The flag ForceStopAsyncSend was added to fluent logger lib in v1.5.0 (at
this time named AsyncStop) to tell fluentd to abort sending logs
asynchronously as soon as possible, when its Close() method is called.
However this flag was broken because of the way the lib was handling it
(basically, the lib could be stucked in retry-connect loop without
checking this flag).

Since fluent logger lib v1.7.0, calling Close() (when ForceStopAsyncSend
is true) will really stop all ongoing send/connect procedure,
wherever it's stucked.

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
(cherry picked from commit bd61629)
Signed-off-by: Wesley <wppttt@amazon.com>
@PettitWesley
Copy link
Contributor Author

@thaJeztah Fixed the commits as requested.

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants