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

Comments

@lavalamp
Member

lavalamp commented Apr 24, 2017

Feature Description

@bklau

This comment has been minimized.

bklau commented May 16, 2017

Is this feature going to be Beta in 1.7 release?

@lavalamp

This comment has been minimized.

Member

lavalamp commented May 16, 2017

That is the goal

@philips

This comment has been minimized.

Contributor

philips commented Jun 15, 2017

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

cc @brendandburns

@smarterclayton

This comment has been minimized.

@smarterclayton

This comment has been minimized.

Contributor

smarterclayton commented Jun 15, 2017

@philips

This comment has been minimized.

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

This comment has been minimized.

Contributor

smarterclayton commented Jun 15, 2017

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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Contributor

smarterclayton commented Jun 15, 2017

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

This comment has been minimized.

Contributor

smarterclayton commented Jun 15, 2017

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

This comment has been minimized.

Contributor

smarterclayton commented Jun 15, 2017

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

@brendandburns

This comment has been minimized.

brendandburns commented Jun 15, 2017

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

This comment has been minimized.

Member

lavalamp commented Jun 15, 2017

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

This comment has been minimized.

Member

idvoretskyi commented Jun 16, 2017

@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

This comment has been minimized.

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

This comment has been minimized.

Member

lavalamp commented Jun 19, 2017

@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

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Member

lavalamp commented Jun 22, 2017

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

This comment has been minimized.

brendandburns commented Jun 22, 2017

@philips

This comment has been minimized.

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

This comment has been minimized.

Contributor

jbeda commented Jun 23, 2017

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

@lavalamp

This comment has been minimized.

Member

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 Sep 1, 2017

@luxas

This comment has been minimized.

Member

luxas commented Sep 2, 2017

Going to GA in v1.8?

@lavalamp

This comment has been minimized.

Member

lavalamp commented Sep 2, 2017

@luxas luxas reopened this Sep 2, 2017

@luxas luxas modified the milestones: next-milestone, 1.7 Sep 2, 2017

@luxas luxas added stage/stable and removed stage/beta labels Sep 2, 2017

@luxas

This comment has been minimized.

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

This comment has been minimized.

Contributor

smarterclayton commented Oct 27, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Member

idvoretskyi commented Jan 22, 2018

@deads2k updated milestone to 1.10

@idvoretskyi idvoretskyi modified the milestones: next-milestone, v1.10 Jan 22, 2018

k8s-merge-robot added a commit to kubernetes/kubernetes that referenced this issue Jan 24, 2018

Merge pull request #58393 from deads2k/agg-02-ga
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

Merge pull request #58393 from deads2k/agg-02-ga
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
@Bradamant3

This comment has been minimized.

Member

Bradamant3 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.

@deads2k

This comment has been minimized.

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

This comment has been minimized.

Member

justaugustus commented Apr 17, 2018

@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

This comment has been minimized.

deads2k commented Apr 17, 2018

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

No plans in 1.11. It remains stable.

@lavalamp

This comment has been minimized.

Member

lavalamp commented Apr 17, 2018

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.

@lavalamp lavalamp closed this Apr 17, 2018

@justaugustus

This comment has been minimized.

Member

justaugustus commented Apr 17, 2018

Thanks for the update, @deads2k + @lavalamp!

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