[GitHub] [incubator-yunikorn-k8shim] yangwwei commented on a change in pull request #256: [YUNIKORN-650] Retrieve user identity from predefined labels

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

yangwwei commented on a change in pull request #256:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/256#discussion_r619532081



##########
File path: pkg/common/constants/constants.go
##########
@@ -34,6 +34,8 @@ const AppTagNamespace = "namespace"
 const AppTagNamespaceResourceQuota = "namespace.resourcequota"
 const AppTagNamespaceParentQueue = "namespace.parentqueue"
 const DefaultAppNamespace = "default"
+const DefaultUserLabel = "yunikorn.apache.org/user"

Review comment:
       Suggest to use "yunikorn.apache.org/username"

##########
File path: pkg/common/utils/utils_test.go
##########
@@ -367,3 +367,27 @@ func TestMergeMaps(t *testing.T) {
 	assert.Equal(t, result["c"], "c1")
 	assert.Equal(t, result["d"], "d2")
 }
+
+func TestGetUserFromPod(t *testing.T) {
+	userInLabel := "testuser"
+	userNotInLabel := "default"

Review comment:
       pls use the constant: `constant.DefaultUser`

##########
File path: pkg/common/constants/constants.go
##########
@@ -34,6 +34,8 @@ const AppTagNamespace = "namespace"
 const AppTagNamespaceResourceQuota = "namespace.resourcequota"
 const AppTagNamespaceParentQueue = "namespace.parentqueue"
 const DefaultAppNamespace = "default"
+const DefaultUserLabel = "yunikorn.apache.org/user"
+const DefaultUser = "default"

Review comment:
       would it be better to use "nobody" as the default user name?

##########
File path: pkg/conf/schedulerconf.go
##########
@@ -150,6 +151,8 @@ func initConfigs() {
 	enableConfigHotRefresh := flag.Bool("enableConfigHotRefresh", false, "Flag for enabling "+
 		"configuration hot-refresh. If this value is set to true, the configuration updates in the configmap will be "+
 		"automatically reloaded without restarting the scheduler.")
+	userLabelKey := flag.String("userLabelKey", constants.DefaultUserLabel,
+		"provides label key to be used to identify user")

Review comment:
       nit: provide pod label key to be used to identify an 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.

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