craigcondit commented on code in PR #623:
URL: https://github.com/apache/yunikorn-k8shim/pull/623#discussion_r1244264667
##########
pkg/schedulerplugin/scheduler_plugin.go:
##########
@@ -216,17 +216,17 @@ func (sp *YuniKornSchedulerPlugin) PostBind(_
context.Context, _ *framework.Cycl
// NewSchedulerPlugin initializes a new plugin and returns it
func NewSchedulerPlugin(_ runtime.Object, handle framework.Handle)
(framework.Plugin, error) {
- log.SetDefaultLogger(log.K8Shim)
- log.Logger().Info(fmt.Sprintf("Build info: version=%s date=%s
isPluginVersion=%t goVersion=%s arch=%s coreSHA=%s siSHA=%s shimSHA=%s",
conf.BuildVersion, conf.BuildDate, conf.IsPluginVersion, conf.GoVersion,
conf.Arch, conf.CoreSHA, conf.SiSHA, conf.ShimSHA))
+ log.SetDefaultLogger(log.ShimSchedulerPlugin)
Review Comment:
This should still be `log.SetDefaultLogger(log.Shim)` -- almost all the code
still resolves to shim, only the plugin code is specific.
##########
pkg/admission/pki/certs.go:
##########
Review Comment:
Let's use `log.AdmissionUtils` for the `pkg/admission/pki/*` files.
##########
pkg/log/logger.go:
##########
@@ -51,20 +51,46 @@ const (
levelSuffix = ".level"
)
-// Predefined loggers: when adding new loggers, ids must be sequential, and
all must be added to the loggers slice in the same order
+// Defined loggers: when adding new loggers, ids must be sequential, and all
must be added to the loggers slice in the same order
var (
- K8Shim = &LoggerHandle{id: 1, name: "k8shim"}
- Kubernetes = &LoggerHandle{id: 2, name: "kubernetes"}
- Admission = &LoggerHandle{id: 3, name: "admission"}
- Test = &LoggerHandle{id: 4, name: "test"}
+ Kubernetes = &LoggerHandle{id: 1, name: "kubernetes"}
+ Test = &LoggerHandle{id: 2, name: "test"}
+ Admission = &LoggerHandle{id: 3, name: "admission"}
+ AdmissionClient = &LoggerHandle{id: 4, name:
"admission.client"}
+ AdmissionController = &LoggerHandle{id: 5, name:
"admission.controller"}
Review Comment:
Introduce `log.AdmissionConf` => `admission.conf` for configuration-related
admission controller logs.
##########
pkg/common/si_helper.go:
##########
@@ -42,7 +42,7 @@ func CreateTagsForTask(pod *v1.Pod) map[string]string {
if value.Kind == constants.DaemonSetType {
if pod.Spec.Affinity == nil ||
pod.Spec.Affinity.NodeAffinity == nil ||
pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution
== nil {
- log.Logger().Debug("DaemonSet pod's
Affinity, NodeAffinity, RequiredDuringSchedulingIgnoredDuringExecution might
empty")
+
log.Log(log.ShimContext).Debug("DaemonSet pod's Affinity, NodeAffinity,
RequiredDuringSchedulingIgnoredDuringExecution might empty")
Review Comment:
Let's use `log.ShimUtils` for this as that's logically where this goes
(helper is a synonym for utility).
##########
pkg/log/logger.go:
##########
@@ -51,20 +51,46 @@ const (
levelSuffix = ".level"
)
-// Predefined loggers: when adding new loggers, ids must be sequential, and
all must be added to the loggers slice in the same order
+// Defined loggers: when adding new loggers, ids must be sequential, and all
must be added to the loggers slice in the same order
var (
- K8Shim = &LoggerHandle{id: 1, name: "k8shim"}
- Kubernetes = &LoggerHandle{id: 2, name: "kubernetes"}
- Admission = &LoggerHandle{id: 3, name: "admission"}
- Test = &LoggerHandle{id: 4, name: "test"}
+ Kubernetes = &LoggerHandle{id: 1, name: "kubernetes"}
+ Test = &LoggerHandle{id: 2, name: "test"}
+ Admission = &LoggerHandle{id: 3, name: "admission"}
+ AdmissionClient = &LoggerHandle{id: 4, name:
"admission.client"}
+ AdmissionController = &LoggerHandle{id: 5, name:
"admission.controller"}
+ AdmissionWebhook = &LoggerHandle{id: 6, name:
"admission.webhook"}
+ AdmissionUtils = &LoggerHandle{id: 7, name: "admission.utils"}
+ Shim = &LoggerHandle{id: 8, name: "shim"}
Review Comment:
Put `Shim` as the first logger, with ID 1, as this is the default if none is
provided.
##########
pkg/admission/conf/am_conf.go:
##########
Review Comment:
Let's introduce `log.AdmissionConf` => `admission.conf` for these.
##########
pkg/log/logger.go:
##########
@@ -51,20 +51,46 @@ const (
levelSuffix = ".level"
)
-// Predefined loggers: when adding new loggers, ids must be sequential, and
all must be added to the loggers slice in the same order
+// Defined loggers: when adding new loggers, ids must be sequential, and all
must be added to the loggers slice in the same order
var (
- K8Shim = &LoggerHandle{id: 1, name: "k8shim"}
- Kubernetes = &LoggerHandle{id: 2, name: "kubernetes"}
- Admission = &LoggerHandle{id: 3, name: "admission"}
- Test = &LoggerHandle{id: 4, name: "test"}
+ Kubernetes = &LoggerHandle{id: 1, name: "kubernetes"}
+ Test = &LoggerHandle{id: 2, name: "test"}
+ Admission = &LoggerHandle{id: 3, name: "admission"}
+ AdmissionClient = &LoggerHandle{id: 4, name:
"admission.client"}
+ AdmissionController = &LoggerHandle{id: 5, name:
"admission.controller"}
Review Comment:
Remove `log.AdmissionController` and replace all usages with `log.Admission`
as these are synonyms.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
