Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@manirajv06
manirajv06 committed Jul 10, 2023
1 parent 909fa7b commit 8b03b29
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 93 deletions.
2 changes: 1 addition & 1 deletion pkg/appmgmt/general/podevent_handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down
2 changes: 1 addition & 1 deletion pkg/appmgmt/general/podevent_handler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
176 changes: 88 additions & 88 deletions pkg/conf/schedulerconf.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
}

Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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)"
Expand Down Expand Up @@ -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

Check warning on line 305 in pkg/conf/schedulerconf.go

View check run for this annotation

Codecov / codecov/patch

pkg/conf/schedulerconf.go#L302-L305 

Added lines #L302 - L305 were not covered by tests
}

func createConfigs() {
Expand All @@ -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,
}
}

Expand All @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions pkg/conf/schedulerconf_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 8b03b29

Please sign in to comment.