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

Add CRI container log rotation support #59898

Merged
merged 4 commits into from Feb 23, 2018

Conversation

@Random-Liu
Member

Random-Liu commented Feb 15, 2018

For kubernetes/enhancements#552.
Fixes #58823.

This PR:

  1. Added pkg/kubelet/logs/container_log_manager.go which manages and rotates container logs.
  2. Added a feature gate CRIContainerLogRotation to enable the alpha feature. And 2 kubelet flags --container-log-max-size and --container-log-max-files to configure the rotation behavior.
  3. Added unit test and node e2e test for container log rotation.

Note that:

  1. Container log manager only starts when the container runtime is remote (not docker), because we can't implement ReopenContainerLog for docker.
  2. Rotated logs are compressed with gzip.
  3. The latest rotated log is not compressed. Because fluentd may still be reading the file right after rotation.
  4. kubectl logs still doesn't support log rotation. This is not a regression anyway, it doesn't support log rotation for docker log today. We'll probably fix this in the future. (Issue: #59902)

An example of container log directory with --container-log-max-files=3:

$ ls -al /var/log/pods/57146449-11ec-11e8-90e1-42010af00002
total 592
drwxr-xr-x 2 root root   4096 Feb 15 01:07 .
drwxr-xr-x 3 root root  12288 Feb 15 01:06 ..
-rw-r----- 1 root root 176870 Feb 15 01:07 log-container_0.log
-rw-r--r-- 1 root root  40239 Feb 15 01:07 log-container_0.log.20180215-010737.gz
-rw-r----- 1 root root 365996 Feb 15 01:07 log-container_0.log.20180215-010747

/assign @mtaufen for the config change.
/assign @dashpole @crassirostris for the log change.
/assign @feiskyer for CRI related change.
/cc @yujuhong @feiskyer @abhi @mikebrow @mrunalp @runcom
/cc @kubernetes/sig-node-pr-reviews @kubernetes/sig-instrumentation-pr-reviews

Release note:

[Alpha] Kubelet now supports container log rotation for container runtime which implements CRI(container runtime interface).
The feature can be enabled with feature gate `CRIContainerLogRotation`.
The flags `--container-log-max-size` and `--container-log-max-files` can be used to configure the rotation behavior.
@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 15, 2018

Node e2e test result with cri-containerd:

$ make test-e2e-node RUNTIME=remote CONTAINER_RUNTIME_ENDPOINT=unix:///var/run/cri-containerd.sock TEST_ARGS='--feature-gates=DynamicKubeletConfig=true --kubelet-flags=--cgroups-per-qos=true --kubelet-flags=--cgroup-root=/ --prepull-images=false' PARALLELISM=1 FOCUS="ContainerLogRotation" SKIP=""
+++ [0215 01:04:58] Building the toolchain targets:
    k8s.io/kubernetes/hack/cmd/teststale
    k8s.io/kubernetes/vendor/github.com/jteeuwen/go-bindata/go-bindata
+++ [0215 01:04:58] Generating bindata:
    test/e2e/generated/gobindata_util.go
+++ [0215 01:04:59] Building go targets for linux/amd64:
    vendor/github.com/onsi/ginkgo/ginkgo
Creating artifacts directory at /tmp/_artifacts/180215T010508
Test artifacts will be written to /tmp/_artifacts/180215T010508
+++ [0215 01:05:11] Building the toolchain targets:
    k8s.io/kubernetes/hack/cmd/teststale
    k8s.io/kubernetes/vendor/github.com/jteeuwen/go-bindata/go-bindata
+++ [0215 01:05:12] Generating bindata:
    test/e2e/generated/gobindata_util.go
+++ [0215 01:05:12] Building go targets for linux/amd64:
    cmd/kubelet
    test/e2e_node/e2e_node.test
    vendor/github.com/onsi/ginkgo/ginkgo
    cluster/gce/gci/mounter
STEP: Enabling support for Device Plugin
STEP: Enabling support for Device Plugin
Feb 15 01:05:22.841: INFO: Parsing ds from https://raw.githubusercontent.com/kubernetes/kubernetes/master/cluster/addons/device-plugins/nvidia-gpu/daemonset.yaml
W0215 01:05:23.048978   18013 test_context.go:359] Unable to find in-cluster config, using default host : http://127.0.0.1:8080
Running Suite: E2eNode Suite
============================
Random Seed: 1518656722 - Will randomize all specs
Will run 1 of 260 specs

