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.

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

API Aggregation #263

Closed
lavalamp opened this issue Apr 24, 2017 · 37 comments
Closed

API Aggregation #263

lavalamp opened this issue Apr 24, 2017 · 37 comments
Assignees
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status
Milestone

Comments

@lavalamp
Copy link
Member

lavalamp commented Apr 24, 2017

Feature Description

@liggitt liggitt added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Apr 25, 2017
@deads2k deads2k added this to the v1.7 milestone May 2, 2017
@idvoretskyi idvoretskyi added the stage/beta Denotes an issue tracking an enhancement targeted for Beta status label May 3, 2017
@idvoretskyi idvoretskyi added this to In Progress in Kubernetes 1.7 features May 3, 2017
@bklau
Copy link

bklau commented May 16, 2017

Is this feature going to be Beta in 1.7 release?

@lavalamp
Copy link
Member Author

That is the goal

@philips
Copy link
Contributor

philips commented Jun 15, 2017

How did this feature go directly to beta? It is a huge impacting feature.

cc @brendandburns

@smarterclayton
Copy link
Contributor

@smarterclayton
Copy link
Contributor

@philips
Copy link
Contributor

philips commented Jun 15, 2017

@smarterclayton that was when aggregator was a separate binary. I understand this feature is the built-in aggregator/proxy to kube-apiserver.

@smarterclayton
Copy link
Contributor

So alpha / beta is typically related to stability of features and API. I guess you're saying the checked in code should be considered alpha even though the API is beta? Seems weird that the SIG wouldn't decide whether the API is stable or not (especially having evolved it), and the relative health of the feature.

@philips
Copy link
Contributor

philips commented Jun 15, 2017

What I am trying to understand is that this feature seems net-new in 1.7 and I haven't seen features jump directly to beta in the first release they are introduced. I could be wrong two ways:

  1. This isn't a net new feature in this release
  2. This is a new feature but other features have jumped directly to beta in the past

I don't have a strong opinion on this but it was flagged during the community hangout as a "feature going directly to beta".

Also, I would say that having the API server proxy other APIs is a new API which should go through the usual alpha/beta/stable. But, again, I am just trying to understand the thinking and where the discussion happened.

@jbeda
Copy link
Contributor

jbeda commented Jun 15, 2017

Regardless of process, I assume that this will have impact on how folks deploy and manage k8s. We haven't seen any discussion of this in SIG-cluster-lifecycle and I suspect that it hasn't shown up in SIG-cluster-ops either. Features probably shouldn't be moved to beta until there is a way for a wider set of users to get access.

@smarterclayton
Copy link
Contributor

I don't see it in the 1.6 feature tracking spreadsheet, but the proposal was merged prior to 1.6 and it looks like the functionality was considered alpha by the sig and part of 1.6 (correct me if I'm wrong). I remember the discussions were about whether it should be in process or out, and the resolution prior to 1.6 was in process. Looks like this was also broadly communicated in the kubernetes-dev mailing list in 1.6 as well as in community meetings.

@smarterclayton
Copy link
Contributor

Looks like this was the PR merging the proposal kubernetes/community#261, the discussion in sig-apps https://groups.google.com/forum/#!msg/kubernetes-sig-apps/0gbmMNvZWUo/fgYSHNoWCQAJ

Raises the interesting question about alpha features with beta apis, for sure.

@smarterclayton
Copy link
Contributor

I thought CRD was the feature going directly to beta since it's TPR changing

@brendandburns
Copy link
Contributor

Having this built into the apiserver and on-by default for the first time in 1.7 (if that is the case) is concerning to me.

I have less concerns about the API itself and many more about the deploy/operator/stability issues.

--brendan

@lavalamp
Copy link
Member Author

Look at the date on the OP here; this has been plan-of-record for even longer than that. :)

The standalone was our alpha.

This has been in head for a while now and the tests seem fine, so I'm not super worried.

@idvoretskyi
Copy link
Member

@lavalamp please, provide us with the design proposal link and docs PR link (and update the features tracking spreadsheet with it).
cc @kubernetes/sig-api-machinery-feature-requests

@philips
Copy link
Contributor

philips commented Jun 19, 2017

The plan of record sounds like the api-aggregator binary IMHO.

@lavalamp Where are the docs on non-standalone mode?

@lavalamp
Copy link
Member Author

@philips Weren't you one of the people arguing it ought to be built in? e.g. look at your comment here: https://docs.google.com/document/d/1lU1SnVtEec2iIfYx5U3L5N0za2YhfEOik0uPen276Ks/edit#heading=h.6ae9qgk4mhne

@philips
Copy link
Contributor

philips commented Jun 19, 2017

@lavalamp I 100% think this is the right architectural thing.

But, turning it on by default on the very first release to have the functionality doesn't seem in-line with the way we have approached these things on other API server features. e.g. --experimental-bootstrap-token-auth

@philips
Copy link
Contributor

philips commented Jun 21, 2017

Chatted with @smarterclayton and @lavalamp on this. Essentially I want to ensure that when components start to behave differently in the system, particularly on the network, that there is solid documentation on how to enable/disable those behaviors.

To set a good precedent the docs I think we need here are:

  • The RBAC rules required to turn this off. Idea: As a feature flag default RBAC roles default aggr off for all roles in 1.7.
  • A doc, which @lavalamp said should already exist, on the network behavior once turned on

I think having the standalone kube-aggr being the alpha is fine. Probably would be good if on this repo when something moves from alpha to beta a comment is left so there is a trail to follow

WDYT @brendandburns? @jbeda?

@lavalamp
Copy link
Member Author

Or you could turn the entire APIService api off with --runtime-config (if that is plumbed correctly-- @deads2k?).

It sounds like you've got a specific requirement to know about all network traffic changes, I think staying on top of such things is going to be an uphill battle. I would not have guessed that that was the sticking point! It may be worth working on a central location for discussing architectural network traffic changes, maybe sig architecture. But I don't think we've said anywhere that we need to pay special attention to communicating such changes in particular.

Control plane components already sometimes made calls to nodes. If a user api is registered, the master will now proxy just that api's traffic to the service, which is probably located in a pod on some node. See: https://docs.google.com/document/d/1KNT4iS_Y2miLARrfSPumBIiFo_h7eb5B2pVOZJ0ZmjQ/edit

@brendandburns
Copy link
Contributor

brendandburns commented Jun 22, 2017 via email

@philips
Copy link
Contributor

philips commented Jun 22, 2017

@lavalamp I can't tell from that doc what exists today vs future proposals. What are the next steps to getting this doc mergeable into the docs. Can I help?

Some of the comments from deads makes it seem like some of it doesn't exist. Also, a number of the comments are unaddressed.

@jbeda
Copy link
Contributor

jbeda commented Jun 23, 2017

I'll trust @philips and @brendandburns to make the call. I think they are capturing my concerns.

@lavalamp
Copy link
Member Author

lavalamp commented Sep 1, 2017

I think this issue can be closed at this point. Only one issue turned up at launch, and it affected few people. I'll consider that a success. :)

@lavalamp lavalamp closed this as completed Sep 1, 2017
@luxas
Copy link
Member

luxas commented Sep 2, 2017

Going to GA in v1.8?

@lavalamp
Copy link
Member Author

lavalamp commented Sep 2, 2017 via email

@luxas luxas reopened this Sep 2, 2017
@luxas luxas modified the milestones: next-milestone, 1.7 Sep 2, 2017
@luxas luxas added stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status and removed stage/beta Denotes an issue tracking an enhancement targeted for Beta status labels Sep 2, 2017
@luxas
Copy link
Member

luxas commented Sep 2, 2017

Reopened, added the right labels and retargeted for v1.9
Also edited the first comment to reflect the v1.9 -> GA target

@smarterclayton
Copy link
Contributor

