opened
derekwaynecarr
username_0
I agree.

cc @username_1 @kubernetes/rh-cluster-infra 
cc @kubernetes/goog-control-plane
Remove experimentalMode from namespace_controller
issue
created
deads2k
username_1
@username_0 Yes please.
comment
created
mikedanese
username_2
@username_3 any recommendation on how to do this?
comment
created
caesarxuchao
username_3
This [line](https://github.com/kubernetes/kubernetes/blob/master/cmd/kube-controller-manager/app/controllermanager.go#L277) seems wrong: 
```
experimentalMode := s.EnableHorizontalPodAutoscaler || s.EnableDeploymentController
```
It should deduce if experimental group is registered by checking if latest.Group("experimental") returns an error.

I think we should remove experimentalMode, keep s.EnableHorizontalPodAutoscaler and s.EnableDeploymentController, and checks these two flags and latest.Group("experimental") before execute the code related to these two controllers.
comment
created
caesarxuchao
username_3
@nikhiljindal @username_4
comment
created
deads2k
username_1
Checking if the group is registered is not the same as knowing whether the resources are being served.  It's possible to have the schemes and codecs present, but not serve the resources to be cleaned up.
comment
created
caesarxuchao
username_3
By "being served", do you mean if there are endpoints exposed on API server? If so, the controller manager can use the API discovery mechanism introduced in #13955, i.e., it needs to check if these two controllers exist in the APIReourceList returned at /apis/experimental/version
comment
created
deads2k
username_1
I think that would be more appropriate.  Find the exposed, namespaced resources and delete them.  That fulfills the intent.
comment
created
lavalamp
username_4
namespace controller runs in controller manager, not apiserver, right? (please say yes)

It should deduce whether experimental is on by querying apiserver, not by what is compiled into it.

Future: In fact it should not use the high level client at all. It should use raw RESTClients (like kubectl builder). And it should discover the entire list of namespaced resources by querying apiserver, which @username_3 recently made possible.

(disclaimer: I've never looked at the component, so I don't know how much of that it does already)
comment
created
derekwaynecarr
username_0
@username_4 - its in controller manager.
comment
created
derekwaynecarr
username_0
@username_3 - are there client libraries for dealing with the API discovery mechanism you can direct me towards?
comment
created
lavalamp
username_4
We are still building them :(
comment
created
derekwaynecarr
username_0
@username_4 - is there a PR i can track?
comment
created
lavalamp
username_4
The issue #14321 is the closest thing I can give you at the moment.
comment
created
derekwaynecarr
username_0
The change in https://github.com/kubernetes/kubernetes/pull/15427 will remove the experimental flag and work against the discovery API.  

As a result, I am closing this issue.

I will shift focus to https://github.com/kubernetes/kubernetes/issues/14765 to make it delete all resources based on discovery with no manual need for future developers to write code in namespace_controller to take part in deletion lifecycle.
comment
closed
derekwaynecarr
username_0
issue