add controller to autoregister APIServices #42732
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is NOT APPROVED The following people have approved this PR: deads2k Needs approval from an approver in each of these OWNERS Files: We suggest the following people: |
k8s-github-robot
added
size/XL
|
This change is |
deads2k
force-pushed
the
agg-03-tpr-controller
branch
from
1cc35df to
824a826
Compare
|
ncdc
added
the
sig/api-machinery
ncdc
reviewed
Mar 9, 2017
| // AutoAPIServiceRegistration is an interface which callers can re-declare locally and properly cast to for | ||
| // adding and removing APIServices | ||
| type AutoAPIServiceRegistration interface { | ||
| AddAPIServiceToSync(in *apiregistration.APIService) |
ncdc
reviewed
Mar 9, 2017
| } | ||
|
|
||
| // AutoRegisterController is used to keep a particular set of APIServices present in the API. It is useful | ||
| // for cases where you want to auto-register APIs like TPRs or core API server |
There was a problem hiding this comment.
"or core API server" ... ?
clarifying. "or groups from the core kube-apiserver"
ncdc
reviewed
Mar 9, 2017
| apiServiceSynced cache.InformerSynced | ||
| apiServiceClient apiregistrationclient.APIServicesGetter | ||
|
|
||
| apiServiceToSyncLock sync.RWMutex |
There was a problem hiding this comment.
Pluralize services? apiServicesToSyncLock
ncdc
reviewed
Mar 9, 2017
| queue workqueue.RateLimitingInterface | ||
| } | ||
|
|
||
| func NewAutoRegisterAutoRegisterController(apiServiceInformer informers.APIServiceInformer, apiServiceClient apiregistrationclient.APIServicesGetter) *AutoRegisterController { |
There was a problem hiding this comment.
Did you mean to have AutoRegister duplicated in the function name?
ncdc
reviewed
Mar 9, 2017
|
|
||
| // AutoRegisterController is used to keep a particular set of APIServices present in the API. It is useful | ||
| // for cases where you want to auto-register APIs like TPRs or core API server | ||
| type AutoRegisterController struct { |
There was a problem hiding this comment.
Do you need this type exported?
I can make it private to start. I suspect that most callers will use the interface and call the run method.
deads2k
force-pushed
the
agg-03-tpr-controller
branch
from
824a826 to
8a3b57b
Compare
|
comments so far addressed. |
ncdc
reviewed
Mar 9, 2017
| } | ||
| cast, ok = tombstone.Obj.(*apiregistration.APIService) | ||
| if !ok { | ||
| glog.V(2).Infof("Tombstone contained object that is not a CSR: %#v", obj) |
ncdc
reviewed
Mar 9, 2017
| case desired == nil: | ||
| return c.apiServiceClient.APIServices().Delete(curr.Name, nil) | ||
|
|
||
| // if the spec's already match, nothing for us to do |
ncdc
reviewed
Mar 9, 2017
| return c.apiServiceClient.APIServices().Delete(curr.Name, nil) | ||
|
|
||
| // if the spec's already match, nothing for us to do | ||
| case reflect.DeepEqual(curr.Spec, desired.Spec): |
There was a problem hiding this comment.
is reflect ok or do we need semantic?
There was a problem hiding this comment.
is reflect ok or do we need semantic?
Semantic isn't cleanly divorced from a global scheme yet. Reflect works for now.
There was a problem hiding this comment.
is reflect ok or do we need semantic?
Semantic isn't cleanly divorced from a global scheme yet. Reflect works for now.
|
@k8s-bot gce etcd3 e2e test this |
|
/release-note-none |
k8s-ci-robot
added
release-note-none
|
@ncdc did you make it through? |
deads2k
force-pushed
the
agg-03-tpr-controller
branch
from
8a3b57b to
c7318e0
Compare
k8s-github-robot
added
the
needs-rebase
ncdc
reviewed
Mar 13, 2017
| func(c AutoAPIServiceRegistration, fakeWatch *watch.FakeWatcher, client internalclientset.Interface) { | ||
| c.AddAPIServiceToSync(&apiregistration.APIService{ObjectMeta: metav1.ObjectMeta{Name: "foo"}}) | ||
| }, | ||
| // adding an API service which claims to be managed but isn't should be deleted |
deads2k
force-pushed
the
agg-03-tpr-controller
branch
from
c7318e0 to
55adc4d
Compare
deads2k
added
the
approved
deads2k
added
the
lgtm
k8s-github-robot
removed
the
needs-rebase
|
LGTM |
|
@k8s-bot gce etcd3 e2e test this |
This was referenced Mar 15, 2017
|
@deads2k PR needs rebase |
k8s-github-robot
added
the
needs-rebase
fejta
removed
the
needs-rebase
k8s-github-robot
pushed a commit
that referenced
this pull request
Mar 26, 2017
Automatic merge from submit-queue (batch tested with PRs 43429, 43416, 43312, 43141, 43421) Create controller to auto register TPRs with the aggregator Builds on #42732 (already lgtmed) Creates a simple controller to wire TPRs with the API Service autoregistration controller. @kubernetes/sig-api-machinery-misc @ncdc
deads2k
force-pushed
the
agg-03-tpr-controller
branch
from
55adc4d to
e38c575
Compare
deads2k
removed
the
lgtm
k8s-github-robot
pushed a commit
that referenced
this pull request
Mar 27, 2017
Automatic merge from submit-queue (batch tested with PRs 43694, 41262, 42911) combine kube-apiserver and kube-aggregator This combines several pulls currently in progress and wires them together. The aggregator sits in front of the normal kube-apiserver and allows local fallthrough instead of proxying. @kubernetes/sig-api-machinery-misc @DirectXMan12 since you seem invested, your life will get easier @luxas FYI since you've started trying to wire something together. Dependent Pulls LGTM: - [x] #42801 - [x] #42886 - [x] #42900 - [x] #42732 - [x] #42672 - [x] #43141 - [x] #43076 - [x] #43149 - [x] #43226 - [x] #43144
12 tasks
hoegaarden
added a commit
to hoegaarden/frameworks
that referenced
this pull request
Mar 19, 2018
We found that the APIServer was not fully up when it logged "Serving (in)securely on ...". We tried to find a different log line we can wait for to know when the APIServer is fully up. The autoregister controller came in with kubernetes/kubernetes#42732. It seems to be the last thing to come up before the server is ready to go.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As we look to combine the aggregator and the core kube-apiserver which includes things like TPRs, we need a way to reliably auto-manage APIServices for the aggregator so that discovery works as expected and so that users don't have to maintain particular resources that they shouldn't have to.
This controller deals with a dynamic set of APIServices which are synchronized. The mutation of that set is exposed via threadsafe methods. If a user wants to take control, he can remove our label and do whatever he wants.
The controller approach provides the functionality we need while preserving the independence of various layers.
@kubernetes/sig-api-machinery-pr-reviews @ncdc