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

Detailed installation instructions #106

Merged
merged 1 commit into from
Feb 15, 2017
Merged

Conversation

lavalamp
Copy link
Member

Fix #105.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 14, 2017
@k8s-reviewable
Copy link

This change is Reviewable

Copy link
Member

@caesarxuchao caesarxuchao left a comment

Choose a reason for hiding this comment

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

@lavalamp Feel free to merge if you think you know the answers :)


# Make sure you have a go file in your directory which imports k8s.io/client-go
# first--I suggest copying one of the examples.
$ dep ensure k8s.io/client-go@^1.5.1
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should use 2.0-alpha or 2.0-beta as an example. 1.5.1 is not maintained at the moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't randomly stop maintaining branches. I'll make an issue about 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.

INSTALL.md Outdated
godep save ./...
```

Alternatively, if you want to build using the dependencsie in your `$GOPATH`,
Copy link
Member

Choose a reason for hiding this comment

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

s/dependencsie/dependencies

Copy link
Member Author

Choose a reason for hiding this comment

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

done

INSTALL.md Outdated
```

At this point, you could copy the `glide.yaml` file into your own project and
run `glide up`. Alternatively, if you already have a `glide.yaml` file, you'll
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look right. The glide.yaml won't contain any entry for client-go, but "my own project" depends on client-go. Could you double check?

Sorry, I can't verify if this works. The redirection of gopkg.in doesn't work for me, maybe it's because on mac, or because i'm in China..

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, good point, I'll fix

@lavalamp
Copy link
Member Author

Self merging to get this out there.

@lavalamp lavalamp merged commit 8b466d6 into kubernetes:master Feb 15, 2017
@@ -0,0 +1,136 @@
# Installing client-go

## For the casual user
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that this approach will ever work. The minute you start using any API objects you pull in apimachinery from your gopath. I think the only real way to make this work is to flatten dependencies.

$ mkdir -p ~/tmp/go
$ export GOPATH=~/tmp/go
$ go get k8s.io/client-go/...
$ cd ~/tmp
$ cp go/src/k8s.io/client-go/examples/out-of-cluster/main.go
$ go build main.go
main.go:24:2: cannot find package "k8s.io/apimachinery/pkg/apis/meta/v1" in any of:
	/usr/local/Cellar/go/1.7.4/libexec/src/k8s.io/apimachinery/pkg/apis/meta/v1 (from $GOROOT)
	/Users/jbeda/tmp/go/src/k8s.io/apimachinery/pkg/apis/meta/v1 (from $GOPATH)
$ go get k8s.io/apimachinery/...
$ go build main.go
# command-line-arguments
./main.go:46: cannot use "k8s.io/apimachinery/pkg/apis/meta/v1".ListOptions literal (type "k8s.io/apimachinery/pkg/apis/meta/v1".ListOptions) as type "k8s.io/client-go/vendor/k8s.io/apimachinery/pkg/apis/meta/v1".ListOptions in argument to clientset.CoreV1().Pods("").List

Copy link
Member Author

Choose a reason for hiding this comment

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

Chao is removing such dependencies (k8s.io/apimachinery & glog for starters) from vendor/

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Verified that #2077 did fix the problem @jbeda mentioned.

@jbeda
Copy link
Contributor

jbeda commented Feb 15, 2017

This is a huge improvement but I don't think we have a solution here without flattening dependencies. This sucks but I'm not sure what is to be done.


One thing to note about dep is that it will omit dependencies that aren't
actually used, and some dependencies of `client-go` are used only if you import
one of the plugins (for example, the auth plugins). So you may need to run `dep
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it's a bug and I'm pretty sure I filed an issue about it some time ago.

@sttts
Copy link
Contributor

sttts commented Feb 16, 2017

@lavalamp well written. Thanks!


```sh
$ go get github.com/golang/dep
$ go install github.com/golang/dep/cmd
Copy link
Member

Choose a reason for hiding this comment

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

this is already out-of-date btw.

go install github.com/golang/dep/cmd/dep

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 think someone already fixed it.

@jbeda
Copy link
Contributor

jbeda commented Mar 1, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants