Let release_1_5 clientset include multiple versions of a group #35471

caesarxuchao · 2016-10-24T23:25:56Z

This PR make versioned clientset to include multiple versions of a group. Currently only batch has v1 and v2alpha1. The clientset interface now looks like:

    BatchV2alpha1() v2alpha1batch.BatchV2alpha1Interface
    BatchV1() v1batch.BatchV1Interface
    // Deprecated: please explicitly pick a version if possible.
    Batch() v1batch.BatchV1Interface

Commit "update client-gen to say internalversion rather than unversioned" fixes #24481.

cc @kubernetes/sig-api-machinery @soltysh @deads2k @nikhiljindal

This change is

release_1_5 clientset supports multiple versions of a group.

k8s-ci-robot · 2016-10-24T23:29:20Z

Jenkins Kubemark GCE e2e failed for commit 6c75e63. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

k8s-ci-robot · 2016-10-24T23:29:50Z

Jenkins GCE e2e failed for commit 6c75e63. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

soltysh

Some comments. Additionally the deprecation part could squashed into the main parts where the codegen is changed.

soltysh · 2016-10-26T15:10:02Z

cmd/libs/go2idl/client-gen/types/helpers_test.go

+
+func TestVersionSort(t *testing.T) {
+	unsortedVersions := []string{"v4beta1", "v2beta1", "v2alpha1", "v3", "v1"}
+	expected := []string{"v2alpha1", "v2beta1", "v4beta1", "v1", "v3"}


The order here doesn't seem to be correct. I'd expect following:

{"v1", "v2alpha1", "v2beta1", "v4beta1", "v3"}

I want to keep the default referring to a stable version. (e.g., .Batch() returns v1, not v2alpha1, which is behavior before this change)

@soltysh do you think it's reasonable?

Discussed on slack - ok.

soltysh · 2016-10-26T15:13:05Z

test/e2e/generated_clientset.go

@@ -187,6 +188,124 @@ var _ = framework.KubeDescribe("Generated release_1_5 clientset", func() {
 	})
 })

+func observeJobCreation(w watch.Interface) {


observeScheduledJobCreation

OT: instead of having multiple observe* why not having single generic function doing this, especially you're using watch.Interface as an argument.

Done. Combined the functions.

mml · 2016-10-26T22:40:01Z

test/e2e/generated_clientset.go

+
+var _ = framework.KubeDescribe("Generated release_1_5 clientset", func() {
+	f := framework.NewDefaultFramework("clientset")
+	It("should create v2alpha1 scheduleJobs, delete scheduleJobs, watch scheduleJobs", func() {


This is a good specific test to watch for regression in the 1.5 clientset. Nice job.

A more general test of this functionality would be good eventually, but seems like a big project.

mml · 2016-10-26T22:40:36Z

LGTM once you address soltysh changes.

caesarxuchao · 2016-10-27T22:38:04Z

Squashed and addressed the comments. Self apply the lgtm. Thank you for the quick review for a monster PR.

k8s-ci-robot · 2016-10-27T23:24:59Z

Jenkins GKE smoke e2e failed for commit 8ba6391a72e0d7ae8fee922bce4ac93cbd76c57b. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

liggitt · 2016-10-27T23:49:48Z

I thought this was intended to drop after code freeze. This seems enormously disruptive for things working toward code freeze.

caesarxuchao · 2016-10-27T23:59:07Z

@liggitt client-go currently passively copy the contents from the main repo, we don't develop in client-go. I need this PR to go in so that during code freeze I can have a client-go that's in the right shape for the migration.

By "disruptive" do you mean it will cause people to rebase? It shouldn't. I keep the original method like "Batch()", which returns the same interface as it used to do.

fejta · 2016-10-29T18:55:12Z

BTW Given the scope of this PR I recommend you consider bumping up the priority.

caesarxuchao · 2016-10-29T19:13:24Z

I'll add the milestone. The PR shouldn't cause many conflicts. Most changes are import lines.

caesarxuchao · 2016-10-29T19:48:35Z

@k8s-bot node e2e test this

caesarxuchao · 2016-10-29T20:25:21Z

@k8s-bot node e2e test this, issue #34990

update client-gen to use the term "internalversion" rather than "unversioned"; leave internal one unqualified; cleanup client-gen

caesarxuchao · 2016-10-29T20:31:48Z

Got conflict in test_owners.csv again. This is too much. I'm bumping the priority.

k8s-github-robot · 2016-10-29T21:50:25Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-ci-robot · 2016-10-29T22:13:11Z

Jenkins GCE Node e2e failed for commit 850729b. Full PR test history.

The magic incantation to run this job again is @k8s-bot node e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

caesarxuchao · 2016-10-29T22:17:22Z

@k8s-bot node e2e test this

k8s-github-robot · 2016-10-29T22:40:13Z

Automatic merge from submit-queue

caesarxuchao · 2016-10-29T23:42:01Z

ScheduleJob is not enabled on gke. So gke tests are failing now. I'll send a fix soon.

caesarxuchao · 2016-10-29T23:59:29Z

Sent #35855.

googlebot added the cla: yes label Oct 24, 2016

k8s-github-robot assigned mml Oct 24, 2016

k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note-label-needed labels Oct 24, 2016

caesarxuchao force-pushed the client-gen-multi-versions branch 3 times, most recently from dfc1c53 to 7cbd2c5 Compare October 25, 2016 05:14

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 25, 2016

soltysh requested changes Oct 26, 2016

View reviewed changes

soltysh mentioned this pull request Oct 26, 2016

Add tooling to generate listers #35639

Merged

mml reviewed Oct 26, 2016

View reviewed changes

caesarxuchao force-pushed the client-gen-multi-versions branch 2 times, most recently from 0f68c3c to 8ba6391 Compare October 27, 2016 22:36

caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2016

k8s-github-robot added do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 27, 2016

caesarxuchao added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. labels Oct 27, 2016

caesarxuchao force-pushed the client-gen-multi-versions branch from 8ba6391 to 0e15b0e Compare October 27, 2016 23:39

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2016

caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 27, 2016

caesarxuchao added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 29, 2016

k8s-github-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Oct 29, 2016

liggitt mentioned this pull request Oct 29, 2016

test_owners.csv causes repo-wide merge conflicts #35850

Closed

caesarxuchao force-pushed the client-gen-multi-versions branch from 9762c5d to 6c691e9 Compare October 29, 2016 19:11

caesarxuchao added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 29, 2016

caesarxuchao added this to the v1.5 milestone Oct 29, 2016

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2016

include multiple versions in clientset

850729b

update client-gen to use the term "internalversion" rather than "unversioned"; leave internal one unqualified; cleanup client-gen

caesarxuchao force-pushed the client-gen-multi-versions branch from 6c691e9 to 850729b Compare October 29, 2016 20:31

caesarxuchao added lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Oct 29, 2016

k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 29, 2016

k8s-github-robot merged commit 7f309f5 into kubernetes:master Oct 29, 2016

chentao1596 mentioned this pull request Dec 5, 2016

WIP:kubelet: support multi-headers when getting pod from HTTP source #38089

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Let release_1_5 clientset include multiple versions of a group #35471

Let release_1_5 clientset include multiple versions of a group #35471

caesarxuchao commented Oct 24, 2016 •

edited

Loading

k8s-ci-robot commented Oct 24, 2016

k8s-ci-robot commented Oct 24, 2016

soltysh left a comment

soltysh Oct 26, 2016

caesarxuchao Oct 26, 2016 •

edited

Loading

caesarxuchao Oct 27, 2016

soltysh Oct 27, 2016

soltysh Oct 26, 2016

soltysh Oct 26, 2016

caesarxuchao Oct 26, 2016

caesarxuchao Oct 27, 2016

mml Oct 26, 2016

mml commented Oct 26, 2016

caesarxuchao commented Oct 27, 2016

k8s-ci-robot commented Oct 27, 2016

liggitt commented Oct 27, 2016

caesarxuchao commented Oct 27, 2016

fejta commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

k8s-github-robot commented Oct 29, 2016

k8s-ci-robot commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

k8s-github-robot commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

Let release_1_5 clientset include multiple versions of a group #35471

Let release_1_5 clientset include multiple versions of a group #35471

Conversation

caesarxuchao commented Oct 24, 2016 • edited Loading

k8s-ci-robot commented Oct 24, 2016

k8s-ci-robot commented Oct 24, 2016

soltysh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caesarxuchao Oct 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mml commented Oct 26, 2016

caesarxuchao commented Oct 27, 2016

k8s-ci-robot commented Oct 27, 2016

liggitt commented Oct 27, 2016

caesarxuchao commented Oct 27, 2016

fejta commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

k8s-github-robot commented Oct 29, 2016

k8s-ci-robot commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

k8s-github-robot commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 29, 2016

caesarxuchao commented Oct 24, 2016 •

edited

Loading

caesarxuchao Oct 26, 2016 •

edited

Loading