fixed gopkg.toml for Kubebuilder and generated project #122
Conversation
k8s-ci-robot
added
cncf-cla: yes
test.sh
Outdated
| @@ -221,4 +240,6 @@ prepare_testdir_under_gopath | |||
|
|
|||
| generate_crd_resources | |||
| test_generated_controller | |||
| run_dep_ensure | |||
| test_generated_controller | |||
There was a problem hiding this comment.
I don't expect test_generated_controller to be called twice. Maybe you only need it after run_dep_ensure.
There was a problem hiding this comment.
I wanted to keep the tests as close as possible to the end user's workflow. users or we don't invoke dep ensure currently, so wanted to test controller tests before and after dep ensure.
I will add a comment explaining that. We will eventually have test controller when we start running dep ensure in project setup itself.
There was a problem hiding this comment.
That's an interesting angle I hadn't thought of! That makes a lot more sense now.
There was a problem hiding this comment.
Added a comment in script explaining the reasoning.
test.sh
Outdated
| # like `dep ensure`. | ||
| # | ||
| # $ KB_VERSION=0.1.7 test.sh | ||
| KB_VERSION=${KB_VERSION:-unknown} |
There was a problem hiding this comment.
In the Travis testing, do we want to test this against one kb_version or multiple?
There was a problem hiding this comment.
hmm...to cover version scenario, we should have an additional tests (preferably a unit test). Running travis with another version may turn out to be an over-kill and can bloat these tests.
There was a problem hiding this comment.
Also clarified the semantics for this version and renamed it to reflect its functionality.
| // updateDepConfig updates the Dep config Gopkg.toml to include Kubebuilder | ||
| // project dependency. It uses the Kubebuilder version to determine whether | ||
| // to include branch or version in the contraint stanza. | ||
| func updateDepConfig() error { |
There was a problem hiding this comment.
What usage does the generated project make of kubebuilder as a vendored dep? Does it depend on kubebuilder/pkg?
I have been wondering about this, still as a relative newcomer without a lot of hands on experience.
There was a problem hiding this comment.
Yes, you are right. The generated project uses controller libs from kubebuilder/pkg. Godoc link --> https://godoc.org/github.com/kubernetes-sigs/kubebuilder
Highlights: - Removed self dependency in Gopkg.toml for Kubebuilder project - Update the Gopkg.toml file correctly for generated project - Added `dep ensure` test in test.sh - Update Travis config to add 'dep install'
|
Addressed review comments. PTAL. |
|
I am thinking if it could be better to remove constraints of below in Gopkg.toml It is because that the listed constraints are not used in client side generated project. Run dep in client side will cause |
|
@fanzhangio The reason I did not remove those constraints is because we share Gopkg.toml/lock file for kubebuilder project and the generated projects. Current Gopkg.toml file is good for Kubebuilder project but has extra pkgs for generated packages. I am working on a PR which decouples these files, so "dep ensure" warnings for generated project will get addressed in that PR. |
Highlights:
dep ensuretest in test.shNote: This also brings us one step closer to the goal of "not shipping vendors dir" in KB build. I will make a separate PR for that.