[GitHub] [yunikorn-k8shim] craigcondit commented on a diff in pull request #412: [YUNIKORN-966] Retrieve the username from the SparkApp CRD

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

craigcondit commented on code in PR #412:
URL: https://github.com/apache/yunikorn-k8shim/pull/412#discussion_r1013241679


##########
pkg/appmgmt/sparkoperator/spark.go:
##########
@@ -21,15 +21,19 @@ package sparkoperator
 import (
 	"github.com/GoogleCloudPlatform/spark-on-k8s-operator/pkg/apis/sparkoperator.k8s.io/v1beta2"
 	"go.uber.org/zap"
+	v1 "k8s.io/api/core/v1"
 	k8sCache "k8s.io/client-go/tools/cache"
 
 	"github.com/apache/yunikorn-k8shim/pkg/appmgmt/interfaces"
 	"github.com/apache/yunikorn-k8shim/pkg/client"
+	"github.com/apache/yunikorn-k8shim/pkg/common/constants"
 	"github.com/apache/yunikorn-k8shim/pkg/log"
 	crcClientSet "github.com/apache/yunikorn-k8shim/pkg/sparkclient/clientset/versioned"
 	crInformers "github.com/apache/yunikorn-k8shim/pkg/sparkclient/informers/externalversions"
 )
 
+var crdInformerFactory crInformers.SharedInformerFactory
+

Review Comment:
   Why is this a global? We already have it as part of the Manager object.



##########
pkg/appmgmt/sparkoperator/spark.go:
##########
@@ -124,3 +127,21 @@ func (os *Manager) deleteApplication(obj interface{}) {
 	log.Logger().Info("spark app deleted", zap.Any("SparkApplication", app))
 	os.amProtocol.NotifyApplicationComplete(app.Status.SparkApplicationID)
 }
+
+func GetProxyUser(pod *v1.Pod) string {
+	if appName, ok := pod.Labels[constants.SparkOperatorLabelAppName]; ok {
+		if crdInformerFactory == nil {
+			log.Logger().Info("Spark operator AppMgmt service is not initialized, so the username from the SparkApp CRD cannot be obtained")
+			return ""
+		}
+		app, err := crdInformerFactory.Sparkoperator().V1beta2().SparkApplications().Lister().SparkApplications(pod.Namespace).Get(appName)

Review Comment:
   This should not make an API server call but use the informers instead. Basically, cache all the Spark application objects in-memory (track add/update/delete events) and use those to lookup the proxy user.



-- 
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