STEP: Enabling support for Device Plugin
STEP: Enabling support for Device Plugin
Feb 15 01:05:23.218: INFO: Parsing ds from https://raw.githubusercontent.com/kubernetes/kubernetes/master/cluster/addons/device-plugins/nvidia-gpu/daemonset.yaml
W0215 01:05:23.407333   18028 test_context.go:359] Unable to find in-cluster config, using default host : http://127.0.0.1:8080
I0215 01:05:23.407435   18028 validators.go:44] Validating os...
OS: Linux
I0215 01:05:23.408395   18028 validators.go:44] Validating kernel...
I0215 01:05:23.409234   18028 kernel_validator.go:81] Validating kernel version
KERNEL_VERSION: 4.13.0-32-generic
I0215 01:05:23.409342   18028 kernel_validator.go:96] Validating kernel config
CONFIG_NAMESPACES: enabled
CONFIG_NET_NS: enabled
CONFIG_PID_NS: enabled
CONFIG_IPC_NS: enabled
CONFIG_UTS_NS: enabled
CONFIG_CGROUPS: enabled
CONFIG_CGROUP_CPUACCT: enabled
CONFIG_CGROUP_DEVICE: enabled
CONFIG_CGROUP_FREEZER: enabled
CONFIG_CGROUP_SCHED: enabled
CONFIG_CPUSETS: enabled
CONFIG_MEMCG: enabled
CONFIG_INET: enabled
CONFIG_EXT4_FS: enabled
CONFIG_PROC_FS: enabled
CONFIG_NETFILTER_XT_TARGET_REDIRECT: enabled (as module)
CONFIG_NETFILTER_XT_MATCH_COMMENT: enabled (as module)
CONFIG_OVERLAY_FS: enabled (as module)
CONFIG_AUFS_FS: enabled (as module)
CONFIG_BLK_DEV_DM: enabled
I0215 01:05:23.420246   18028 validators.go:44] Validating cgroups...
CGROUPS_CPU: enabled
CGROUPS_CPUACCT: enabled
CGROUPS_CPUSET: enabled
CGROUPS_DEVICES: enabled
CGROUPS_FREEZER: enabled
CGROUPS_MEMORY: enabled
I0215 01:05:23.420384   18028 validators.go:44] Validating package...
PASS
I0215 01:05:23.423468   18013 kubelet.go:115] Starting kubelet
I0215 01:05:23.423648   18013 feature_gate.go:190] feature gates: map[DynamicKubeletConfig:true]
I0215 01:05:23.424753   18013 server.go:147] Starting server "kubelet" with command "/usr/bin/systemd-run --unit=kubelet-1777808992.service --slice=runtime.slice --remain-after-exit /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/kubelet --kubeconfig /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/kubeconfig --root-dir /var/lib/kubelet --docker-disable-shared-pid=false --v 4 --logtostderr --allow-privileged true --feature-gates DynamicKubeletConfig=true --dynamic-config-dir /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/dynamic-kubelet-config --network-plugin=kubenet --cni-bin-dir /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/cni/bin --cni-conf-dir /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/cni/net.d --hostname-override workstation --container-runtime remote --container-runtime-endpoint unix:///var/run/cri-containerd.sock --config /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/kubelet-config --container-runtime=remote --network-plugin= --cni-bin-dir= --cgroups-per-qos=true --cgroup-root=/"
I0215 01:05:23.424805   18013 server.go:102] Running readiness check for service "kubelet"
I0215 01:05:23.424875   18013 server.go:175] Output file for server "kubelet": /tmp/_artifacts/180215T010508/kubelet.log
I0215 01:05:23.425227   18013 server.go:217] Running health check for service "kubelet"
I0215 01:05:23.425242   18013 server.go:102] Running readiness check for service "kubelet"
I0215 01:05:24.426027   18013 server.go:147] Starting server "services" with command "/home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/bin/e2e_node.test --run-services-mode --test.timeout=24h --ginkgo.seed=1518656722 --ginkgo.focus=ContainerLogRotation --ginkgo.slowSpecThreshold=5.00000 --container-runtime=remote --alsologtostderr --v 4 --report-dir=/tmp/_artifacts/180215T010508 --node-name workstation --kubelet-flags=--container-runtime=remote --kubelet-flags=--network-plugin= --cni-bin-dir= --container-runtime-endpoint=unix:///var/run/cri-containerd.sock --feature-gates=DynamicKubeletConfig=true --kubelet-flags=--cgroups-per-qos=true --kubelet-flags=--cgroup-root=/ --prepull-images=false"
I0215 01:05:24.426069   18013 server.go:102] Running readiness check for service "services"
I0215 01:05:24.426095   18013 server.go:228] Initial health check passed for service "kubelet"
I0215 01:05:24.426147   18013 server.go:175] Output file for server "services": /tmp/_artifacts/180215T010508/services.log
I0215 01:05:24.426578   18013 server.go:206] Waiting for server "services" start command to complete
I0215 01:05:30.433048   18013 e2e_node_suite_test.go:161] Node services started.  Running tests...
I0215 01:05:30.433072   18013 e2e_node_suite_test.go:166] Wait for the node to be ready
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
------------------------------
[k8s.io] ContainerLogRotation [Slow] [Serial] [Disruptive] when a container generates a lot of log 
  should be rotated and limited to a fixed amount of files
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/container_log_rotation_test.go:59
[BeforeEach] [k8s.io] ContainerLogRotation [Slow] [Serial] [Disruptive]
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:141
STEP: Creating a kubernetes client
STEP: Building a namespace api object
Feb 15 01:05:30.499: INFO: No PodSecurityPolicies found; assuming PodSecurityPolicy is disabled.
Feb 15 01:05:30.499: INFO: Skipping waiting for service account
[BeforeEach] when a container generates a lot of log
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/container_log_rotation_test.go:46
[BeforeEach] when a container generates a lot of log
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/util.go:103
I0215 01:05:36.435706   18013 server.go:268] Restarting server "kubelet" with restart command
I0215 01:05:36.440832   18013 server.go:217] Running health check for service "kubelet"
I0215 01:05:36.440865   18013 server.go:102] Running readiness check for service "kubelet"
I0215 01:05:37.442153   18013 server.go:228] Initial health check passed for service "kubelet"
I0215 01:05:40.560059   18013 util.go:192] new configuration has taken effect
[It] should be rotated and limited to a fixed amount of files
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/container_log_rotation_test.go:59
STEP: create log container
STEP: get container log path
I0215 01:05:46.587988   18013 remote_runtime.go:43] Connecting to runtime service unix:///var/run/cri-containerd.sock
I0215 01:05:46.588051   18013 remote_image.go:40] Connecting to image service unix:///var/run/cri-containerd.sock
[AfterEach] when a container generates a lot of log
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/util.go:119
I0215 01:08:09.552113   18013 server.go:268] Restarting server "kubelet" with restart command
I0215 01:08:09.556587   18013 server.go:217] Running health check for service "kubelet"
I0215 01:08:09.556618   18013 server.go:102] Running readiness check for service "kubelet"
I0215 01:08:10.557662   18013 server.go:228] Initial health check passed for service "kubelet"
I0215 01:08:11.644976   18013 util.go:192] new configuration has taken effect
[AfterEach] [k8s.io] ContainerLogRotation [Slow] [Serial] [Disruptive]
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:142
Feb 15 01:08:11.645: INFO: Waiting up to 3m0s for all (but 0) nodes to be ready
Feb 15 01:08:11.647: INFO: Condition Ready of node workstation is false instead of true. Reason: KubeletNotReady, message: container runtime is down
Feb 15 01:08:13.650: INFO: Condition Ready of node workstation is false instead of true. Reason: KubeletNotReady, message: container runtime is down
Feb 15 01:08:15.650: INFO: Condition Ready of node workstation is false instead of true. Reason: KubeletNotReady, message: container runtime is down
Feb 15 01:08:17.650: INFO: Condition Ready of node workstation is false instead of true. Reason: KubeletNotReady, message: container runtime is down
Feb 15 01:08:19.649: INFO: Condition Ready of node workstation is false instead of true. Reason: KubeletNotReady, message: container runtime is down
STEP: Destroying namespace "e2e-tests-container-log-rotation-test-d545v" for this suite.
Feb 15 01:09:13.662: INFO: Waiting up to 30s for server preferred namespaced resources to be successfully discovered
Feb 15 01:09:13.728: INFO: namespace: e2e-tests-container-log-rotation-test-d545v, resource: bindings, ignored listing per whitelist
Feb 15 01:09:13.731: INFO: namespace e2e-tests-container-log-rotation-test-d545v deletion completed in 52.081340378s

