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

move proxytransport config out of the genericapiserver #34973

Merged
merged 1 commit into from
Oct 20, 2016
Merged

move proxytransport config out of the genericapiserver #34973

merged 1 commit into from
Oct 20, 2016
+16 −43

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Oct 17, 2016 •
edited by k8s-oncall
Loading

Proxy transport is not generic. This moves it to the master config where it is used.

This change is Reviewable

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Oct 17, 2016
@sttts sttts added release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Oct 18, 2016
sttts
sttts reviewed Oct 18, 2016
View reviewed changes
pkg/genericapiserver/genericapiserver_test.go
@@ -89,14 +85,6 @@ func TestNew(t *testing.T) {
serviceReadWriteIP, _ := ipallocator.GetIndexedIP(serviceClusterIPRange, 1)
assert.Equal(s.ServiceReadWriteIP, serviceReadWriteIP)
assert.Equal(s.ExternalAddress, net.JoinHostPort(config.PublicAddress.String(), "6443"))

// These functions should point to the same memory location
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there any value in these tests? They are gone now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was there any value in these tests? They are gone now.

No. It was just wiring that was unused and no longer exists since we only care about the transport.

@sttts
Copy link
Contributor

sttts commented Oct 18, 2016

Only question about the destiny of the old test code. Otherwise, lgtm.

@deads2k deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 18, 2016
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2016
@deads2k deads2k force-pushed the api-28-proxy-dialer branch from d353b3d to 4674cdb Compare October 18, 2016 12:48
@deads2k deads2k 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 18, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2016
@deads2k deads2k force-pushed the api-28-proxy-dialer branch from 4674cdb to 02f3c66 Compare October 18, 2016 13:54
@deads2k deads2k 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 18, 2016
@deads2k
Copy link
Contributor Author

deads2k commented Oct 18, 2016

@k8s-bot unit test this: issue #34976

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 18, 2016
@deads2k deads2k force-pushed the api-28-proxy-dialer branch from 02f3c66 to 9fe79b9 Compare October 19, 2016 13:31
@deads2k deads2k 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 19, 2016
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 19, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 9fe79b90e781b8468987316d6e54457fd5679180. Full PR test history.

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

@deads2k
Copy link
Contributor Author

deads2k commented Oct 19, 2016

@k8s-bot unit test this: issue #34085

@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit 9fe79b90e781b8468987316d6e54457fd5679180. 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.

@deads2k deads2k force-pushed the api-28-proxy-dialer branch from 9fe79b9 to 9578523 Compare October 20, 2016 15:24
@deads2k deads2k 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 20, 2016
@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit 9578523. Full PR test history.

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

@k8s-github-robot
Copy link

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

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit a9e53de into kubernetes:master Oct 20, 2016
@sttts
Copy link
Contributor

sttts commented Oct 21, 2016

43 flakes later 👍

@deads2k deads2k deleted the api-28-proxy-dialer branch February 1, 2017 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sttts sttts sttts left review comments

Assignees

@sttts sttts

Labels
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@deads2k @sttts @k8s-ci-robot @k8s-github-robot @googlebot