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

VPA: go modules reference local files #2507

Closed
wozniakjan opened this issue Nov 1, 2019 · 6 comments · Fixed by #2531
Closed

VPA: go modules reference local files #2507

wozniakjan opened this issue Nov 1, 2019 · 6 comments · Fixed by #2531

Comments

@wozniakjan
Copy link
Member

#2486 has added support for go modules but the modules file references locally checked out repositories that may not be present on other machines

k8s.io/api => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/api
k8s.io/apiextensions-apiserver => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/apiextensions-apiserver
k8s.io/apimachinery => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/apimachinery
k8s.io/apiserver => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/apiserver
k8s.io/cli-runtime => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/cli-runtime
k8s.io/client-go => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/client-go
k8s.io/cloud-provider => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/cloud-provider
k8s.io/cluster-bootstrap => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/cluster-bootstrap
k8s.io/code-generator => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/code-generator
k8s.io/component-base => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/component-base
k8s.io/cri-api => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/cri-api
k8s.io/csi-translation-lib => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/csi-translation-lib
k8s.io/gengo => k8s.io/gengo v0.0.0-20190822140433-26a664648505
k8s.io/heapster => k8s.io/heapster v1.2.0-beta.1
k8s.io/klog => k8s.io/klog v0.4.0
k8s.io/kube-aggregator => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/kube-aggregator
k8s.io/kube-controller-manager => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/kube-controller-manager
k8s.io/kube-openapi => k8s.io/kube-openapi v0.0.0-20190816220812-743ec37842bf
k8s.io/kube-proxy => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/kube-proxy
k8s.io/kube-scheduler => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/kube-scheduler
k8s.io/kubectl => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/kubectl
k8s.io/kubelet => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/kubelet
k8s.io/legacy-cloud-providers => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/legacy-cloud-providers
k8s.io/metrics => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/metrics
k8s.io/node-api => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/node-api
k8s.io/repo-infra => k8s.io/repo-infra v0.0.0-20181204233714-00fe14e3d1a3
k8s.io/sample-apiserver => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/sample-apiserver
k8s.io/sample-cli-plugin => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/sample-cli-plugin
k8s.io/sample-controller => /usr/local/google/home/bskiba/k8s-vpa-update-deps/kubernetes/staging/src/k8s.io/sample-controller

The recommended hack scripts to update dependencies also seem to be missing from the repository

// This is a generated file. Do not edit directly.
// Run hack/pin-dependency.sh to change pinned dependency versions.
// Run hack/update-vendor.sh to update go.mod files and the vendor directory.

cc: @bskiba

@bskiba
Copy link
Member

bskiba commented Nov 4, 2019

#2486 Does not unfortunately fully add support for go modules. TL;DR: we use go mod only to construct the vendor directory, based on kubernetes/kubernetes dependencies.

Long version:
VPA (and Cluster Autoscaler as well) has an issue where we depend on both kubernetes/kubernetes repo and a bunch of stuff that is synced from kubernetes/staging repo to other repos. This makes managing dependencies really hard.

What we do is we copy the go.mod file from kubernetes/kubernetes repo and then fix all the dependencies to what is in the staging directory. Then we use go mod to construct the vendor directory. So the go.mod file is not fully valid. See https://github.com/kubernetes/autoscaler/blob/master/vertical-pod-autoscaler/update_deps.sh

I'd like to both document the process better and work on making this setup less ugly, but it will take some time. If you have ideas, let me know.
The scripts mentioned in the auto-generated files will not work with this setup unfortunately, but I don't know if I can make go mod skip generation of that part.

@wozniakjan
Copy link
Member Author

wondering if we can reuse any ideas from kubernetes/kubernetes#79384 or in the worst case create a staging dir here as well to move the absolute paths /usr/local/google/home/bskiba from go.mod to relative so others can collaborate and update the dependencies?

I quite like and prefer the script mentioned in kubernetes/kubernetes#79384 (comment) which based on depending kubernetes/kubernetes version figures out the sha1 of those staged repos. That should transform all replace xyz => /absolute/path/xyz directives to replace xyz => xyz v0.0.0-date-sha which sounds to me like more portable and reproducible environment. What do you think?

I have used it in #2485 (f98049c), so you can inspect the output and go.mod a little bit before trying it. The tests were passing but not sure if it indeed covered all scenarios necessary. Also in my PR I completely moved from go dep to go mod which, I understand now, may not be desired?

@bskiba
Copy link
Member

bskiba commented Nov 7, 2019

The script looks really handy. Would you be willing to have a go at another PR that incorporates the script into the deps management, so that we can pin our dependencies to some k8s version? Moving to go mod should be fine, as long as we keep the vendor directory.

@wozniakjan
Copy link
Member Author

wozniakjan commented Nov 7, 2019

sure thing, happy to help (probably will find time either late Friday or this weekend) :)

@bskiba
Copy link
Member

bskiba commented Nov 7, 2019

Perfect, thank you! Let me know if you need any help.

@wozniakjan
Copy link
Member Author

wozniakjan commented Nov 11, 2019

To keep you informed @bskiba, over the weekend there has been some discussion about go modules and common practices when using kubernetes as modules - kubernetes/kube-state-metrics#949 (comment)

Feel free to chime in, we are trying to identify requirements and potential paths about how to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants