From abafc5850e1a06cc23b785dce90ed1ad3bc80807 Mon Sep 17 00:00:00 2001 From: Manikandan R Date: Mon, 10 Jul 2023 17:18:03 +0530 Subject: [PATCH] Addressed review comments --- pkg/appmgmt/general/podevent_handler.go | 2 +- pkg/appmgmt/general/podevent_handler_test.go | 2 +- pkg/conf/schedulerconf.go | 176 +++++++++---------- pkg/conf/schedulerconf_test.go | 6 +- 4 files changed, 93 insertions(+), 93 deletions(-) diff --git a/pkg/appmgmt/general/podevent_handler.go b/pkg/appmgmt/general/podevent_handler.go index 3f7c02616..52e11635d 100644 --- a/pkg/appmgmt/general/podevent_handler.go +++ b/pkg/appmgmt/general/podevent_handler.go @@ -128,7 +128,7 @@ func (p *PodEventHandler) addPod(pod *v1.Pod, eventSource EventSource) interface Metadata: appMeta, }) } else { - if conf.GetSchedulerConf().GetAllowSimilarAppIdsByDifferentUsers() && app.GetApplicationState() == "Running" && app.GetUser() != appMeta.User { + if conf.GetSchedulerConf().GetSingleUserPerApplication() && app.GetUser() != appMeta.User { log.Log(log.ShimAppMgmtGeneral).Warn("application has been submitted by different user", zap.String("app id ", appMeta.ApplicationID), zap.String("app user", app.GetUser()), diff --git a/pkg/appmgmt/general/podevent_handler_test.go b/pkg/appmgmt/general/podevent_handler_test.go index bccc3ae41..6a306f633 100644 --- a/pkg/appmgmt/general/podevent_handler_test.go +++ b/pkg/appmgmt/general/podevent_handler_test.go @@ -119,7 +119,7 @@ func TestAllowSimilarAppIdsByDifferentUsers(t *testing.T) { // set allowSimilarAppIdsByDifferentUsers to true err := conf.UpdateConfigMaps([]*v1.ConfigMap{ - {Data: map[string]string{conf.CMSvcAllowSimilarAppIdsByDifferentUsers: "true"}}, + {Data: map[string]string{conf.CMSvcSingleUserPerApplication: "true"}}, }, true) assert.NilError(t, err, "UpdateConfigMap failed") diff --git a/pkg/conf/schedulerconf.go b/pkg/conf/schedulerconf.go index 7eab64b95..ccd6baf86 100644 --- a/pkg/conf/schedulerconf.go +++ b/pkg/conf/schedulerconf.go @@ -53,37 +53,37 @@ const ( PrefixKubernetes = "kubernetes." // service - CMSvcClusterID = PrefixService + "clusterId" - CMSvcPolicyGroup = PrefixService + "policyGroup" - CMSvcSchedulingInterval = PrefixService + "schedulingInterval" - CMSvcVolumeBindTimeout = PrefixService + "volumeBindTimeout" - CMSvcEventChannelCapacity = PrefixService + "eventChannelCapacity" - CMSvcDispatchTimeout = PrefixService + "dispatchTimeout" - CMSvcOperatorPlugins = PrefixService + "operatorPlugins" - CMSvcDisableGangScheduling = PrefixService + "disableGangScheduling" - CMSvcEnableConfigHotRefresh = PrefixService + "enableConfigHotRefresh" - CMSvcPlaceholderImage = PrefixService + "placeholderImage" - CMSvcNodeInstanceTypeNodeLabelKey = PrefixService + "nodeInstanceTypeNodeLabelKey" - CMSvcAllowSimilarAppIdsByDifferentUsers = PrefixService + "allowSimilarAppIdsByDifferentUsers" + CMSvcClusterID = PrefixService + "clusterId" + CMSvcPolicyGroup = PrefixService + "policyGroup" + CMSvcSchedulingInterval = PrefixService + "schedulingInterval" + CMSvcVolumeBindTimeout = PrefixService + "volumeBindTimeout" + CMSvcEventChannelCapacity = PrefixService + "eventChannelCapacity" + CMSvcDispatchTimeout = PrefixService + "dispatchTimeout" + CMSvcOperatorPlugins = PrefixService + "operatorPlugins" + CMSvcDisableGangScheduling = PrefixService + "disableGangScheduling" + CMSvcEnableConfigHotRefresh = PrefixService + "enableConfigHotRefresh" + CMSvcPlaceholderImage = PrefixService + "placeholderImage" + CMSvcNodeInstanceTypeNodeLabelKey = PrefixService + "nodeInstanceTypeNodeLabelKey" + CMSvcSingleUserPerApplication = PrefixService + "singleUserPerApplication" // kubernetes CMKubeQPS = PrefixKubernetes + "qps" CMKubeBurst = PrefixKubernetes + "burst" // defaults - DefaultNamespace = "default" - DefaultClusterID = "mycluster" - DefaultPolicyGroup = "queues" - DefaultSchedulingInterval = time.Second - DefaultVolumeBindTimeout = 10 * time.Second - DefaultEventChannelCapacity = 1024 * 1024 - DefaultDispatchTimeout = 300 * time.Second - DefaultOperatorPlugins = "general" - DefaultDisableGangScheduling = false - DefaultEnableConfigHotRefresh = true - DefaultKubeQPS = 1000 - DefaultKubeBurst = 1000 - DefaultAllowSimilarAppIdsByDifferentUsers = false + DefaultNamespace = "default" + DefaultClusterID = "mycluster" + DefaultPolicyGroup = "queues" + DefaultSchedulingInterval = time.Second + DefaultVolumeBindTimeout = 10 * time.Second + DefaultEventChannelCapacity = 1024 * 1024 + DefaultDispatchTimeout = 300 * time.Second + DefaultOperatorPlugins = "general" + DefaultDisableGangScheduling = false + DefaultEnableConfigHotRefresh = true + DefaultKubeQPS = 1000 + DefaultKubeBurst = 1000 + DefaultSingleUserPerApplication = false ) var ( @@ -103,26 +103,26 @@ var confHolder atomic.Value var kubeLoggerOnce sync.Once type SchedulerConf struct { - SchedulerName string `json:"schedulerName"` - ClusterID string `json:"clusterId"` - ClusterVersion string `json:"clusterVersion"` - PolicyGroup string `json:"policyGroup"` - Interval time.Duration `json:"schedulingIntervalSecond"` - KubeConfig string `json:"absoluteKubeConfigFilePath"` - VolumeBindTimeout time.Duration `json:"volumeBindTimeout"` - TestMode bool `json:"testMode"` - EventChannelCapacity int `json:"eventChannelCapacity"` - DispatchTimeout time.Duration `json:"dispatchTimeout"` - KubeQPS int `json:"kubeQPS"` - KubeBurst int `json:"kubeBurst"` - OperatorPlugins string `json:"operatorPlugins"` - EnableConfigHotRefresh bool `json:"enableConfigHotRefresh"` - DisableGangScheduling bool `json:"disableGangScheduling"` - UserLabelKey string `json:"userLabelKey"` - PlaceHolderImage string `json:"placeHolderImage"` - InstanceTypeNodeLabelKey string `json:"instanceTypeNodeLabelKey"` - Namespace string `json:"namespace"` - AllowSimilarAppIdsByDifferentUsers bool `json:"allowSimilarAppIdsByDifferentUsers"` + SchedulerName string `json:"schedulerName"` + ClusterID string `json:"clusterId"` + ClusterVersion string `json:"clusterVersion"` + PolicyGroup string `json:"policyGroup"` + Interval time.Duration `json:"schedulingIntervalSecond"` + KubeConfig string `json:"absoluteKubeConfigFilePath"` + VolumeBindTimeout time.Duration `json:"volumeBindTimeout"` + TestMode bool `json:"testMode"` + EventChannelCapacity int `json:"eventChannelCapacity"` + DispatchTimeout time.Duration `json:"dispatchTimeout"` + KubeQPS int `json:"kubeQPS"` + KubeBurst int `json:"kubeBurst"` + OperatorPlugins string `json:"operatorPlugins"` + EnableConfigHotRefresh bool `json:"enableConfigHotRefresh"` + DisableGangScheduling bool `json:"disableGangScheduling"` + UserLabelKey string `json:"userLabelKey"` + PlaceHolderImage string `json:"placeHolderImage"` + InstanceTypeNodeLabelKey string `json:"instanceTypeNodeLabelKey"` + Namespace string `json:"namespace"` + SingleUserPerApplication bool `json:"singleUserPerApplication"` sync.RWMutex } @@ -131,25 +131,25 @@ func (conf *SchedulerConf) Clone() *SchedulerConf { defer conf.RUnlock() return &SchedulerConf{ - SchedulerName: conf.SchedulerName, - ClusterID: conf.ClusterID, - ClusterVersion: conf.ClusterVersion, - PolicyGroup: conf.PolicyGroup, - Interval: conf.Interval, - KubeConfig: conf.KubeConfig, - VolumeBindTimeout: conf.VolumeBindTimeout, - TestMode: conf.TestMode, - EventChannelCapacity: conf.EventChannelCapacity, - DispatchTimeout: conf.DispatchTimeout, - KubeQPS: conf.KubeQPS, - KubeBurst: conf.KubeBurst, - OperatorPlugins: conf.OperatorPlugins, - EnableConfigHotRefresh: conf.EnableConfigHotRefresh, - DisableGangScheduling: conf.DisableGangScheduling, - UserLabelKey: conf.UserLabelKey, - PlaceHolderImage: conf.PlaceHolderImage, - Namespace: conf.Namespace, - AllowSimilarAppIdsByDifferentUsers: conf.AllowSimilarAppIdsByDifferentUsers, + SchedulerName: conf.SchedulerName, + ClusterID: conf.ClusterID, + ClusterVersion: conf.ClusterVersion, + PolicyGroup: conf.PolicyGroup, + Interval: conf.Interval, + KubeConfig: conf.KubeConfig, + VolumeBindTimeout: conf.VolumeBindTimeout, + TestMode: conf.TestMode, + EventChannelCapacity: conf.EventChannelCapacity, + DispatchTimeout: conf.DispatchTimeout, + KubeQPS: conf.KubeQPS, + KubeBurst: conf.KubeBurst, + OperatorPlugins: conf.OperatorPlugins, + EnableConfigHotRefresh: conf.EnableConfigHotRefresh, + DisableGangScheduling: conf.DisableGangScheduling, + UserLabelKey: conf.UserLabelKey, + PlaceHolderImage: conf.PlaceHolderImage, + Namespace: conf.Namespace, + SingleUserPerApplication: conf.SingleUserPerApplication, } } @@ -207,7 +207,7 @@ func handleNonReloadableConfig(old *SchedulerConf, new *SchedulerConf) { checkNonReloadableBool(CMSvcDisableGangScheduling, &old.DisableGangScheduling, &new.DisableGangScheduling) checkNonReloadableString(CMSvcPlaceholderImage, &old.PlaceHolderImage, &new.PlaceHolderImage) checkNonReloadableString(CMSvcNodeInstanceTypeNodeLabelKey, &old.InstanceTypeNodeLabelKey, &new.InstanceTypeNodeLabelKey) - checkNonReloadableBool(CMSvcAllowSimilarAppIdsByDifferentUsers, &old.AllowSimilarAppIdsByDifferentUsers, &new.AllowSimilarAppIdsByDifferentUsers) + checkNonReloadableBool(CMSvcSingleUserPerApplication, &old.SingleUserPerApplication, &new.SingleUserPerApplication) } const warningNonReloadable = "ignoring non-reloadable configuration change (restart required to update)" @@ -299,10 +299,10 @@ func GetSchedulerNamespace() string { return DefaultNamespace } -func (conf *SchedulerConf) GetAllowSimilarAppIdsByDifferentUsers() bool { +func (conf *SchedulerConf) GetSingleUserPerApplication() bool { conf.RLock() defer conf.RUnlock() - return conf.AllowSimilarAppIdsByDifferentUsers + return conf.SingleUserPerApplication } func createConfigs() { @@ -324,26 +324,26 @@ func GetDefaultKubeConfigPath() string { // CreateDefaultConfig creates and returns a configuration representing all default values func CreateDefaultConfig() *SchedulerConf { return &SchedulerConf{ - SchedulerName: constants.SchedulerName, - Namespace: GetSchedulerNamespace(), - ClusterID: DefaultClusterID, - ClusterVersion: buildVersion, - PolicyGroup: DefaultPolicyGroup, - Interval: DefaultSchedulingInterval, - KubeConfig: GetDefaultKubeConfigPath(), - VolumeBindTimeout: DefaultVolumeBindTimeout, - TestMode: false, - EventChannelCapacity: DefaultEventChannelCapacity, - DispatchTimeout: DefaultDispatchTimeout, - KubeQPS: DefaultKubeQPS, - KubeBurst: DefaultKubeBurst, - OperatorPlugins: DefaultOperatorPlugins, - EnableConfigHotRefresh: DefaultEnableConfigHotRefresh, - DisableGangScheduling: DefaultDisableGangScheduling, - UserLabelKey: constants.DefaultUserLabel, - PlaceHolderImage: constants.PlaceholderContainerImage, - InstanceTypeNodeLabelKey: constants.DefaultNodeInstanceTypeNodeLabelKey, - AllowSimilarAppIdsByDifferentUsers: DefaultAllowSimilarAppIdsByDifferentUsers, + SchedulerName: constants.SchedulerName, + Namespace: GetSchedulerNamespace(), + ClusterID: DefaultClusterID, + ClusterVersion: buildVersion, + PolicyGroup: DefaultPolicyGroup, + Interval: DefaultSchedulingInterval, + KubeConfig: GetDefaultKubeConfigPath(), + VolumeBindTimeout: DefaultVolumeBindTimeout, + TestMode: false, + EventChannelCapacity: DefaultEventChannelCapacity, + DispatchTimeout: DefaultDispatchTimeout, + KubeQPS: DefaultKubeQPS, + KubeBurst: DefaultKubeBurst, + OperatorPlugins: DefaultOperatorPlugins, + EnableConfigHotRefresh: DefaultEnableConfigHotRefresh, + DisableGangScheduling: DefaultDisableGangScheduling, + UserLabelKey: constants.DefaultUserLabel, + PlaceHolderImage: constants.PlaceholderContainerImage, + InstanceTypeNodeLabelKey: constants.DefaultNodeInstanceTypeNodeLabelKey, + SingleUserPerApplication: DefaultSingleUserPerApplication, } } @@ -369,7 +369,7 @@ func parseConfig(config map[string]string, prev *SchedulerConf) (*SchedulerConf, parser.boolVar(&conf.EnableConfigHotRefresh, CMSvcEnableConfigHotRefresh) parser.stringVar(&conf.PlaceHolderImage, CMSvcPlaceholderImage) parser.stringVar(&conf.InstanceTypeNodeLabelKey, CMSvcNodeInstanceTypeNodeLabelKey) - parser.boolVar(&conf.AllowSimilarAppIdsByDifferentUsers, CMSvcAllowSimilarAppIdsByDifferentUsers) + parser.boolVar(&conf.SingleUserPerApplication, CMSvcSingleUserPerApplication) // kubernetes parser.intVar(&conf.KubeQPS, CMKubeQPS) diff --git a/pkg/conf/schedulerconf_test.go b/pkg/conf/schedulerconf_test.go index 565924c77..2b92bef80 100644 --- a/pkg/conf/schedulerconf_test.go +++ b/pkg/conf/schedulerconf_test.go @@ -70,7 +70,7 @@ func assertDefaults(t *testing.T, conf *SchedulerConf) { assert.Equal(t, conf.KubeQPS, DefaultKubeQPS) assert.Equal(t, conf.KubeBurst, DefaultKubeBurst) assert.Equal(t, conf.UserLabelKey, constants.DefaultUserLabel) - assert.Equal(t, conf.AllowSimilarAppIdsByDifferentUsers, DefaultAllowSimilarAppIdsByDifferentUsers) + assert.Equal(t, conf.SingleUserPerApplication, DefaultSingleUserPerApplication) } func TestParseConfigMap(t *testing.T) { @@ -92,7 +92,7 @@ func TestParseConfigMap(t *testing.T) { {CMSvcNodeInstanceTypeNodeLabelKey, "InstanceTypeNodeLabelKey", "node.kubernetes.io/instance-type"}, {CMKubeQPS, "KubeQPS", 2345}, {CMKubeBurst, "KubeBurst", 3456}, - {CMSvcAllowSimilarAppIdsByDifferentUsers, "AllowSimilarAppIdsByDifferentUsers", true}, + {CMSvcSingleUserPerApplication, "SingleUserPerApplication", true}, } for _, tc := range testCases { @@ -125,7 +125,7 @@ func TestUpdateConfigMapNonReloadable(t *testing.T) { {CMSvcNodeInstanceTypeNodeLabelKey, "InstanceTypeNodeLabelKey", "node.kubernetes.io/instance-type", false}, {CMKubeQPS, "KubeQPS", 2345, false}, {CMKubeBurst, "KubeBurst", 3456, false}, - {CMSvcAllowSimilarAppIdsByDifferentUsers, "AllowSimilarAppIdsByDifferentUsers", true, false}, + {CMSvcSingleUserPerApplication, "SingleUserPerApplication", true, false}, } for _, tc := range testCases {