Skip to content

Logging sidecar refactoring #2141

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 25, 2017
Merged

Logging sidecar refactoring #2141

merged 2 commits into from
Jan 25, 2017

Conversation

@crassirostris
Copy link

@crassirostris crassirostris commented Jan 5, 2017 •
edited by k8s-reviewable
Loading

Moved from #2114

This is a replacement for https://github.com/kubernetes/contrib/tree/master/logging/fluentd-sidecar-gcp and https://github.com/kubernetes/contrib/tree/master/logging/fluentd-sidecar-es, describing general sidecar approach, including things not covered by docs above:

  • Configuring sidecar logging agent using ConfigMaps
  • Alternative use for sidecar, where it streams logs to stdout/stderr rather than has a logging agent inside

Related to #548

CC @piosz @devin-donnelly @steveperry-53

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jan 5, 2017
@janetkuo janetkuo assigned piosz and devin-donnelly and unassigned erictune Jan 6, 2017
@crassirostris crassirostris mentioned this pull request Jan 11, 2017
Closed
@piosz
Copy link
Member

piosz commented Jan 13, 2017

cc @kubernetes/sig-instrumentation-misc

piosz
piosz approved these changes Jan 13, 2017
View reviewed changes
Copy link
Member

@piosz piosz left a comment

Choose a reason for hiding this comment

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

LGTM, just a nit

Could you get rig of merge commits. I've clicked Update button and it seems you've also done it before.

docs/user-guide/logging/overview.md Outdated

Using this approach you can re-use per-node agent and kubelet log handling
mechanisms. A separate container should contain simple piece of software that
reads logs from a file, a socker or the journald and prints it to the
Copy link
Member

@piosz piosz Jan 13, 2017

Choose a reason for hiding this comment

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

/s/socker/socket/

Copy link
Author

@crassirostris crassirostris Jan 13, 2017

Choose a reason for hiding this comment

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

ACK

@piosz piosz requested a review from devin-donnelly January 13, 2017 10:20
@crassirostris
Copy link
Author

crassirostris commented Jan 13, 2017 •
edited
Loading

@piosz I had some input from @coffeepac and will add some more information soon. I will mention you when it's ready

@steveperry-53
Copy link
Contributor

steveperry-53 commented Jan 18, 2017 •
edited
Loading

@devin-donnelly, Would you like me to do the docs review for this PR? It would make sense, because I just finished working on #2142.

@piosz, Now that #2142 is merged, can you resolve the conflicts here? Sorry, I meant to say, @crassirostris, can you resolve conflicts.

@piosz piosz requested review from steveperry-53 and removed request for devin-donnelly January 18, 2017 06:12
@piosz
Copy link
Member

piosz commented Jan 18, 2017

Thanks @steveperry-53 for offering your help!
@crassirostris please rebase.

@crassirostris
Copy link
Author

crassirostris commented Jan 18, 2017

@steveperry-53 Thanks!

@piosz Sure, will prepare this PR for reviewing by the EOW

@crassirostris
Copy link
Author

crassirostris commented Jan 23, 2017

@piosz @steveperry-53 PTAL

@steveperry-53
Copy link
Contributor

steveperry-53 commented Jan 24, 2017

@crassirostris, See this commit for suggested edits to overview.md.

@steveperry-53
Copy link
Contributor

steveperry-53 commented Jan 24, 2017

lgtm for stackdriver.md

@crassirostris
Copy link
Author

crassirostris commented Jan 24, 2017

@steveperry-53 I agree with all changes, except ones which remote prompts from the shell listings. I think this is a common notation, used everywhere in the docs

@crassirostris
Copy link
Author

crassirostris commented Jan 24, 2017

@steveperry-53 Thanks! PTAL

@steveperry-53 steveperry-53 merged commit 8da2d4f into kubernetes:master Jan 25, 2017
Okabe-Junya pushed a commit to Okabe-Junya/website that referenced this pull request Dec 4, 2023
@ronitblenz @CathPag @nate-double-u
Added a procedure to join the CNCF Slack workspace on main page (kube…
6af19de 
…rnetes#2141)

* doc changed

Signed-off-by: ronitblenz <ronitbanerjee03@gmail.com>

* Update content/en/_index.md

Co-authored-by: Nate W. <natew@cncf.io>
Signed-off-by: Catherine Paganini <74001907+CathPag@users.noreply.github.com>

* Update content/en/_index.md

Co-authored-by: Nate W. <natew@cncf.io>
Signed-off-by: Catherine Paganini <74001907+CathPag@users.noreply.github.com>

---------

Signed-off-by: ronitblenz <ronitbanerjee03@gmail.com>
Signed-off-by: Catherine Paganini <74001907+CathPag@users.noreply.github.com>
Co-authored-by: ronitblenz <ronitbanerjee03@gmail.com>
Co-authored-by: Catherine Paganini <74001907+CathPag@users.noreply.github.com>
Co-authored-by: Nate W. <natew@cncf.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@steveperry-53 steveperry-53 Awaiting requested review from steveperry-53

1 more reviewer

@piosz piosz piosz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@piosz piosz

@steveperry-53 steveperry-53

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@crassirostris @piosz @steveperry-53 @erictune @devin-donnelly @k8s-ci-robot