• [SLOW TEST:223.271 seconds]
[k8s.io] ContainerLogRotation [Slow] [Serial] [Disruptive]
/home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e/framework/framework.go:669
  when a container generates a lot of log
  /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/container_log_rotation_test.go:45
    should be rotated and limited to a fixed amount of files
    /home/lantaol/workspace/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/test/e2e_node/container_log_rotation_test.go:59
------------------------------
SSSSSSSI0215 01:09:13.731548   18013 e2e_node_suite_test.go:182] Stopping node services...
I0215 01:09:13.731558   18013 server.go:303] Kill server "services"
I0215 01:09:13.731571   18013 server.go:340] Killing process 18092 (services) with -TERM
I0215 01:09:13.753719   18013 server.go:303] Kill server "kubelet"
I0215 01:09:13.757091   18013 services.go:145] Fetching log files...
I0215 01:09:13.757158   18013 services.go:154] Get log file "kubelet.log" with journalctl command [-u kubelet-1777808992.service].
I0215 01:09:13.965030   18013 services.go:154] Get log file "kern.log" with journalctl command [-k].
I0215 01:09:13.972477   18013 services.go:154] Get log file "cloud-init.log" with journalctl command [-u cloud*].
I0215 01:09:13.976059   18013 services.go:154] Get log file "docker.log" with journalctl command [-u docker].
I0215 01:09:13.978788   18013 e2e_node_suite_test.go:187] Tests Finished

