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

set logger for controller-runtime framework #2490

Merged

Conversation

Icarus9913
Copy link
Collaborator

controller-runtime framework add stack print for no logger initialization: kubernetes-sigs/controller-runtime#2357

		if logFullfilled.CompareAndSwap(false, true) {
			stack := debug.Stack()
			stackLines := bytes.Count(stack, []byte{'\n'})
			sep := []byte{'\n', '\t', '>', ' ', ' '}

			fmt.Fprintf(os.Stderr,
				"[controller-runtime] log.SetLogger(...) was never called; logs will not be displayed.\nDetected at:%s%s", sep,
				// prefix every line, so it's clear this is a stack trace related to the above message
				bytes.Replace(stack, []byte{'\n'}, sep, stackLines-1),
			)
			SetLogger(logr.New(NullLogSink{}))
		}
	}

And the controller-runtime would set null logger for its usage. So, I just pre-do it in spiderpool to avoid debug stack print.

Signed-off-by: Icarus9913 icaruswu66@qq.com

What this PR does / why we need it:
enhancement

Which issue(s) this PR fixes:
close #2450

@Icarus9913 Icarus9913 added pr/ready-review This pull is ready for review enhancement source codes enhancement release/none no release note labels Oct 30, 2023
@@ -26,10 +28,12 @@ func init() {
}

func newCRDManager() (ctrl.Manager, error) {
// set logger for controller-runtime framework
ctrl.SetLogger(logr.New(controllerruntimelog.NullLogSink{}))
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment 可以记下 issue 链接

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

@codecov
Copy link

codecov bot commented Oct 30, 2023

Codecov Report

Merging #2490 (d7ff9c7) into main (2752f70) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2490   +/-   ##
=======================================
  Coverage   80.79%   80.79%           
=======================================
  Files          49       49           
  Lines        5280     5280           
=======================================
  Hits         4266     4266           
  Misses        857      857           
  Partials      157      157           
Flag Coverage Δ
unittests 80.79% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Signed-off-by: Icarus9913 <icaruswu66@qq.com>
@weizhoublue weizhoublue merged commit 4352581 into spidernet-io:main Oct 30, 2023
35 checks passed
@Icarus9913 Icarus9913 mentioned this pull request Oct 31, 2023
@Icarus9913 Icarus9913 deleted the fix/wk/controllerruntime-log branch November 5, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement source codes enhancement pr/ready-review This pull is ready for review release/none no release note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spider-controller pod log exception
2 participants