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

Google Cloud DNS dnsprovider - replacement for #25389 #26020

Merged
2 commits merged into from May 28, 2016
Merged

Google Cloud DNS dnsprovider - replacement for #25389 #26020

2 commits merged into from May 28, 2016

Conversation

ghost
Copy link

@ghost ghost commented May 21, 2016

Add Google Cloud DNS dnsprovider.

@ghost ghost added 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. team/control-plane labels May 21, 2016
@ghost ghost added this to the v1.3 milestone May 21, 2016
@ghost ghost assigned madhusudancs May 21, 2016
@ghost
Copy link
Author

ghost commented May 21, 2016

I somehow messed up my local git repo - the easiest way to fix it was to create this new PR, which replaces #25389.

@k8s-github-robot k8s-github-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 21, 2016
@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 21, 2016
@ghost ghost mentioned this pull request May 21, 2016
@ghost
Copy link
Author

ghost commented May 21, 2016

Seeing as this is identical to #25389, reapplying LGTM here.

@ghost
Copy link
Author

ghost commented May 21, 2016

@mfanjie FYI

@ghost
Copy link
Author

ghost commented May 21, 2016

Fixed boilerplate comment in rrstype.go. Will reapply LGTM.

@ghost ghost 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 May 21, 2016
@ghost
Copy link
Author

ghost commented May 21, 2016

google.golang.org/api/dns is missing from the vendor tree. Investigating...

federation/pkg/dnsprovider/providers/google/clouddns/clouddns.go:27:2: cannot find package "google.golang.org/api/dns/v1" in any of:
    /go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/google.golang.org/api/dns/v1 (vendor tree)

@ghost
Copy link
Author

ghost commented May 21, 2016

Blocked on #26026

@ghost
Copy link
Author

ghost commented May 23, 2016

@nikhiljindal FYI, as you're doing such a marvellous job of shepherding the cluster federation PR's through the pipeline.

@bprashanth
Copy link
Contributor

fyi

federation/pkg/dnsprovider/providers/google/clouddns/clouddns.go:27:2: cannot find package "google.golang.org/api/dns/v1" in any of:
    /go/src/k8s.io/kubernetes/_output/local/go/src/k8s.io/kubernetes/vendor/google.golang.org/api/dns/v1 (vendor tree)
    /usr/local/go/src/google.golang.org/api/dns/v1 (from $GOROOT)
    /go/src/k8s.io/kubernetes/_output/local/go/src/google.golang.org/api/dns/v1 (from $GOPATH)
+++ [0521 12:04:18] Saved JUnit XML test report to /workspace/artifacts/junit_v1,extensions-v1beta1,metrics-v1alpha1,federation-v1alpha1_20160521-120417.xml
!!! Error in ./hack/test-go.sh:241

@ghost
Copy link
Author

ghost commented May 23, 2016

@bprashanth Yes, thanks, that's why it's blocked on #26026

@ghost ghost 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 May 24, 2016
@ghost
Copy link
Author

ghost commented May 24, 2016

OK, dependency issues should be resolved now. Hopefully this will auto-merge as soon as the merge-bot is working again.

@ghost
Copy link
Author

ghost commented May 24, 2016

Same problem as #26026

Verifying ./hack/../hack/verify-godep-licenses.sh
Checking for 'Godeps/' changes against 'origin/master'
Created workspace: /go/src/k8s.io/kubernetes/_tmp/kube-godep-licenses.UNebSp
Your godep licenses file is out of date. Run hack/update-godep-licenses.sh --create-missing and commit the results.
--- /go/src/k8s.io/kubernetes/Godeps/LICENSES   2016-05-24 13:37:08.800171089 -0700
+++ /go/src/k8s.io/kubernetes/_tmp/kube-godep-licenses.UNebSp/Godeps/LICENSES   2016-05-24 13:53:40.696567852 -0700
@@ -64925,6 +64925,38 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE


 ================================================================================
+= vendor/google.golang.org/api/dns/v1 licensed under: =
+
+Copyright (c) 2011 Google Inc. All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are
+met:
+
+   * Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+   * Redistributions in binary form must reproduce the above
+copyright notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with the
+distribution.
+   * Neither the name of Google Inc. nor the names of its
+contributors may be used to endorse or promote products derived from
+this software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+================================================================================
 = vendor/google.golang.org/api/gensupport licensed under: =

 Copyright (c) 2011 Google Inc. All rights reserved.
