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.

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

Make fluentd container runtime service configurable. #71124

Merged
merged 1 commit into from
Nov 21, 2018
Merged

Make fluentd container runtime service configurable. #71124

merged 1 commit into from
Nov 21, 2018

Conversation

Random-Liu
Copy link
Member

@Random-Liu Random-Liu commented Nov 16, 2018 •
edited

This doesn't change any existing logic.

This fixes a bug on GCE, and only affects GCE.

Previously, we were using the same environment variable CONTAINER_RUNTIME_NAME for both fluentd config, and health-monitor.

However, this causes an issue on GCE and GKE that when the master is using Docker, and nodes are using other container runtimes (e.g. containerd). There is no way to set master env to configure fluentd to collect containerd logs on nodes, but still run docker health monitor on master, because there is only one CONTAINER_RUNTIME_NAME environment variable.

This is a critical urgent issue fix for Kubernetes on GCE.

none

@Random-Liu Random-Liu added kind/bug Categorizes issue or PR as related to a bug. area/logging sig/node Categorizes an issue or PR as relevant to SIG Node. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. labels Nov 16, 2018
@Random-Liu Random-Liu added this to the v1.11 milestone Nov 16, 2018
@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 16, 2018
@Random-Liu Random-Liu added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. and removed cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Nov 16, 2018
@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/gcp and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Nov 16, 2018
@yujuhong
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 16, 2018
@yujuhong
Copy link
Contributor

/retest

@Random-Liu
Copy link
Member Author

/cc @loburm @piosz Can you guys take a look at this change? Thanks!

@loburm
Copy link
Contributor

loburm commented Nov 19, 2018

/lgtm

@yujuhong
Copy link
Contributor

/assign @dchen1107
for approval

@dchen1107
Copy link
Member

/lgtm

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, Random-Liu, yujuhong

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 19, 2018
@Random-Liu Random-Liu modified the milestones: v1.11, v1.13 Nov 20, 2018
@nikopen
Copy link
Contributor

nikopen commented Nov 20, 2018

@Random-Liu bot requires the cla label, does this command put it back?

/check-cla

@yujuhong yujuhong added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 20, 2018
@nikopen
Copy link
Contributor

nikopen commented Nov 20, 2018

blocked by #71242

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@AishSundar
Copy link
Contributor

/test pull-kubernetes-e2e-gke

1 similar comment
@AishSundar
Copy link
Contributor

/test pull-kubernetes-e2e-gke

@AishSundar
Copy link
Contributor

AishSundar commented Nov 21, 2018 •
edited

/cc @loburm @mikedanese

@loburm we have a couple of PRs failing on pull-kubernetes-e2e-gke consistently today. Can you please help take a look to see if there's a wider GKE issue. this is critical PR blocked on getting into 1.13 release.

@AishSundar
Copy link
Contributor

/test pull-kubernetes-e2e-gke

@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

1 similar comment
@fejta-bot
Copy link

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel comment for consistent failures.

@nikopen
Copy link
Contributor

nikopen commented Nov 21, 2018

/retest

1 similar comment
@nikopen
Copy link
Contributor

nikopen commented Nov 21, 2018

/retest

@k8s-ci-robot k8s-ci-robot merged commit a19bf33 into kubernetes:master Nov 21, 2018
@AishSundar AishSundar mentioned this pull request Nov 21, 2018
Closed
This was referenced Nov 21, 2018
Closed
Merged
Merged
k8s-ci-robot added a commit that referenced this pull request Dec 3, 2018
@k8s-ci-robot
Merge pull request #71331 from Random-Liu/automated-cherry-pick-of-#7…
b758198 
…1124-upstream-release-1.11

Automated cherry pick of #71124: Make fluentd container runtime service configurable.
k8s-ci-robot added a commit that referenced this pull request Dec 17, 2018
@k8s-ci-robot
Merge pull request #71332 from Random-Liu/automated-cherry-pick-of-#7…
d6385cd 
…1124-upstream-release-1.12

Automated cherry pick of #71124: Make fluentd container runtime service configurable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coffeepac coffeepac Awaiting requested review from coffeepac

@piosz piosz Awaiting requested review from piosz

@loburm loburm Awaiting requested review from loburm

@mikedanese mikedanese Awaiting requested review from mikedanese

Assignees

@dchen1107 dchen1107

@loburm loburm

@yujuhong yujuhong

@piosz piosz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/logging cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. release-note-none Denotes a PR that doesn't merit a release note. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/node Categorizes an issue or PR as relevant to SIG Node. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.13
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@Random-Liu @yujuhong @loburm @dchen1107 @k8s-ci-robot @nikopen @fejta-bot @AishSundar @piosz