Skip to content

Why is a Logger not injected by the Controller Manager? #1056

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

Closed
akutz opened this issue Jul 19, 2020 · 5 comments · Fixed by #1072
Closed

Why is a Logger not injected by the Controller Manager? #1056

akutz opened this issue Jul 19, 2020 · 5 comments · Fixed by #1072
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Milestone

Comments

@akutz
Copy link
Contributor

akutz commented Jul 19, 2020

Hi there,

Along with the other injector-style interfaces, there is InjectLogger,

// Logger is used to inject Loggers into components that need them
// and don't otherwise have opinions.
type Logger interface {
InjectLogger(l logr.Logger) error
}

However, the only place this seems to be used is with respect to webhooks.

Why is InjectLogger not part of the SetFields function in the Controller Manager so loggers are added to components added to the manager at

func (cm *controllerManager) SetFields(i interface{}) error {
if _, err := inject.ConfigInto(cm.config, i); err != nil {
return err
}
if _, err := inject.ClientInto(cm.client, i); err != nil {
return err
}
if _, err := inject.APIReaderInto(cm.apiReader, i); err != nil {
return err
}
if _, err := inject.SchemeInto(cm.scheme, i); err != nil {
return err
}
if _, err := inject.CacheInto(cm.cache, i); err != nil {
return err
}
if _, err := inject.InjectorInto(cm.SetFields, i); err != nil {
return err
}
if _, err := inject.StopChannelInto(cm.internalStop, i); err != nil {
return err
}
if _, err := inject.MapperInto(cm.mapper, i); err != nil {
return err
}
return nil
}

@alvaroaleman
Copy link
Member

Looks like an oversight

@vincepri
Copy link
Member

/milestone v0.6.x
/kind bug

Can/Should also be backported.

@k8s-ci-robot k8s-ci-robot added this to the v0.6.x milestone Jul 22, 2020
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 22, 2020
@vincepri
Copy link
Member

/help

@k8s-ci-robot
Copy link
Contributor

@vincepri:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jul 22, 2020
@jiachengxu
Copy link
Contributor

Would like to work on this :)
/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants