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

Generated REST client code for the DANM CRDs should be stored in the repository #47

Closed
antalm opened this issue Feb 22, 2019 · 12 comments
Closed
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@antalm
Copy link
Contributor

antalm commented Feb 22, 2019

Is this a BUG REPORT or FEATURE REQUEST?:
Bug

What happened:
I tried to build a standalone application that imports the k8s client library generated from github.com/nokia/danm/pkg/crd/apis/danm/v1, that is:

import (
  ...
  danmtypes "github.com/nokia/danm/pkg/crd/apis/danm/v1"
  danmclientset "github.com/nokia/danm/pkg/crd/client/clientset/versioned"
)

golang's dep init / ensure fails with:

init failed: unable to solve the dependency graph: Solving failure: No versions of github.com/nokia/danm met constraints:
	v3.1.0: Could not introduce github.com/nokia/danm@v3.1.0, as its subpackage github.com/nokia/danm/pkg/crd/client/clientset/versioned is missing. (Package is required by (root).)
	v3.0.0: Could not introduce github.com/nokia/danm@v3.0.0, as its subpackage github.com/nokia/danm/pkg/crd/client/clientset/versioned is missing. (Package is required by (root).)
	master: Could not introduce github.com/nokia/danm@master, as its subpackage github.com/nokia/danm/pkg/crd/client/clientset/versioned is missing. (Package is required by (root).)

Unfortunately I found no way to generate the k8s client on my own and then successfully use dep init even when playing with Gopkg.toml ignored parameter (first generating dependencies ignoring the generated code, then removing the ignore for generated code and updating dependencies).

I believe the generated informers (github.com/nokia/danm/pkg/crd/client/informers/externalversions) and listers (github.com/nokia/danm/pkg/crd/client/listers/danm/v1) are also missing from the repo.

What you expected to happen:
golang dep init / ensure should be able to gather all dependencies. According to this ticket (golang/dep#1077) go dep currently assumes all generated code is committed to source repository.

How to reproduce it:
Write any code that tries to import github.com/nokia/danm/pkg/crd/client/clientset/versioned then run "dep init" or "dep ensure".

Environment:

  • danm version: 3.0.0
  • Kubernetes version (use kubectl version): 1.12.4
  • danm configuration: not relevant
  • OS (e.g. from /etc/os-release): CentOS Linux 7 (Core)
  • Kernel (e.g. uname -a): Linux antal 3.10.0-862.2.3.el7.x86_64 Correcting the LICENSE link #1 SMP Wed May 9 18:05:47 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
@Levovar
Copy link
Collaborator

Levovar commented Feb 23, 2019

the dependency manager tool used by the project is glide, not dep
proper glide.lock, and glide.yaml files are in place
if we would move away from glide, we would move to the official Golang version management system, that is: vgo, and go modules. It is not dep

also, I'm very much against the practice of storing generated code under source control. people will inevitably try and modify it, then the generated and the stored code will fall out of synch, then all kinds of problems will happen
generating the code is the job of a CI. and we have a fully automated CI, which does just that.
you should consider using the already provided scripting to build the binaries, rather than trying to do it on your own, manually

@antalm
Copy link
Contributor Author

antalm commented Feb 25, 2019

I have similar issue with glide update:
[INFO] Resolving imports
...
[INFO] --> Fetching updates for github.com/Nvveen/Gotty.
[ERROR] Error scanning github.com/nokia/danm/pkg/crd/client/clientset/versioned: open /root/.glide/cache/src/https-git.luolix.top-nokia-danm/pkg/crd/client/clientset/versioned: no such file or directory

Among other things I want to import the following in a standalone application:
danmtypes "github.com/nokia/danm/pkg/crd/apis/danm/v1"
danmclientset "github.com/nokia/danm/pkg/crd/client/clientset/versioned"
"github.com/nokia/danm/pkg/danmep"
"github.com/nokia/danm/pkg/ipam"

I used the github.com/nokia/danm/pkg/glide.yaml and lock file, but I had to remove the following ignores from glide.yaml in my project so that the packages are downloaded by glide install/update:

  • github.com/nokia/danm/pkg/crd
  • github.com/nokia/danm/pkg/danmep
  • github.com/nokia/danm/pkg/ipam

And I had to add the following ignores otherwise it would complain about the missing generated code:

  • github.com/nokia/danm/pkg/crd/client/clientset/versioned
  • github.com/nokia/danm/pkg/crd/client/informers/externalversions

My problem with generated code not being committed to source repo is that this way danm can only be built on its own, but its subpackages cannot be integrated and used in a standalone application.

Other projects also commit generated code to source repository and there's a git push hook that checks whether the generated code and the committed code differs and warns the user before it is pushed to source repository.

@Levovar
Copy link
Collaborator

Levovar commented Feb 25, 2019

I understand your points, but is it just as easy to generate the code in your standalone project, as it is to generate within DANM.
Here it is automated: https://github.com/nokia/danm/blob/master/build/build.sh#L9

But okay, I will meet you half-way :) If someone would commit the generated code together (e.g. same PR) with this automated CI enhancement checking that generated code 100% matches the stored, I'm okay with that :)

@Levovar Levovar changed the title dep init missing github.com/nokia/danm/pkg/crd/client/clientset/versioned Generated REST client code for the DANM CRDs should be stored in the repository Feb 26, 2019
@Levovar Levovar added help wanted Extra attention is needed good first issue Good for newcomers labels Feb 26, 2019
@Levovar
Copy link
Collaborator

Levovar commented Feb 27, 2019

aaaaaand one day later somebody thought it is a good idea to remove the generation of unversioned interfaces from the code generator in a totally non-backward compatible way, so I'm double convinced now

we also need to fix the exact version of the generator we will use in this new CI process

@janosi
Copy link
Contributor

janosi commented Feb 28, 2019

Excuse me for entering. Actually, one of my worst developer experiences was when I had to work with a git project that had derived (compiled) parts in it. OK, that was a bit bigger project with a lot of parallel contributors (no offence, I wish the same for danm, definitely), but it was a pain to manage collisions in my generated stuff and other's generated stuff (no surprise they eliminated the generated stuff from the repo - more or less).
Is it possible to store the generated things in another repo?

@Levovar
Copy link
Collaborator

Levovar commented Feb 28, 2019

do you mean the vendor directory in kubernetes/kubernetes? actually my open-source guy heavily pushing us to also store our vendor in our repo :)

I think here the situation is anyway different in a sense, that there is no "your", or "my" generated stuff. there is only one package which is generated, everyone needs that, everyone needs the same code, and everyone shall anyway use the freshly generated version, nobody will modify that.

do you see any issue with the approach of not letting in a PR if somebody modified the generated code manually? next PR would just check out whatever is in the repo, and I guess that's it

@janosi
Copy link
Contributor

janosi commented Feb 28, 2019

I mean, in that project (k8s ;) the generated API "whatever" (generated...) files were stored in the ordinary repo (in 2016). So, if I changed the API, I had to re-generate those (because that was part of the repo). If someone else changed the API meanwhile...that was a great opportunity to learn the depths of git... (in 2016, so I only remember the "fun" part, not the exact details, sorry :(
kubernetes/kubernetes#8830
I think the vendor thing is a bit different. In the vendor dir we keep the source of the version of the 3rd party code with which we are compliant at the given hash.
Or I can be totally wrong, too :)

@Levovar
Copy link
Collaborator

Levovar commented Feb 28, 2019

ah, thanks!

yes, they removed it. and as I see they are discussing it ever since whether it was a good idea, or not :)

@Levovar
Copy link
Collaborator

Levovar commented Feb 28, 2019

I think in our case:

  • I don't expect DanmNet and DanmEp APIs to change that frequently as the K8s API
  • merge problems are only an issue if you don't rebase to the head of master, which you kinda should... :)

@antalm
Copy link
Contributor Author

antalm commented Mar 1, 2019

The reason why it would be good if generated code was added is because otherwise glide / dep or other dependency tools cannot work. And if you put the generated code import to ignore then all its dependency subtree has to be added manually. Which is of course a pain to a maintain.

As a workaround, to build my standalone application I now git clone whole danm (not only the needed subpackages) and copy my standalone application code to github.com/nokia/danm/pkg/ so that it can find the generated code and and its dependencies that is installed by glide in build.sh. However if my application also has dependencies then I need to add them, because a glide update would refresh all dependencies' versions, not only add the still missing dependencies. So now I have a script to automatically add my missing dependencies to glide.lock, but this is pretty ugly and not maintainable in the long run.

@anannaya
Copy link

@Levovar : I am planing to write MutationWebhook configuration using some of the danm packages,
As mentioned in issue #65 ,I am able to geenerated those packages by following the command in the build.sh . Now i am trying to build my binaray getting this below error, Could you please help me to sort out this.

# github.com/nokia/danm/crd/apis/danm/v1 ../../../../../STUDIES/PROGRAMMING/Go_programs/go/src/github.com/nokia/danm/crd/apis/danm/v1/register.go:31:3: cannot use DanmEp literal (type *DanmEp) as type runtime.Object in argument to scheme.AddKnownTypes: *DanmEp does not implement runtime.Object (missing DeepCopyObject method) ../../../../../STUDIES/PROGRAMMING/Go_programs/go/src/github.com/nokia/danm/crd/apis/danm/v1/register.go:32:3: cannot use DanmEpList literal (type *DanmEpList) as type runtime.Object in argument to scheme.AddKnownTypes: *DanmEpList does not implement runtime.Object (missing DeepCopyObject method) ../../../../../STUDIES/PROGRAMMING/Go_programs/go/src/github.com/nokia/danm/crd/apis/danm/v1/register.go:33:3: cannot use DanmNet literal (type *DanmNet) as type runtime.Object in argument to scheme.AddKnownTypes: *DanmNet does not implement runtime.Object (missing DeepCopyObject method) ../../../../../STUDIES/PROGRAMMING/Go_programs/go/src/github.com/nokia/danm/crd/apis/danm/v1/register.go:34:3: cannot use DanmNetList literal (type *DanmNetList) as type runtime.Object in argument to scheme.AddKnownTypes: *DanmNetList does not implement runtime.Object (missing DeepCopyObject method

@Levovar
Copy link
Collaborator

Levovar commented Jan 7, 2020

solved by 73ef029

@Levovar Levovar closed this as completed Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants