New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ServiceReference in ExternalAdmissionHookConfiguration can't be used by most consumers #53825

Closed
deads2k opened this Issue Oct 12, 2017 · 11 comments

Comments

@deads2k
Contributor

deads2k commented Oct 12, 2017

The only server in the cluster which is guaranteed to be able to lookup services and endpoints is the kube-apiserver. All other consumers of the ExternalAdmissionHookConfiguration API cannot use the service reference as-is. We should optimize for the normal case and make this a DNS name.

The case in APIServices is different, since those are only terminated by the kube-apiserver

@kubernetes/sig-api-machinery-bugs

@caesarxuchao

This comment has been minimized.

Show comment
Hide comment
@caesarxuchao

caesarxuchao Oct 12, 2017

Member

All other consumers of the ExternalAdmissionHookConfiguration API

Are you referring to the user apiservers, or are there additional consumers?

If it's UAS, is moving ServiceResolver code to the generic apiserver enough to close this issue?

Member

caesarxuchao commented Oct 12, 2017

All other consumers of the ExternalAdmissionHookConfiguration API

Are you referring to the user apiservers, or are there additional consumers?

If it's UAS, is moving ServiceResolver code to the generic apiserver enough to close this issue?

@deads2k

This comment has been minimized.

Show comment
Hide comment
@deads2k

deads2k Oct 13, 2017

Contributor

Are you referring to the user apiservers, or are there additional consumers?

If it's UAS, is moving ServiceResolver code to the generic apiserver enough to close this issue?

It's not even just UAS's. The vast majority of kube-apiserver installations and the other webhook APIs use DNS and IP routing. To my knowledge, only GKE uses the ssh tunnel.

Contributor

deads2k commented Oct 13, 2017

Are you referring to the user apiservers, or are there additional consumers?

If it's UAS, is moving ServiceResolver code to the generic apiserver enough to close this issue?

It's not even just UAS's. The vast majority of kube-apiserver installations and the other webhook APIs use DNS and IP routing. To my knowledge, only GKE uses the ssh tunnel.

@deads2k

This comment has been minimized.

Show comment
Hide comment
@deads2k

deads2k Oct 13, 2017

Contributor

If it's UAS, is moving ServiceResolver code to the generic apiserver enough to close this issue?

Moving the ServiceResolver into the genericapiserver is inappropriate since only the kube-apiserver can actually resolve services.

Contributor

deads2k commented Oct 13, 2017

If it's UAS, is moving ServiceResolver code to the generic apiserver enough to close this issue?

Moving the ServiceResolver into the genericapiserver is inappropriate since only the kube-apiserver can actually resolve services.

@deads2k

This comment has been minimized.

Show comment
Hide comment
@deads2k

deads2k Oct 13, 2017

Contributor

/priority important-soon

Needed for beta since the API hurts all but one deployment of one component.

Contributor

deads2k commented Oct 13, 2017

/priority important-soon

Needed for beta since the API hurts all but one deployment of one component.

@k8s-merge-robot

This comment has been minimized.

Show comment
Hide comment
@k8s-merge-robot

k8s-merge-robot Oct 13, 2017

Contributor

[MILESTONENOTIFIER] Milestone Issue Current

@caesarxuchao @deads2k

Issue Labels
  • sig/api-machinery: Issue will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/bug: Fixes a bug discovered during the current release.
Help
Contributor

k8s-merge-robot commented Oct 13, 2017

[MILESTONENOTIFIER] Milestone Issue Current

@caesarxuchao @deads2k

Issue Labels
  • sig/api-machinery: Issue will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/bug: Fixes a bug discovered during the current release.
Help
@caesarxuchao

This comment has been minimized.

Show comment
Hide comment
@caesarxuchao

caesarxuchao Oct 18, 2017

Member

Moving the ServiceResolver into the genericapiserver is inappropriate since only the kube-apiserver can actually resolve services.

The service resolver uses the services and endpoints API to resolve a service, I don't understand why "only the kube-apiserver can actually resolve services."

Member

caesarxuchao commented Oct 18, 2017

Moving the ServiceResolver into the genericapiserver is inappropriate since only the kube-apiserver can actually resolve services.

The service resolver uses the services and endpoints API to resolve a service, I don't understand why "only the kube-apiserver can actually resolve services."

@deads2k

This comment has been minimized.

Show comment
Hide comment
@deads2k

deads2k Oct 18, 2017

Contributor

The service resolver uses the services and endpoints API to resolve a service, I don't understand why "only the kube-apiserver can actually resolve services."

No other API server will have permission to read random services and endpoints in the cluster. That means the resolution will fail. They have to ignore the resolution attempt, which indicates the API doesn't serve most consumers.

Contributor

deads2k commented Oct 18, 2017

The service resolver uses the services and endpoints API to resolve a service, I don't understand why "only the kube-apiserver can actually resolve services."

No other API server will have permission to read random services and endpoints in the cluster. That means the resolution will fail. They have to ignore the resolution attempt, which indicates the API doesn't serve most consumers.

@caesarxuchao

This comment has been minimized.

Show comment
Hide comment
@caesarxuchao

caesarxuchao Oct 18, 2017

Member

Ok, agree that UAS shouldn't use ServiceResolver.

But I still think the service reference in the current registration API is enough. I think UAS can rely on kube-dns and kube-proxy to properly route "serviceName.namespaceName.svc" DNS name.

Member

caesarxuchao commented Oct 18, 2017

Ok, agree that UAS shouldn't use ServiceResolver.

But I still think the service reference in the current registration API is enough. I think UAS can rely on kube-dns and kube-proxy to properly route "serviceName.namespaceName.svc" DNS name.

@deads2k

This comment has been minimized.

Show comment
Hide comment
@deads2k

deads2k Oct 23, 2017

Contributor

Ok, agree that UAS shouldn't use ServiceResolver.

But I still think the service reference in the current registration API is enough. I think UAS can rely on kube-dns and kube-proxy to properly route "serviceName.namespaceName.svc" DNS name.

You can actually encapsulate that inside of the dialer and kube-apiserver already passes a special dialer. See #54163 (comment)

Contributor

deads2k commented Oct 23, 2017

Ok, agree that UAS shouldn't use ServiceResolver.

But I still think the service reference in the current registration API is enough. I think UAS can rely on kube-dns and kube-proxy to properly route "serviceName.namespaceName.svc" DNS name.

You can actually encapsulate that inside of the dialer and kube-apiserver already passes a special dialer. See #54163 (comment)

@dims

This comment has been minimized.

Show comment
Hide comment
@dims

dims Nov 15, 2017

Member

@deads2k what do we need in this for 1.9? (Can we move this to 1.10)?

Member

dims commented Nov 15, 2017

@deads2k what do we need in this for 1.9? (Can we move this to 1.10)?

@deads2k

This comment has been minimized.

Show comment
Hide comment
@deads2k

deads2k Nov 16, 2017

Contributor

@deads2k what do we need in this for 1.9? (Can we move this to 1.10)?

Already addressed.

Contributor

deads2k commented Nov 16, 2017

@deads2k what do we need in this for 1.9? (Can we move this to 1.10)?

Already addressed.

@deads2k deads2k closed this Nov 16, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment