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

Added support for persisting logs in 'emptyDir' volume attached to agones controller. #620

Merged
merged 2 commits into from
Feb 26, 2019
Merged

Added support for persisting logs in 'emptyDir' volume attached to agones controller. #620

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8285096
Added support for persisting logs in 'emptyDir' volume attached to ag…
jkowalski Feb 22, 2019
7b7ec63
Merge branch 'master' into persistent-logs
jkowalski Feb 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Gopkg.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,3 +103,7 @@
[[constraint]]
name = "fortio.org/fortio"
version = "1.3.1"

[[constraint]]
name = "gopkg.in/natefinch/lumberjack.v2"
version = "2.1.0"
38 changes: 38 additions & 0 deletions cmd/controller/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package main

import (
"io"
"net/http"
"os"
"path/filepath"
Expand All @@ -39,8 +40,10 @@ import (
"github.com/heptiolabs/healthcheck"
"github.com/pkg/errors"
prom "github.com/prometheus/client_golang/prometheus"
"github.com/sirupsen/logrus"
"github.com/spf13/pflag"
"github.com/spf13/viper"
lumberjack "gopkg.in/natefinch/lumberjack.v2"
extclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
"k8s.io/apimachinery/pkg/api/resource"
"k8s.io/client-go/informers"
Expand All @@ -63,6 +66,8 @@ const (
numWorkersFlag = "num-workers"
apiServerSustainedQPSFlag = "api-server-qps"
apiServerBurstQPSFlag = "api-server-qps-burst"
logDirFlag = "log-dir"
logSizeLimitMBFlag = "log-size-limit-mb"
kubeconfigFlag = "kubeconfig"
defaultResync = 30 * time.Second
)
Expand All @@ -71,9 +76,32 @@ var (
logger = runtime.NewLoggerWithSource("main")
)

func setupLogging(logDir string, logSizeLimitMB int) {
logFileName := filepath.Join(logDir, "agones-controller-"+time.Now().Format("20060102_150405")+".log")

const maxLogSizeMB = 100
maxBackups := (logSizeLimitMB - maxLogSizeMB) / maxLogSizeMB
logger.WithField("filename", logFileName).WithField("numbackups", maxBackups).Info("logging to file")
logrus.SetOutput(
io.MultiWriter(
logrus.StandardLogger().Out,
&lumberjack.Logger{
Filename: logFileName,
MaxSize: maxLogSizeMB,
MaxBackups: maxBackups,
},
),
)
}

// main starts the operator for the gameserver CRD
func main() {
ctlConf := parseEnvFlags()

if ctlConf.LogDir != "" {
setupLogging(ctlConf.LogDir, ctlConf.LogSizeLimitMB)
}

logger.WithField("version", pkg.Version).
WithField("ctlConf", ctlConf).Info("starting gameServer operator...")

Expand Down Expand Up @@ -203,6 +231,8 @@ func parseEnvFlags() config {
viper.SetDefault(numWorkersFlag, 64)
viper.SetDefault(apiServerSustainedQPSFlag, 100)
viper.SetDefault(apiServerBurstQPSFlag, 200)
viper.SetDefault(logDirFlag, "")
viper.SetDefault(logSizeLimitMBFlag, 10000) // 10 GB, will be split into 100 MB chunks

pflag.String(sidecarImageFlag, viper.GetString(sidecarImageFlag), "Flag to overwrite the GameServer sidecar image that is used. Can also use SIDECAR env variable")
pflag.String(sidecarCPULimitFlag, viper.GetString(sidecarCPULimitFlag), "Flag to overwrite the GameServer sidecar container's cpu limit. Can also use SIDECAR_CPU_LIMIT env variable")
Expand All @@ -219,6 +249,8 @@ func parseEnvFlags() config {
pflag.Int32(numWorkersFlag, 64, "Number of controller workers per resource type")
pflag.Int32(apiServerSustainedQPSFlag, 100, "Maximum sustained queries per second to send to the API server")
pflag.Int32(apiServerBurstQPSFlag, 200, "Maximum burst queries per second to send to the API server")
pflag.String(logDirFlag, viper.GetString(logDirFlag), "If set, store logs in a given directory.")
pflag.Int32(logSizeLimitMBFlag, 1000, "Log file size limit in MB")
pflag.Parse()

viper.SetEnvKeyReplacer(strings.NewReplacer("-", "_"))
Expand All @@ -238,6 +270,8 @@ func parseEnvFlags() config {
runtime.Must(viper.BindEnv(numWorkersFlag))
runtime.Must(viper.BindEnv(apiServerSustainedQPSFlag))
runtime.Must(viper.BindEnv(apiServerBurstQPSFlag))
runtime.Must(viper.BindEnv(logDirFlag))
runtime.Must(viper.BindEnv(logSizeLimitMBFlag))

request, err := resource.ParseQuantity(viper.GetString(sidecarCPURequestFlag))
if err != nil {
Expand Down Expand Up @@ -265,6 +299,8 @@ func parseEnvFlags() config {
NumWorkers: int(viper.GetInt32(numWorkersFlag)),
APIServerSustainedQPS: int(viper.GetInt32(apiServerSustainedQPSFlag)),
APIServerBurstQPS: int(viper.GetInt32(apiServerBurstQPSFlag)),
LogDir: viper.GetString(logDirFlag),
LogSizeLimitMB: int(viper.GetInt32(logSizeLimitMBFlag)),
}
}

Expand All @@ -285,6 +321,8 @@ type config struct {
NumWorkers int
APIServerSustainedQPS int
APIServerBurstQPS int
LogDir string
LogSizeLimitMB int
}

// validate ensures the ctlConfig data is valid.
Expand Down
15 changes: 15 additions & 0 deletions install/helm/agones/templates/controller.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,12 @@ spec:
value: {{ .Values.agones.controller.apiServerQPS | quote }}
- name: API_SERVER_QPS_BURST
value: {{ .Values.agones.controller.apiServerQPSBurst | quote }}
{{- if .Values.agones.controller.persistentLogs }}
- name: LOG_DIR
value: "/home/agones/logs"
- name: LOG_SIZE_LIMIT_MB
value: {{ .Values.agones.controller.persistentLogsSizeLimitMB | quote }}
{{- end }}
livenessProbe:
httpGet:
path: /live
Expand All @@ -114,10 +120,19 @@ spec:
- name: certs
mountPath: /home/agones/certs
readOnly: true
{{- if .Values.agones.controller.persistentLogs }}
- name: logs
mountPath: /home/agones/logs
readOnly: false
{{- end }}
volumes:
- name: certs
secret:
secretName: {{ template "agones.fullname" . }}-cert
{{- if .Values.agones.controller.persistentLogs }}
markmandel marked this conversation as resolved.
Show resolved Hide resolved
- name: logs
emptyDir: {}
{{- end }}
{{- if .Values.agones.image.controller.pullSecret }}
imagePullSecrets:
- name: {{.Values.agones.image.controller.pullSecret}}
Expand Down
2 changes: 2 additions & 0 deletions install/helm/agones/values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ agones:
operator: Exists
generateTLS: true
safeToEvict: false
persistentLogs: true
Copy link
Member

Choose a reason for hiding this comment

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

Thought: Should this be on by default? I'm thinking most people won't be doing load tests, and stackdriver/fluentd are likely fast enough. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's not just load testing, we currently emit enough logs to overwhelm fluentd. For any fleets of significant size there are dropped messages.

Copy link
Member

Choose a reason for hiding this comment

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

Ah. In that case, yes - let's keep this on by default. I didn't realise we were getting dropped messages, that's unfortunate.

Sounds like something we should push upstream? (or should we be logging less?)

But otherwise, this makes sense, and looks good to me.

Only other thing I can think of - should we add a section in (to be released in 0.9.0) https://agones.dev/site/docs/guides/troubleshooting/ to let people know that this is an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, I'll submit this one now and make next PR with doc change and ability to use PVC for durable logs

persistentLogsSizeLimitMB: 10000
numWorkers: 100
apiServerQPS: 400
apiServerQPSBurst: 500
Expand Down
9 changes: 9 additions & 0 deletions install/yaml/install.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1079,6 +1079,10 @@ spec:
value: "400"
- name: API_SERVER_QPS_BURST
value: "500"
- name: LOG_DIR
value: "/home/agones/logs"
- name: LOG_SIZE_LIMIT_MB
value: "10000"
livenessProbe:
httpGet:
path: /live
Expand All @@ -1091,10 +1095,15 @@ spec:
- name: certs
mountPath: /home/agones/certs
readOnly: true
- name: logs
mountPath: /home/agones/logs
readOnly: false
volumes:
- name: certs
secret:
secretName: agones-manual-cert
- name: logs
emptyDir: {}

---
# Source: agones/templates/ping.yaml
Expand Down
2 changes: 2 additions & 0 deletions site/content/en/docs/Installation/helm.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ The following tables lists the configurable parameters of the Agones chart and t

| Parameter | Description | Default |
| --------------------------------------------------- | ----------------------------------------------------------------------------------------------- | ---------------------- |
| `agones.controller.persistentLogs` | Store Agones controller logs in a temporary volume attached to a container for debugging | `true` |
| `agones.controller.persistentLogsSizeLimitMB` | Maximum total size of all Agones container logs in MB | `10000` |

{{% /feature %}}

Expand Down
23 changes: 23 additions & 0 deletions vendor/gopkg.in/natefinch/lumberjack.v2/.gitignore
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions vendor/gopkg.in/natefinch/lumberjack.v2/LICENSE
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading