Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Proposal for federated API servers #17191

Merged
merged 1 commit into from
Jan 28, 2016
Merged

Proposal for federated API servers #17191

merged 1 commit into from
Jan 28, 2016

Conversation

nikhiljindal
Copy link
Contributor

Proposal for #16558

cc @kubernetes/goog-csi @lavalamp @smarterclayton @erictune @bgrant0607 @brendandburns

@nikhiljindal nikhiljindal mentioned this pull request Nov 13, 2015
Closed
@k8s-github-robot
Copy link

Labelling this PR as size/L

@k8s-github-robot k8s-github-robot added kind/design Categorizes issue or PR as related to design. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 13, 2015
@k8s-bot
Copy link

k8s-bot commented Nov 13, 2015

GCE e2e test build/test passed for commit 16be0afaaa7178c5bb40def6963dadfe2e21da76.

lavalamp
lavalamp reviewed Nov 13, 2015
View reviewed changes
docs/proposals/third-party-api-servers.md

## Caveats

* Resources can have same uid: kubectl users, for example, can get 2 resources
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UUIDs are supposed to be unique in space and time. This should not be true or we are doing something wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're continuing to say "we do not coordinate to ensure uids are unique across resource types" - clients should not assume that a uid has any meaning outside of its resource.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

@nikhiljindal
Copy link
Contributor Author

Reading through the comments, I see 2 high level discussion points:

  • Whats the mapping between API servers and API groups: Is it 1:1, 1:many or many:many? Are we modelling API groups as providers (for ex: kubernetes, openshift) or are we modelling them as a grouping of logically similar resources (for ex: admin, user).
  • How much do we care about the validation of new servers: Do we assume that they are valid and dont do any auto-validation (Maybe provide an endpoint which admins can call manually before registering a server to validate it?). Or do we validate them at the time of registration? Synchronously or ashynchronously? Or keep validating them after regular intervals? I guess the answer also depends on what all do we want to validate?

The current proposal was written assuming

  • An API server can provide resources in multiple API groups and resources in same API group can be provided by multiple API servers.
  • We need validation. This proposed an asynchronous validation after registration.

WDYT?

@liggitt
Copy link
Member

liggitt commented Nov 17, 2015

An API server can provide resources in multiple API groups

I would expect that to be true

Resources in same API group can be provided by multiple API servers.

I would not expect that to be true

@lavalamp
Copy link
Member

What @liggitt said. Really do not want multiple apiservers per group.

As far as validation-- I think only need to validate that the group(s) being registered are unique. If the other apiservers don't actually work, that's not our problem.

@nikhiljindal
Copy link
Contributor Author

ok some notes based on IRL discussion with @lavalamp

  • We do not want multiple apiservers per group. We are building clients that use only one version per group and it wont be possible to enforce the same version of a group spread across multiple apiservers.
  • We want each registered API group to be unique, so instead of designing it as an APIServer registration API, we should design it as an API group registration API. The name of the object stored in etcd will be the group name, ensuring its uniqueness.
  • Validation: We do not need any other explicit validation apart from group name uniqueness. We do want to have a "conformance test" which ensures that api servers follow api-conventions.md (even the core server). Doesnt look like we will have the conformance test in time for 1.2.
  • GroupName: "federation" or "apifederation" to distinguish it from "clusterfederation" that @quinton-hoole might need for Ubernetes.
  • ResourceName: "apigrouplocation" or just "grouplocation" if the groupName has "api" in it.
    cc @bgrant0607 to vet the names.

@nikhiljindal
Copy link
Contributor Author

Added a "Running on hosted kubernetes cluster" section.
Also renamed the doc to "federated-api-servers".

@k8s-bot
Copy link

k8s-bot commented Jan 19, 2016

GCE e2e test build/test passed for commit 87a2adb868a4e03baafd6574909bd91611e0eeba.

roberthbailey
roberthbailey reviewed Jan 20, 2016
View reviewed changes
docs/proposals/federated-api-servers.go
## High Level Architecture

There will be 2 new components in the cluster:
* A smiple program to summarize discovery information from all the servers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/smiple/simple

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@nikhiljindal
Copy link
Contributor Author

Thanks for the review @roberthbailey
Updated the proposal as per comments.
PTAL.

@k8s-bot
Copy link

k8s-bot commented Jan 20, 2016

GCE e2e test build/test passed for commit 4d948ce79d49c6222e41b7da453ce2591a3e307b.

@roberthbailey
Copy link
Contributor

@nikhiljindal - I spoke with @lavalamp and am delegating my lgtm to him.

lavalamp
lavalamp reviewed Jan 26, 2016
View reviewed changes
docs/proposals/federated-api-servers.md
provide a lot more functionality like: rate-limiting, caching, logging,
transformations and authentication.
In future, we can also use ingress. That will give cluster admins the flexibility to
easily swap out the ingress controller by a Go reverse proxy, ngingx, hapraoxy
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: praoxy -> proxy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@lavalamp
Copy link
Member

Just a few nits. Then I think this is close enough that we can merge and iterate from there.

@nikhiljindal
Copy link
Contributor Author

Thanks for the comments @lavalamp.
Pushed a new version with the fixes.

@k8s-bot
Copy link

k8s-bot commented Jan 26, 2016

GCE e2e test build/test passed for commit 094e537.

@smarterclayton
Copy link
Contributor

This is Lgtm

@nikhiljindal nikhiljindal mentioned this pull request Jan 27, 2016
Closed
@lavalamp lavalamp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 27, 2016
@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Jan 28, 2016
Merge pull request #17191 from nikhiljindal/3rdpartyServers
4e27ed6 
Auto commit by PR queue bot
@k8s-github-robot k8s-github-robot merged commit 4e27ed6 into kubernetes:master Jan 28, 2016
@lavalamp
Copy link
Member

\o/

@nikhiljindal nikhiljindal mentioned this pull request Feb 1, 2016
Closed
xingzhou pushed a commit to xingzhou/kubernetes that referenced this pull request Dec 15, 2016
Merge pull request kubernetes#17191 from nikhiljindal/3rdpartyServers
4815285 
Auto commit by PR queue bot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@lavalamp lavalamp

Labels
kind/design Categorizes issue or PR as related to design. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.