Ran 1 of 260 Specs in 230.920 seconds
SUCCESS! -- 1 Passed | 0 Failed | 0 Pending | 259 Skipped PASS

Ginkgo ran 1 suite in 3m51.307633834s
Test Suite Passed
@mtaufen

This comment has been minimized.

Contributor

mtaufen commented Feb 15, 2018

Need to update the fuzzer (pkg/kubelet/apis/kubeletconfig/fuzzer) with a non-empty value for your field that has a default and also update KubeletConfigurationPathFields in pkg/kubelet/apis/kubeletconfig/helpers_test.go.

obj.ContainerLogMaxSize = "10Mi"
}
if obj.ContainerLogMaxFiles == nil {
temp := int32(5)

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

use utilpointer.Int32Var(5)

This comment has been minimized.

@Random-Liu
@@ -254,6 +254,10 @@ type KubeletConfiguration struct {
FeatureGates map[string]bool `json:"featureGates,omitempty"`
// Tells the Kubelet to fail to start if swap is enabled on the node.
FailSwapOn *bool `json:"failSwapOn,omitempty"`
// A quantity defines the maximum size of the container log file before it is rotated. For example: "5Mi" or "256Ki".
ContainerLogMaxSize string `json:"containerLogMaxSize"`

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

Please format the comment as follows:

// description-text
// Default: val
// +optional

and also add omitempty in the json tag.

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

(same format as #53833)

This comment has been minimized.

@Random-Liu
// A quantity defines the maximum size of the container log file before it is rotated. For example: "5Mi" or "256Ki".
ContainerLogMaxSize string `json:"containerLogMaxSize"`
// Maximum number of container log files that can be present for a container.
ContainerLogMaxFiles *int32 `json:"containerLogMaxFiles"`

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

comment and omitempty here as well

This comment has been minimized.

@Random-Liu
@@ -841,6 +841,7 @@ func (m *kubeGenericRuntimeManager) removeContainerLog(containerID string) error
}
labeledInfo := getContainerInfoFromLabels(status.Labels)
annotatedInfo := getContainerInfoFromAnnotations(status.Annotations)
// TODO(random-liu): Add container log directory and cleanup directory.

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

If this is for a future PR, please open an issue and include the issue number in the comment, e.g. // TODO(#issue): issue-title.

This comment has been minimized.

@Random-Liu

Random-Liu Feb 16, 2018

Member

I'll make sure this is addressed in #59906. Already commented there.

Will remove this TODO.

@@ -465,5 +486,12 @@ func (r *FakeRuntimeService) ReopenContainerLog(containerID string) error {
defer r.Unlock()
r.Called = append(r.Called, "ReopenContainerLog")
// TODO: Add error injection support in other functions and

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

Please file an issue for this TODO, similar to below.

This comment has been minimized.

@Random-Liu

Random-Liu Feb 16, 2018

Member

Will remove this TODO then. I don't think it worth an issue.

@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 15, 2018

Need to update the fuzzer (pkg/kubelet/apis/kubeletconfig/fuzzer) with a non-empty value for your field that has a default and also update KubeletConfigurationPathFields in pkg/kubelet/apis/kubeletconfig/helpers_test.go.

@mtaufen Done. Thanks a lot!

@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 15, 2018

/retest

// TODO(random-liu): Leverage this function to support log rotation in `kubectl logs`.
func GetAllLogs(log string) ([]string, error) {
// pattern is used to match all rotated files.
pattern := fmt.Sprintf("%s.*", log)

This comment has been minimized.

@mtaufen

mtaufen Feb 15, 2018

Contributor

Is the log string user provided/should we sanitize or validate it?

This comment has been minimized.

@Random-Liu

Random-Liu Feb 16, 2018

Member

This is not a direct user facing function. :)
It is only used for testing now. Even in the future, it will only be used by kubelet/crictl.

This comment has been minimized.

@mtaufen

mtaufen Feb 16, 2018

Contributor

ok, sounds good

@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 16, 2018

@mtaufen Addressed comments.

@@ -757,6 +758,22 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,
}
klet.imageManager = imageManager
if containerRuntime == kubetypes.RemoteContainerRuntime && utilfeature.DefaultFeatureGate.Enabled(features.CRIContainerLogRotation) {
maxSize, err := logs.ParseMaxSize(kubeCfg.ContainerLogMaxSize)

This comment has been minimized.

@dashpole

dashpole Feb 16, 2018

Contributor

I think it would be nice to keep this logic in the log container manager. You could just pass both kubeCfg options to NewContainerLogManager, and do the parsing inside that function. Then there is no need to make ParseMaxSize an exported function.

This comment has been minimized.

This comment has been minimized.

@Random-Liu

Random-Liu Feb 16, 2018

Member

I can move it into NewContainerLogMnager. :) I actually prefer that.

This comment has been minimized.

@dashpole

dashpole Feb 16, 2018

Contributor

I think the eviction thresholds are used by the container manager as well?

This comment has been minimized.

@Random-Liu
@@ -1334,6 +1354,11 @@ func (kl *Kubelet) initializeRuntimeDependentModules() {
// Fail kubelet and rely on the babysitter to retry starting kubelet.
glog.Fatalf("Failed to start ContainerManager %v", err)
}
// container log manager must start after container runtime is up to retrieve information from container runtime
// and inform container to reopen log file after log rotation.
if kl.containerLogManager != nil {

This comment has been minimized.

@dashpole

dashpole Feb 16, 2018

Contributor

Instead of having a nil containerLogManager, can we instead make a ContainerLogManagerStub for use with non-cri runtimes? This way when we add other methods or calls to the ContainerLogManager interface, we dont need to do nil-checking everywhere

This comment has been minimized.

@Random-Liu

Random-Liu Feb 16, 2018

Member

Good idea.

This comment has been minimized.

@Random-Liu
// container logs and performs log rotation.
logMonitorPeriod = 10 * time.Second
// timestampFormat is format of the timestamp suffix for rotated log.
timestampFormat = "20060102-150405"

This comment has been minimized.

@dashpole

dashpole Feb 16, 2018

Contributor

where does this come from? if there is a website or docs or something, can we link to it here?

This comment has been minimized.

@Random-Liu

This comment has been minimized.

@Random-Liu
const (
// logMonitorPeriod is the period container log manager monitors
// container logs and performs log rotation.
logMonitorPeriod = 10 * time.Second

This comment has been minimized.

@dashpole

dashpole Feb 16, 2018

Contributor

is listing all containers every 10 seconds necessary? Would it be better to use information from a cache?

This comment has been minimized.

@Random-Liu

Random-Liu Feb 17, 2018

Member

is listing all containers every 10 seconds necessary? Would it be better to use information from a cache?

We only rotate log for running container, so we do still need the latest state of the container.

But actually we could use kubelet pod cache, I thought about that, we just need to add a List function in the cache.

I'm fine with that actually.

This comment has been minimized.

@Random-Liu

Random-Liu Feb 17, 2018

Member

I can do that as a following up optimization. It does need some more change. Will file an issue and add TODO here.

Are you OK with that?

This comment has been minimized.

@dashpole

dashpole Feb 17, 2018

Contributor

sgtm

This comment has been minimized.

@Random-Liu

@k8s-merge-robot k8s-merge-robot removed the lgtm label Feb 21, 2018

@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 21, 2018

Just did a rebase. Apply LGTM based on #59898 (comment)

@Random-Liu Random-Liu added the lgtm label Feb 21, 2018

@jberkus

This comment has been minimized.

jberkus commented Feb 21, 2018

as we are now in code slush, this PR requires status/approved-for-milestone in order to merge. Please add that when ready, thanks!

@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 21, 2018

@dchen1107 Ping for approval. :)

@feiskyer

lgtm

@dchen1107

This comment has been minimized.

Member

dchen1107 commented Feb 23, 2018

/lgtm

@k8s-ci-robot

This comment has been minimized.

Contributor

k8s-ci-robot commented Feb 23, 2018

[APPROVALNOTIFIER] This PR is APPROVED

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

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-merge-robot k8s-merge-robot removed the lgtm label Feb 23, 2018

@Random-Liu

This comment has been minimized.

Member

Random-Liu commented Feb 23, 2018

Just did a rebase. Reapply the lgtm.

@Random-Liu Random-Liu added lgtm approved and removed approved labels Feb 23, 2018

@k8s-merge-robot

This comment has been minimized.

Contributor

k8s-merge-robot commented Feb 23, 2018

Automatic merge from submit-queue (batch tested with PRs 60214, 58762, 59898, 59897, 60204). If you want to cherry-pick this change to another branch, please follow the instructions here.

@k8s-merge-robot k8s-merge-robot merged commit b38f1b9 into kubernetes:master Feb 23, 2018

13 checks passed

Submit Queue Queued to run github e2e tests a second time.
Details
cla/linuxfoundation Random-Liu authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-cross Skipped
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Skipped
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce Job succeeded.
Details
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-unit Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details

@Random-Liu Random-Liu deleted the Random-Liu:add-log-rotation branch Feb 23, 2018

@@ -527,6 +527,8 @@ func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfig
fs.BoolVar(&c.MakeIPTablesUtilChains, "make-iptables-util-chains", c.MakeIPTablesUtilChains, "If true, kubelet will ensure iptables utility rules are present on host.")
fs.Int32Var(&c.IPTablesMasqueradeBit, "iptables-masquerade-bit", c.IPTablesMasqueradeBit, "The bit of the fwmark space to mark packets for SNAT. Must be within the range [0, 31]. Please match this parameter with corresponding parameter in kube-proxy.")
fs.Int32Var(&c.IPTablesDropBit, "iptables-drop-bit", c.IPTablesDropBit, "The bit of the fwmark space to mark packets for dropping. Must be within the range [0, 31].")
fs.StringVar(&c.ContainerLogMaxSize, "container-log-max-size", c.ContainerLogMaxSize, "<Warning: Alpha feature> Set the maximum size (e.g. 10Mi) of container log file before it is rotated.")

This comment has been minimized.

@yujuhong

yujuhong Feb 26, 2018

Contributor

Nit: should probably make it clear that This flag can only be used with --container-runtime=remote
The same applies to the flag below.

k8s-merge-robot added a commit that referenced this pull request May 14, 2018

Merge pull request #62833 from charrywanganthony/log_rotation
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Add necessary explanation for container log rotation.

**What this PR does / why we need it**:
#59898
> Container log manager only starts when the container runtime is remote (not docker), because we can't implement ReopenContainerLog for docker.

**Release note**:
```release-note
NONE
```
@oadekoya

This comment has been minimized.

oadekoya commented Jul 13, 2018

@Random-Liu what does when container runtime is remote mean?

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