-
Notifications
You must be signed in to change notification settings - Fork 385
update apiregistration yaml to v1.7 final #1011
update apiregistration yaml to v1.7 final #1011
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @MHBauer
@cheftako this is an example of the things we need to keep in sync on. |
priority: 100 | ||
{{ else if .Capabilities.APIVersions.Has "apiregistration.k8s.io/v1beta1" -}} | ||
groupPriorityMinimum: 10000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks straightforward, though could we get an explanation as to why these priority values were chosen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kibbles-n-bytes check out https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/kube-aggregator/pkg/apis/apiregistration/v1beta1/types.go#L56-L71 for documentation on generally what these should be. I believe that, in a follow-up, we should parameterize these. Not blocking for this PR.
@MHBauer I have removed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM1
Sorry on jumping the gun there @arschles. |
right now we're broken without the priority existing correctly for 1.7