Kong / kubernetes-ingress-controller

:gorilla: Kong for Kubernetes: The official Ingress Controller for Kubernetes.
https://docs.konghq.com/kubernetes-ingress-controller/
Apache License 2.0
2.21k stars 591 forks source link

Output of entity changes in KIC's logs instead of `Println` in database-reconciler repo #5289

Open randmonkey opened 10 months ago

randmonkey commented 10 months ago

Is there an existing issue for this?

Problem Statement

Currently we enable output of go-database-reconciler in setup when log level is set to trace or debug: https://github.com/Kong/kubernetes-ingress-controller/blob/64a0a6105546dd85891871345af3f25b78ec0a06/internal/manager/setup.go#L48-L50

This uses cprint in https://github.com/Kong/go-database-reconciler to output the entity changes, which calls https://github.com/fatih/color for printing message on the terminal in ANSI colors. This output format is suitable for terminal but not for log files. It brings inconsistency in logs, and may print special characters for colors.

Proposed Solution

Additional information

We should also consider updating the code in https://github.com/Kong/go-database-reconciler after separated it from decK. This repo is not mainly used as a CLI as decK, so the direct output to terminal may need to be cleaned.

Acceptance Criteria

randmonkey commented 6 months ago

@rainest Could this be fixed after #5785 merged?

rainest commented 4 months ago

Not quite. https://github.com/Kong/kubernetes-ingress-controller/blob/5cefa9a44fe51fe09ebad0a82a7aba3b42bec2fa/internal/dataplane/sendconfig/dbmode.go#L94 is currently just discarding the list of change events from https://github.com/Kong/go-database-reconciler/blob/db51b8d3033b178fd0729c19c06429fb75ca40b3/pkg/diff/diff.go#L568 and only handling the errors.

We need to add a new log loop that does handle that with some sort of diff output, or add a debug endpoint for it. My original intent was to do the latter, since even with our own logger and without the colors, the diffs don't lend themselves well to logging because they require multiple lines to be legible, and break the usual log contract as such.

Having a endpoint that just prints the latest diff elsewhere displays that more cleanly. We could opt to still log basic "entity changed successfully" info alongside that.

Edit: nevermind, I forgot that return was for the legacy format. We do log changes through the new channel system in https://github.com/Kong/kubernetes-ingress-controller/blob/5cefa9a44fe51fe09ebad0a82a7aba3b42bec2fa/internal/dataplane/sendconfig/dbmode.go#L125-L134

Full diff endpoint pending https://github.com/Kong/kubernetes-ingress-controller/pull/6101