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.

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

[windows] Improve kube-proxy logging and move calico logs to a better path #5248

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

manuelbuil
Copy link
Contributor

@manuelbuil manuelbuil commented Jan 11, 2024

Proposed Changes

Following on from what we do with Calico processes, use a file to dump the logs of kube-proxy in windows. The current Stdout and Stderr output is supposed to be piped to the Windows Logging System but it does not work. Using a file instead works.

This PR also moves Calico logs to the same place where kubelet logs are. That place is C:\var\lib\rancher\rke2\agent\logs which makes much more sense

Types of Changes

Bugfix

Verification

Check issue

Testing

Linked Issues

#5247

User-Facing Change


Further Comments

@manuelbuil manuelbuil requested a review from a team as a code owner January 11, 2024 15:03
@@ -67,6 +67,8 @@ const (
CNICalico = "calico"
CNICilium = "cilium"
CNICanal = "canal"
LogPath = "C:\\var\\log\\"
Copy link
Member

@brandond brandond Jan 11, 2024

Choose a reason for hiding this comment

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

The default kubelet log for linux is /var/lib/rancher/rke2/agent/logs/kubelet.log, why do we use c:/var/log for this instead of c:/var/lib/rancher/rke2/agent/logs? Can we just put this in the same place as the kubelet log?

logsDir := filepath.Join(dataDir, "agent", "logs")

"log-file=" + filepath.Join(logsDir, "kubelet.log"),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AH! I missed that already existing path! Thanks Brad! I'm actually going to move calico logs to there as well, I think it makes much more sense

@manuelbuil manuelbuil changed the title Use a file to log kube-proxy output in windows [windows] Improve kube-proxy logging and move calico logs to a better path Jan 12, 2024
@@ -220,11 +220,16 @@ func (p *PEBinaryConfig) KubeProxy(ctx context.Context, args []string) error {

logrus.Infof("Running RKE2 kube-proxy %s", args)
go func() {
outputFile, err := os.Create(filepath.Join(p.DataDir, "agent", "logs", "kube-proxy.log"))
Copy link
Member

Choose a reason for hiding this comment

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

I would recommend reusing or at least duplicating the code that we use to capture kubelet logs using lumberjack, which handles rotating and pruning logs. Otherwise this one log file may get very large.

// ExtractFromArgs extracts the legacy klog flags from an args list, and returns both the remaining args,
// and a similarly configured log writer configured using the klog flag values.
func ExtractFromArgs(args []string) ([]string, io.Writer) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea! I was actually wondering if these logs could get too large. I was not aware of this lumberjack library

Signed-off-by: Manuel Buil <mbuil@suse.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants