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

Add federation api and cm servers to hyperkube #27586

Merged
merged 1 commit into from
Jun 21, 2016

Conversation

colhom
Copy link

@colhom colhom commented Jun 17, 2016

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@k8s-bot
Copy link

k8s-bot commented Jun 17, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

3 similar comments
@k8s-bot
Copy link

k8s-bot commented Jun 17, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Jun 17, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Jun 17, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@colhom colhom force-pushed the federation-hyperkube branch from bd40bb1 to 8b2d1f3 Compare June 17, 2016 01:01
@googlebot
Copy link

CLAs look good, thanks!

@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 Jun 17, 2016
@mikedanese mikedanese assigned nikhiljindal and ghost and unassigned mikedanese Jun 17, 2016
@luxas
Copy link
Member

luxas commented Jun 17, 2016

@quinton-hoole @nikhiljindal Is this one for v1.3?

@colhom
Copy link
Author

colhom commented Jun 17, 2016

@luxas given that federation is a blocker, I would think exposing federation binaries via hyperkube is mandatory functionality for v1.3.

@luxas
Copy link
Member

luxas commented Jun 17, 2016

@mikedanese @ixdy @lavalamp Someone that may trigger the tests?

@lavalamp
Copy link
Member

add to whitelist

@ghost
Copy link

ghost commented Jun 17, 2016

My apologies, I overlooked this PR. Sorting it out now.

@ghost
Copy link

ghost commented Jun 17, 2016

ok to test

@ghost ghost added release-note Denotes a PR that will be considered when it comes time to generate release notes. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed release-note-label-needed labels Jun 17, 2016
@ghost ghost added this to the v1.3 milestone Jun 17, 2016
@colhom
Copy link
Author

colhom commented Jun 17, 2016

Verification failures do not seem related to this PR:

Verifying ./hack/../hack/verify-govet.sh
federation/pkg/dnsprovider/providers/aws/route53/testing/route53api.go:79: arg change.ResourceRecordSet.Name for printf verb %s of wrong type: *string
federation/pkg/dnsprovider/providers/aws/route53/testing/route53api.go:84: arg change.ResourceRecordSet.Name for printf verb %s of wrong type: *string
federation/pkg/dnsprovider/providers/aws/route53/testing/route53api.go:105: arg input.Name for printf verb %s of wrong type: *string
federation/pkg/dnsprovider/providers/google/clouddns/clouddns.go:110: k8s.io/kubernetes/federation/pkg/dnsprovider/providers/google/clouddns/internal/stubs.ManagedZone composite literal uses unkeyed fields
FAILED   ./hack/../hack/verify-govet.sh 342s

....

Verifying ./hack/../hack/verify-symbols.sh
Go version: go version go1.6.2 linux/amd64
+++ [0617 13:06:17] Building the toolchain targets:
    k8s.io/kubernetes/hack/cmd/teststale
+++ [0617 13:06:18] Building go targets for linux/amd64:
    cmd/hyperkube
+++ [0617 13:07:01] Placing binaries
Found bad symbol 'testing[.]':
00000000042371c0 r go.importpath.k8s.io/kubernetes/federation/pkg/dnsprovider/providers/aws/route53/testing.
00000000057cb858 b go.itab.*k8s.io/kubernetes/vendor/github.com/aws/aws-sdk-go/service/route53.Route53.k8s.io/kubernetes/federation/pkg/dnsprovider/providers/aws/route53/testing.Route53API
000000000188e470 t k8s.io/kubernetes/federation/pkg/dnsprovider/providers/aws/route53/testing.init
00000000057c85db b k8s.io/kubernetes/federation/pkg/dnsprovider/providers/aws/route53/testing.initdone.
FAILED   ./hack/../hack/verify-symbols.sh   47s

\cc @quinton-hoole @nikhiljindal

@k8s-github-robot
Copy link

@goltermann
You must link to the test flake issue which caused you to request this manual re-test.
Re-test requests should be in the form of: k8s-bot test this issue: #<number>
Here is the list of open test flakes.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 18, 2016
@goltermann goltermann added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 18, 2016
@goltermann
Copy link
Contributor

@k8s-bot verify this issue: #IGNORE

@goltermann
Copy link
Contributor

@colhom No idea why this is the only PR hung up on those vet issues, but they're being fixed by @mikedanese in #27558.

@goltermann
Copy link
Contributor

@k8s-bot verify this issue: #IGNORE

@goltermann
Copy link
Contributor

@colhom the vet errors are cleaned up, but the symbols issue does appear to be related to your PR. Take a look at hack/verify-symbols.sh. It's flagging that your PR now builds in symbols named "testing" into hyperkube.

@quinton-hoole any ideas? Should we alter the regex at https://github.com/kubernetes/kubernetes/blob/master/hack/verify-symbols.sh#L32, or alter something in federation code?

@ghost
Copy link

ghost commented Jun 20, 2016

@goltermann @colhom I'm guessing that this might be tripping up the verify-symbols.sh check.

https://github.com/kubernetes/kubernetes/tree/642049652b991e69f6bd2eb1ba3c5112f0f57339/federation/pkg/dnsprovider/providers/aws/route53/testing

The easiest might be to rename that package. I can send in a PR for that. I'm not familiar enough with the aims behind verify-symbols to know for sure whether that would be subverting a good check or not. That package is so minute that I don' think it makes much difference whether it's in or out of the production binaries.

@ghost
Copy link

ghost commented Jun 20, 2016

#27734 should do the trick.

k8s-github-robot pushed a commit that referenced this pull request Jun 21, 2016
…vider-route53-testing-package

Automatic merge from submit-queue

Rename dnsprovider/providers/aws/route53/internal/testing to stubs.

Resolves #27586 (comment)

cc @colhom @goltermann
@goltermann
Copy link
Contributor

@k8s-bot verify this issue: #27734

@goltermann goltermann reopened this Jun 21, 2016
@k8s-bot
Copy link

k8s-bot commented Jun 21, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

2 similar comments
@k8s-bot
Copy link

k8s-bot commented Jun 21, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Jun 21, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@goltermann
Copy link
Contributor

@k8s-bot ok to test

@k8s-bot
Copy link

k8s-bot commented Jun 21, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

@k8s-bot
Copy link

k8s-bot commented Jun 21, 2016

GCE e2e build/test passed for commit 8b2d1f3.

@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 Jun 21, 2016

GCE e2e build/test passed for commit 8b2d1f3.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 6a7fd05 into kubernetes:master Jun 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants