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

Perform GCE master log rotation check every 5 minutes #72062

Merged
merged 1 commit into from Jan 16, 2019

Conversation

@jpbetz
Copy link
Contributor

jpbetz commented Dec 14, 2018

Reduce how often we check if logs in the master need to be rotate from 1hr to 5min. Rotation policy stays the same (new day starts, log file size > 100MB). When logs are extremely spammy, the logs can grow far beyond what the rotation policy intends, this helps keep them better in check.

What this PR does / why we need it:

Reduce GCE log rotation check from 1 hour to every 5 minutes.  Rotation policy is unchanged (new day starts, log file size > 100MB).

cc @mborsz @jlowdermilk @cheftako @mml

Show resolved Hide resolved cluster/gce/gci/node.yaml Outdated

@jpbetz jpbetz force-pushed the jpbetz:gce-logrotate-check-interval branch from 5d3ec17 to c4ae8af Dec 17, 2018

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 17, 2018

/retest

[Timer]
OnCalendar=hourly
OnCalendar=*:0/5

This comment has been minimized.

@cheftako

cheftako Dec 17, 2018

Member

If we aren't going to doc every 5 minutes then the normalized form seems easier to read/understand *-*-* *:00/5:00

This comment has been minimized.

@jpbetz

jpbetz Dec 17, 2018

Author Contributor

Interesting, most of the systemd docs I found suggested the 0/5 form was more idiomatic, but I'm okay with either.

This comment has been minimized.

@cheftako

cheftako Dec 17, 2018

Member

For systemd experts it probably is more idiomatic as its shorter. However even the systemd tooling refers to the normalized form, which seems easier (to me) to read.
$ systemd-analyze calendar *:0/5
Original form: *:0/5
Normalized form: *-*-* *:00/5:00
Next elapse: Mon 2018-12-17 11:55:00 PST
(in UTC): Mon 2018-12-17 19:55:00 UTC
From now: 4min 55s left

This comment has been minimized.

@jpbetz

jpbetz Dec 17, 2018

Author Contributor

Applied

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 17, 2018

/retest

@jpbetz jpbetz force-pushed the jpbetz:gce-logrotate-check-interval branch from c4ae8af to 199dc09 Dec 17, 2018

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 17, 2018

/retest

@mborsz

This comment has been minimized.

Copy link
Member

mborsz commented Dec 18, 2018

/lgtm

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

/assign @yguo0905

Can I get approval? Thanks!

@yguo0905

This comment has been minimized.

Copy link
Contributor

yguo0905 commented Dec 18, 2018

Just to confirm - we want this change only on nodes but not masters?

/approve

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Dec 18, 2018

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jpbetz, yguo0905

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

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

@yguo0905 Yes, the intent here was to change it just for masters. But maybe we should cc someone on sig-node who might have an opinion if we should do something similar on node? Probably best to make it a separate PR at this point if they decide they want it.

@yguo0905

This comment has been minimized.

Copy link
Contributor

yguo0905 commented Dec 18, 2018

The master config is at cluster/gce/gci/master.yaml. The one changed in this PR is for nodes.

/hold

@jpbetz jpbetz force-pushed the jpbetz:gce-logrotate-check-interval branch from 199dc09 to 1ba05d5 Dec 18, 2018

@k8s-ci-robot k8s-ci-robot removed the lgtm label Dec 18, 2018

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

@yguo0905 Oops, fixed.

@yguo0905

This comment has been minimized.

Copy link
Contributor

yguo0905 commented Dec 18, 2018

/lgtm
Please change the PR description accordingly.

@k8s-ci-robot k8s-ci-robot added the lgtm label Dec 18, 2018

@jpbetz jpbetz changed the title Perform GCE log rotation check every 5 minutes Perform GCE master log rotation check every 5 minutes Dec 18, 2018

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

@yguo0905 Title and description updated to reflect that this is for the master.

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

/test pull-kubernetes-e2e-gke

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

/test pull-kubernetes-e2e-kops-aws

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

/test pull-kubernetes-e2e-gke

2 similar comments
@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 18, 2018

/test pull-kubernetes-e2e-gke

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Dec 20, 2018

/test pull-kubernetes-e2e-gke

@jpbetz

This comment has been minimized.

Copy link
Contributor Author

jpbetz commented Jan 16, 2019

removing hold since issue from #72062 (comment) is resolved

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 0713f29 into kubernetes:master Jan 16, 2019

19 checks passed

cla/linuxfoundation jpbetz 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-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gke Job succeeded.
Details
pull-kubernetes-e2e-kops-aws Job succeeded.
Details
pull-kubernetes-e2e-kubeadm-gce Skipped
pull-kubernetes-godeps Skipped
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped
pull-kubernetes-local-e2e-containerized Skipped
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
tide In merge pool.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment