Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Fix Godeps in cluster-autoscaler #1251

Closed
zmerlynn opened this issue Jun 22, 2016 · 10 comments
Closed

Fix Godeps in cluster-autoscaler #1251

zmerlynn opened this issue Jun 22, 2016 · 10 comments

Comments

@zmerlynn
Copy link
Contributor

zmerlynn commented Jun 22, 2016

I'm about to put up an embarassing PR to fix kubernetes/kubernetes#27821 that will manually alter the vendor/ directory, because the Godeps are currently in conflict, but we need to get a release out:

$ godep go build ./...
# k8s.io/contrib/cluster-autoscaler/utils/gce
utils/gce/gce.go:65: cannot use gce.NewAltTokenSource(cfg.Global.TokenURL, cfg.Global.TokenBody) (type "k8s.io/kubernetes/vendor/golang.org/x/oauth2".TokenSource) as type "k8s.io/contrib/cluster-autoscaler/vendor/golang.org/x/oauth2".TokenSource in assignment:
        "k8s.io/kubernetes/vendor/golang.org/x/oauth2".TokenSource does not implement "k8s.io/contrib/cluster-autoscaler/vendor/golang.org/x/oauth2".TokenSource (wrong type for Token method)
                have Token() (*"k8s.io/kubernetes/vendor/golang.org/x/oauth2".Token, error)
                want Token() (*"k8s.io/contrib/cluster-autoscaler/vendor/golang.org/x/oauth2".Token, error)
# k8s.io/contrib/cluster-autoscaler/simulator
simulator/cluster.go:78: undefined: cmd.GetPodsForDeletionOnNodeDrain
simulator/nodes.go:31: undefined: cmd.GetPodsForDeletionOnNodeDrain
godep: go exit status 2
@eparis
Copy link
Contributor

eparis commented Jun 22, 2016

Wait, why is kubernetes not being vendored in? Unlike GOPATH (which is what Godep used) using vendor is a much stronger all or nothing. You must vendor everything....

zmerlynn added a commit to zmerlynn/contrib that referenced this issue Jun 22, 2016
Fix to kubernetes/kubernetes#27821. Borrows
the necessary piece of
kubernetes/kubernetes#27741 to parse the
gce.conf.

Also opened kubernetes-retired#1251 to fix Godeps correctly
@zmerlynn
Copy link
Contributor Author

@eparis: It is being vendored in, AFAICT. That's after I updated the k8s dep.

@eparis
Copy link
Contributor

eparis commented Jun 22, 2016

The fact that it says you have a k8s.io/kubernetes/vendor/golang.org/x/oauth2 says that it is using $GOPATH/src/k8s.io/kubernetes instead of $GOPATH/src/k8s.io/contrib/vendor/k8s.io/kubernetes. hmmmm

@zmerlynn
Copy link
Contributor Author

@eparis: The second error is the more obnoxious one, and looks like it'll take a PR to the main repo (unless I'm missing something). It's very possible I just screwed something up, too.

@eparis
Copy link
Contributor

eparis commented Jun 22, 2016

"I think you did something wrong to get the first error, I don't get it"

However, yes, it does seem the second error is going to require some change in pkg/kubectl/cmd/drain.go :-(

@aledbf
Copy link
Contributor

aledbf commented Jun 22, 2016

@zmerlynn
Copy link
Contributor Author

@eparis: Yeah, I assume I did.
@aledbf: Yup, that's the culprit, thanks.

Regardless, we chose a faster route rather than me pounding my head against Godeps even longer. If someone wants to start on the right way, we can get it in place for v1.3.1, but it doesn't seem high priority.

@eparis
Copy link
Contributor

eparis commented Jun 22, 2016

@zmerlynn agreed with path chosen.

@davidopp
Copy link
Contributor

cc/ @mml

mwielgus pushed a commit to kubernetes/autoscaler that referenced this issue Apr 14, 2017
Fix to kubernetes/kubernetes#27821. Borrows
the necessary piece of
kubernetes/kubernetes#27741 to parse the
gce.conf.

Also opened kubernetes-retired/contrib#1251 to fix Godeps correctly
mwielgus pushed a commit to kubernetes/autoscaler that referenced this issue Apr 18, 2017
Fix to kubernetes/kubernetes#27821. Borrows
the necessary piece of
kubernetes/kubernetes#27741 to parse the
gce.conf.

Also opened kubernetes-retired/contrib#1251 to fix Godeps correctly
mwielgus pushed a commit to kubernetes/autoscaler that referenced this issue Apr 18, 2017
Fix to kubernetes/kubernetes#27821. Borrows
the necessary piece of
kubernetes/kubernetes#27741 to parse the
gce.conf.

Also opened kubernetes-retired/contrib#1251 to fix Godeps correctly
@mwielgus
Copy link
Contributor

Fixed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants