Add a TODO to update this with the name of the admission controller once you write it.
BTW do you think you can do the admission controller for 1.7? It shouldn't be too hard if you mostly copy one of the existing admission controllers. I don't think any of the existing admission controllers are configured by a ConfigMap, though.

Let me call it "Priority" admission controller for now. If we decided that it is not a good name, I will change it.
And, yes, I would like to do admission controller for 1.7 as well if we reach a consensus on how priority should be defined and work.

I don't think the admission controller name should be in the API - that has nothing to do with the end user's perception of the field. A custom admission controller can handle this as well as anything else.

Same thing here, update with actual name of the admission controller once you write it.

A couple of comments about validation

(1) You should also do validation on Priority (not just PriorityName) even though it is set by an admission controller (user might write a buggy custom admission controller). BTW do we want to allow negative priorities?

(2) I think you need to modify ValidatePodUpdate() to make it possible to update PriorityName (and Priority)

1. Actually any valid integer is a valid priority, even negative numbers. I am not sure if we can do any particular validation for "Priority".

2. I was not so sure whether we want to allow updating priority or not. I am going to modify ValidatePodUpdate() so that it allows change of priority with the assumption that all updates go through admission checks.

Please don't allow priorityName to be updated unless you are absolutely sure you are going to use it in 1.7. If you aren't, leave it create only. We can never tighten validation.

zero is the lowest?

No, priority could potentially be negative. Zero is default.

Will admission controller set it to lowest priority? The concern is that: if not specified, there maybe an undefined priority in system, e.g. zero is not a defined priority.

I updated the proposal a few minutes ago and added more details about the default name resolution. We probably go with resolving unspecified PriorityClassName to zero in the initial version, but in a follow up change, we will add the support to annotate one of the PriorityClasses with a "default" annotation. Once such a priority class exists, the system resolves unspecified PriorityClassName to the value of the default class. If there is no such a class, we will still resolve to zero. Zero is not the lowest possible priority in the system given that any valid integer is a valid priority.

We use CamelCase for all constants, if this is a "opaque constant field". If this is a reference to a priority class API object, the field name should be PriorityClassName.

@bsalamat , "default" annotation makes sense to me :).

@smarterclayton This is a reference to a priority class object. The field is already named PriorityClassName.

@k82cn that's the plan.

s/will will/will

Done. Thanks!

will be default or zero if there is no default.

Are we going to support default now? I found that in the design document, it is will, just want to confirm if the default will be available in the first implementation? If not, how about add some words such as will to identify it is future work?

The plan is to have the default in the first version. We will definitely have the ability to define a "global" default priority. In the future versions, we may add per namespace default as well.

Got it, then how about update the sentence default or zero if there is no default a bit?

Such as when using default one and when setting to zero? Using default or zero if there is no default may make people confused, as they may not clear when using default and when using zero?

different comments with api/types.go, should be by creating a PriorityClass.

personally, I'd like the following if/else:

if utilfeature.DefaultFeatureGate.Enabled(features.PodPriority) {
  ...
} else {
  ...
}

in my version of the code, the feature gate is not checked when priority class is not specified. Also, the condition "if len(spec.PriorityClassName) > 0" should be repeated in both if and else blocks of your version.

yes; my version is for sometimes removing this feature gate (just remove the else block is OK); both are ok to me :).