For 1.9, server side table printing to alpha with client side support so that aggregators can expose CLI output
For 1.10, server side table printing to beta and enabled in kubectl by default.

@deads2k
Copy link
Contributor

deads2k commented Jan 17, 2018

Reopened, added the right labels and retargeted for v1.9
Also edited the first comment to reflect the v1.9 -> GA target

Aggregated API didn't move to GA in 1.9, but I'm planning to update it for 1.10.

@idvoretskyi
Copy link
Member

@deads2k updated milestone to 1.10

@idvoretskyi idvoretskyi modified the milestones: next-milestone, v1.10 Jan 22, 2018
k8s-github-robot pushed a commit to kubernetes/kubernetes that referenced this issue Jan 24, 2018
Automatic merge from submit-queue (batch tested with PRs 54071, 58393). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

promote aggregation API to v1

Finishing kubernetes/enhancements#263 as discussed in apimachinery

The API has been available since 1.6 and beta since 1.7.  Openshift has been using it for about a year and service catalog (@pmorie) and metrics server (@piosz @DirectXMan12) have both been using too.  The feature and the API have both been stable over that time.

@kubernetes/sig-api-machinery-api-reviews @kubernetes/api-approvers 

/assign lavalamp
/assign smarterclayton


```release-note
Promoting the apiregistration.k8s.io (aggregation) to GA
```
k8s-publishing-bot added a commit to kubernetes/kube-aggregator that referenced this issue Jan 24, 2018
Automatic merge from submit-queue (batch tested with PRs 54071, 58393). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

promote aggregation API to v1

Finishing kubernetes/enhancements#263 as discussed in apimachinery

The API has been available since 1.6 and beta since 1.7.  Openshift has been using it for about a year and service catalog (@pmorie) and metrics server (@piosz @DirectXMan12) have both been using too.  The feature and the API have both been stable over that time.

@kubernetes/sig-api-machinery-api-reviews @kubernetes/api-approvers

/assign lavalamp
/assign smarterclayton

```release-note
Promoting the apiregistration.k8s.io (aggregation) to GA
```

Kubernetes-commit: 35ed5338b104f41c91efb2b842e92fded6d0b509
@idvoretskyi idvoretskyi added kind/feature Categorizes issue or PR as related to a new feature. tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team labels Jan 29, 2018
@Bradamant3
Copy link
Contributor

@deads2k 1.10 feature tracking spreadsheet indicates docs need updating. I don't see anything obvious in the original docs PR for 1.7, but could you please check? If not needed, please indicate on spreadsheet. Any questions, lmk.

@deads2k
Copy link
Contributor

deads2k commented Mar 2, 2018

@deads2k 1.10 feature tracking spreadsheet indicates docs need updating. I don't see anything obvious in the original docs PR for 1.7, but could you please check? If not needed, please indicate on spreadsheet. Any questions, lmk.

Updated the spreadsheet. No new docs, it was a promotion without updates.

@justaugustus
Copy link
Member

@lavalamp @luxas @deads2k
Any plans for this in 1.11?

If so, can you please ensure the feature is up-to-date with the appropriate:

  • Description
  • Milestone
  • Assignee(s)
  • Labels:
    • stage/{alpha,beta,stable}
    • sig/*
    • kind/feature

cc @idvoretskyi

@deads2k
Copy link
Contributor

deads2k commented Apr 17, 2018

@lavalamp @luxas @deads2k
Any plans for this in 1.11?

No plans in 1.11. It remains stable.

@lavalamp
Copy link
Member Author

Isn't this GA in 1.10? I think the next step here would be a v2, but we have no plans for that and it'd deserve its own issue.

@justaugustus
Copy link
Member

Thanks for the update, @deads2k + @lavalamp!

@kacole2 kacole2 removed tracked/yes Denotes an enhancement issue is actively being tracked by the Release Team labels Oct 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. stage/stable Denotes an issue tracking an enhancement targeted for Stable/GA status
Projects
No open projects
Development

No branches or pull requests