Removing workspace: /go/src/k8s.io/kubernetes/_tmp/kube-godep-licenses.UNebSp
FAILED   ./hack/../hack/verify-godep-licenses.sh    15s

@k8s-github-robot
Copy link

@quinton-hoole
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.

@ghost
Copy link
Author

ghost commented May 25, 2016

@k8s-bot test this issue: #26270

@ghost
Copy link
Author

ghost commented May 26, 2016

Re-applying LGTM

@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2016
##### update the Godeps/LICENSES file accordingly, but it doesn't.
##### Disabling this check for now to unblock adding dependencies to Kubernetes.

exit 0
Copy link
Member

Choose a reason for hiding this comment

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

I don't see a P0 issue opened for this, but this will quickly bring us out of license compliance, perhaps with this PR itself.

cc @david-mcmahon @lavalamp

@zmerlynn zmerlynn removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2016
@zmerlynn
Copy link
Member

Please open a P0 issue on the license issue and reapply LGTM.

@ghost
Copy link
Author

ghost commented May 26, 2016

Thanks @zmerlynn.
I opened #26328

@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2016
@alex-mohr alex-mohr removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 26, 2016
@alex-mohr
Copy link
Contributor

I have a few questions about this PR I'd like to get answered before we merge, so removed LGTM. Please don't re-apply until then.

@madhusudancs
Copy link
Contributor

@alex-mohr where are the questions?

@david-mcmahon
Copy link
Contributor

@quinton-hoole I cannot repro, but I suspect I'm not putting this together correctly:

$ git fetch origin pull/26020/head:pr-26020
$ git checkout pr-26020
$ vi hack/verify-godep-licenses.sh # and remove the exit 0 from this PR
$ hack/verify-godep-licenses.sh
$ echo $?
1
$ hack/update-godep-licenses.sh
$ hack/verify-godep-licenses.sh
$ echo $?
0

It should be easy enough to remove the jq requirement and bash 4 has been out for 7 years. How was that not updated yet?

@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 26, 2016
@ghost
Copy link
Author

ghost commented May 26, 2016

OK, I removed all of the {verify|update}-godep-license.sh related stuff in this PR, as the fixes for that are in #26348 (which should merge shortly).

All that's required here now is (I hope) to rebase against HEAD once #26348 is in, run hack/update-godep-license.sh, and commit the results. I'll shepherd that through.

Quinton Hoole added 2 commits May 27, 2016 15:22
@ghost
Copy link
Author

ghost commented May 27, 2016

OK @madhusudancs @nikhiljindal I've rebased against the above and regenerated godeps licenses. I think this is good to merge now, once tests pass.

@ghost ghost added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 27, 2016
@k8s-github-robot
Copy link

@quinton-hoole
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 May 28, 2016
@k8s-bot
Copy link

k8s-bot commented May 28, 2016

GCE e2e build/test passed for commit 2240da1.

@mfanjie
Copy link

mfanjie commented May 28, 2016

@mfanjie notify me

@ghost
Copy link
Author

ghost commented May 28, 2016

All tests have passed. Merge queue is blocked, so manually merging, as this is a v1.3 blocker, with several other blocker PR's waiting for it to merge.

@ghost ghost merged commit 4983183 into kubernetes:master May 28, 2016
@ghost
Copy link
Author

ghost commented May 28, 2016

@mfanjie FYI, merged.

@mfanjie
Copy link

mfanjie commented May 28, 2016

working on it.

k8s-github-robot pushed a commit that referenced this pull request Jun 4, 2016
…nsprovider

Automatic merge from submit-queue

AWS Route53 dnsprovider

Still needs unit tests, and some other cleanup.  Review not urgent, but feel free to make a first pass.
Only need to look at the last two commits.  The prior commits will go in as #26020. This will need to be rebased against #26020 once that merges.
 
It's a bare minimum implementation, only what's required for Ubernetes Federated Services (managing basic A and CNAME records).  More functionality (health checks, geolocation etc) can be fairly easily added as required.

It also requires github.com/aws/aws-sdk-go/service/route53 to be vendored into godeps, which I haven't managed to do successfully yet (Oh Godep!)

cc: @justinsb FYI
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants