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

Use the first version as thirdparty resource preferredVersion #24006

Merged
merged 1 commit into from
Apr 17, 2016
Merged

Use the first version as thirdparty resource preferredVersion #24006

merged 1 commit into from
Apr 17, 2016

Conversation

caesarxuchao
Copy link
Member

First commit is a one-liner, which implements the server-half of #23985.

The other two commits rearrange the test code, and add back a commented out test of thirdparty resource.

@lavalamp @nikhiljindal

@caesarxuchao caesarxuchao added the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Apr 7, 2016
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Apr 7, 2016
caesarxuchao
caesarxuchao reviewed Apr 7, 2016
View reviewed changes
pkg/master/master_test.go
@@ -370,114 +393,109 @@ func TestAPIVersionOfDiscoveryEndpoints(t *testing.T) {
}

func TestDiscoveryAtAPIS(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I address the TODO (I think it's left silently by etune months ago :).

Github's diff is misleading, the only change is the master is initialized by newLimitedMaster() instead of newMaster().

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this. I didn't realize that this had gotten commented out.

Copy link
Member

Choose a reason for hiding this comment

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

Is there an issue filed about reenabling this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I couldn't find any.

@caesarxuchao caesarxuchao added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Apr 7, 2016
lavalamp
lavalamp reviewed Apr 8, 2016
View reviewed changes
pkg/master/master_test.go

assert.Equal(4, len(groupList.Groups))
// autoscaling group
assert.Equal(expectGroupNames[0], groupList.Groups[0].Name)
Copy link
Member

Choose a reason for hiding this comment

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

Can you add something that figures out the correct index instead of hard-coding it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Now it uses map instead of lists, with group name as map key, so there is no more hard-coding. PTAL. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

missing push?

@k8s-bot
Copy link

k8s-bot commented Apr 8, 2016

GCE e2e build/test passed for commit 489b1cb55af37393e3f91ec0c8e1970adbe0aadc.

@k8s-bot
Copy link

k8s-bot commented Apr 8, 2016

GCE e2e build/test passed for commit 495ac1a06f1f53df9626629e8358f5be0c8fdd63.

@k8s-bot
Copy link

k8s-bot commented Apr 8, 2016

GCE e2e build/test failed for commit 6279b697a6e7f2f7a247f82291cc696c890c1aab.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

@caesarxuchao
Copy link
Member Author

@k8s-bot e2e test this please, issue #23974

@k8s-bot
Copy link

k8s-bot commented Apr 8, 2016

GCE e2e build/test passed for commit 6279b697a6e7f2f7a247f82291cc696c890c1aab.

@lavalamp
Copy link
Member

LGTM, please squash & add the label

@caesarxuchao caesarxuchao mentioned this pull request Apr 12, 2016
Merged
@caesarxuchao caesarxuchao added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 12, 2016
@caesarxuchao
Copy link
Member Author

Thanks. Squashed. Applying the label.

@k8s-bot
Copy link

k8s-bot commented Apr 12, 2016

GCE e2e build/test passed for commit f315ffd.

@k8s-bot
Copy link

k8s-bot commented Apr 14, 2016

GCE e2e build/test passed for commit f315ffd.

@k8s-bot
Copy link

k8s-bot commented Apr 16, 2016

GCE e2e build/test passed for commit f315ffd.

@k8s-bot
Copy link

k8s-bot commented Apr 17, 2016

GCE e2e build/test passed for commit f315ffd.

@k8s-github-robot
Copy link

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

@k8s-bot
Copy link

k8s-bot commented Apr 17, 2016

GCE e2e build/test passed for commit f315ffd.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit e81663c into kubernetes:master Apr 17, 2016
@caesarxuchao caesarxuchao mentioned this pull request Apr 19, 2016
Closed
@lavalamp lavalamp added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Apr 19, 2016
@lavalamp lavalamp added this to the v1.2 milestone Apr 19, 2016
@lavalamp lavalamp added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 19, 2016
@tomdee tomdee mentioned this pull request Apr 20, 2016
Closed
@caesarxuchao caesarxuchao mentioned this pull request Apr 21, 2016
Closed
@zmerlynn zmerlynn added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Apr 21, 2016
@caesarxuchao caesarxuchao mentioned this pull request Apr 21, 2016
Merged
zmerlynn added a commit that referenced this pull request Apr 21, 2016
@zmerlynn
Merge pull request #24621 from caesarxuchao/automated-cherry-pick-of-…
798c4be 
…#24006-upstream-release-1.2

Automated cherry pick of #24006
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@zmerlynn
Merge pull request kubernetes#24621 from caesarxuchao/automated-cherr…
69adc45 
…y-pick-of-#24006-upstream-release-1.2

Automated cherry pick of kubernetes#24006
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
@zmerlynn
Merge pull request kubernetes#24621 from caesarxuchao/automated-cherr…
bd1265b 
…y-pick-of-#24006-upstream-release-1.2

Automated cherry pick of kubernetes#24006
openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Nov 28, 2019
@k8s-publishing-bot
Merge pull request kubernetes#24006 from bertinatto/cherry-pick-csi-fix
501d199 
[release-4.2] Bug 1762658: UPSTREAM: 83747: Improve efficiency of csiMountMgr.GetAttributes

Origin-commit: a9e953f1319afe0b7a56d5f13d1a6f82e87c51f6
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
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. 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.

None yet
8 participants
@caesarxuchao @k8s-bot @lavalamp @k8s-github-robot @k8s-cherrypick-bot @k8s-teamcity-mesosphere @zmerlynn @googlebot