[GitHub] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #326: [YUNIKORN-932] Added some extra log message

[GitBox <gi...@apache.org>]

wilfred-s commented on a change in pull request #326:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/326#discussion_r745286870



##########
File path: pkg/appmgmt/general/general.go
##########
@@ -320,13 +320,14 @@ func (os *Manager) deletePod(obj interface{}) {
 }
 
 func (os *Manager) ListApplications() (map[string]interfaces.ApplicationMetadata, error) {
+	log.Logger().Info("Listing recoverable apps")
 	// list all pods on this cluster
 	slt := labels.NewSelector()
 	appPods, err := os.apiProvider.GetAPIs().PodInformer.Lister().List(slt)
 	if err != nil {
 		return nil, err
 	}
-
+	log.Logger().Info("Got the list of pods from the api server", zap.Int("nr of pods", len(appPods)))

Review comment:
       "Got the list of pods from the api server" --> "Pod list retrieved from api server"

##########
File path: pkg/shim/scheduler.go
##########
@@ -256,7 +265,9 @@ func (ss *KubernetesShim) canHandle(se events.SchedulerEvent) bool {
 func (ss *KubernetesShim) schedule() {
 	apps := ss.context.SelectApplications(nil)
 	for _, app := range apps {
-		app.Schedule()
+		if app.Schedule() {
+			ss.outstandingAppsFound = true
+		}

Review comment:
       We should be able to simplify this to:
   ```
   ss.outstandingAppsFound = app.Schedule()
   ```

##########
File path: pkg/appmgmt/general/general.go
##########
@@ -320,13 +320,14 @@ func (os *Manager) deletePod(obj interface{}) {
 }
 
 func (os *Manager) ListApplications() (map[string]interfaces.ApplicationMetadata, error) {
+	log.Logger().Info("Listing recoverable apps")
 	// list all pods on this cluster
 	slt := labels.NewSelector()
 	appPods, err := os.apiProvider.GetAPIs().PodInformer.Lister().List(slt)
 	if err != nil {
 		return nil, err
 	}
-
+	log.Logger().Info("Got the list of pods from the api server", zap.Int("nr of pods", len(appPods)))
 	// get existing apps
 	existingApps := make(map[string]interfaces.ApplicationMetadata)
 	for _, pod := range appPods {

Review comment:
       Can we add counters for two extra pieces of info:
   * number of pods fitting general filter but no meta data (podsConsidered add after line 337)
   * number of pods linked to recoverable apps (podsRecovered add after line 339)
   
   Add both values to the new log line.

##########
File path: pkg/appmgmt/general/general.go
##########
@@ -342,6 +343,7 @@ func (os *Manager) ListApplications() (map[string]interfaces.ApplicationMetadata
 			}
 		}
 	}
+	log.Logger().Info("Listing recoverable apps finished", zap.Int("nr of recoverable apps", len(existingApps)))

Review comment:
       "Listing recoverable apps finished" --> "application recovery statistics"
   adding the two values mentioned above to the number of apps

##########
File path: pkg/appmgmt/appmgmt_recovery.go
##########
@@ -43,6 +43,7 @@ func (svc *AppManagementService) WaitForRecovery(maxTimeout time.Duration) error
 }
 
 func (svc *AppManagementService) recoverApps() (map[string]interfaces.ManagedApp, error) {
+	log.Logger().Info("Started app recovery")

Review comment:
       Started --> Starting

##########
File path: pkg/appmgmt/general/general.go
##########
@@ -320,13 +320,14 @@ func (os *Manager) deletePod(obj interface{}) {
 }
 
 func (os *Manager) ListApplications() (map[string]interfaces.ApplicationMetadata, error) {
+	log.Logger().Info("Listing recoverable apps")

Review comment:
       "Listing recoverable apps" --> "Retrieving pod list"




-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org