k8s-friendly dep

[sigma]

What this PR does / why we need it:

This is a followup on #61490

This PR introduces a vendored dep fork that's a superset of the original one, to make it friendlier to the Kubernetes use-cases.

It has the following main extra features (plus a few bugfixes that really should go upstream):

• handles the mono-repository structure (main repo + staging repositories) in a unified dependency graph
• handles "normal" repositories as well, falling back to regular dep logic (so that it could also be used as a drop-in replacement for test-infra, repo-infra, ...)
• removes more useless files (including some otherwise problematic test data)
• optionally generates a fake Godeps.json in the main repo and all staging repos for compatibility with existing scripts
• knows to ignore particular build tags and not pull corresponding dependencies
• preserve vendor/OWNERS

notes:

• for the sake of maintaining a decent transition path, a lot of overrides are introduced. This makes it easier to force-keep things in sync between godep and dep. If we were to merge something like this, followup tasks would then include removing those overrides, and replacing them with constraints where appropriate.
• an exception is the github.com/pelletier/go-toml package which has to move to version 1.1.0 to allow dep itself to build properly. Given that it's used only as part of the e2e tests usage of viper, it should be low-risk.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #61490

Special notes for your reviewer:

The whole dep vs vgo story is messy right now, and might require consideration. AFAICT vgo is nowhere near ready (some parts of it just don't exist at this point, even as a prototype) and dep development has slowed-down considerably since vgo's annoucement. So while I'm still looking at some PRs to integrate things upstream, I'm reluctant just waiting the whole thing out (in particular golang/dep#1764 doesn't seem to go anywhere for now).

Release note:

NONE

[sigma]

@thockin @jessfraz @mattfarina @sttts @fejta @cblecker
@kubernetes/sig-testing-proposals

[k8s-ci-robot]

@sigma: Reiterating the mentions to trigger a notification:
@kubernetes/sig-testing-proposals

In response to this:

@thockin @jessfraz @mattfarina @sttts @fejta @cblecker
@kubernetes/sig-testing-proposals

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[liggitt]

/assign @sttts

[sttts]

the Godeps.json files in the staging dirs have not changed? Do we still generated them?

[sttts]

We have to because the publishing-bot depends on them, and people vendoring with godep or glide depend on them.

[sigma]

I haven't handled that yet, I'll followup shortly with another dep patch to do that. I wasn't quite sure what was the exact requirement, and thought we might get away with generating the corresponding Gopkg.lock. But it seems you're saying we're also going to need valid Godeps.json files no matter what.

[sttts]

But it seems you're saying we're also going to need valid Godeps.json files no matter what.

I think until vgo is official and adopted wide enough, we will have to keep Godeps.json around, largest common denominator. It's sad, yes.

Potentially we can simulate a godep restore run based on gopkg.lock in the publisher bot, i.e. a run which populates the GOPATH.

[sigma]

I tried a few things, and settled for a feature in dep to generate all those files after a solution is found (at this point we have an exact knowledge of what packages are needed at what version for what k8s.io/* repo). I guess that would mean we could retire hack/update-staging-godeps-dockerized.sh ?

Incidentally it seems the currently generated Godeps.json can have duplicated entries, which is slightly weird.

[sttts]

duplicated entries

You mean multiple packages per repo? That's normal for Godep. And the bot makes use of that to avoid running godep safe+restore when nothing changed. So I fear we need that as well.

[sigma]

no I mean multiple entries for the same package. See the diff for staging/src/k8s.io/api/Godeps/Godeps.json for example. The removed entries are the ones that were previously duplicated.

[sttts]

That's indeed weird.

[sttts]

The generated Godeps.json looks fine now. The publishing bot should work with that.

[BenTheElder]

/ok-to-test

[sttts]

where does this file come from? Is it manually written?

Asking because this must be kept up-to-date. At least we need some hack/verify-* script to check that no constraint is missing

Do we really need the branch and version constraints here? Won't your dep fork figure out that the staging packages are in the same repo?

Background: on publishing we will drop this file (via https://github.com/kubernetes/publishing-bot/blob/master/configs/kubernetes-rules-configmap.yaml#L9) for now.

I tried to keep the file in kubernetes/publishing-bot#55 with full dependencies and hit a wall with the solver. Maybe we can do a minimal file like you have here. But it has to be generated by the bot automatically from the branch config.

[sigma]

Sorry, those are leftovers from previous iterations. I think at this point, those should mostly be empty and it's probably easier for all constraints to be expressed in the main Gopkg.toml anyway.
I guess I could implement some smarts that would replicate the constraints that make sense in each sub-project, but I'm not convinced that's needed for now.

I suppose I should consider generating the corresponding Gopkg.lock files though...

[sigma]

btw my experience is that it's easier to generate a Gopkg.toml using overrides rather than constraints when you know ahead of time what the desired solution is.
Of course, the result is not flexible, but since it'll be re-generated when needed anyway, it's probably no big deal.

[sttts]

rather than constraints when you know ahead of time what the desired solution is.

And overrides are ignored when they are in a library. That's the reason they are of no use in client-go. Also the lock files is ignored.

[sttts]

To help users with dep though, we could create a Gopkg.toml.template file. They can copy that into their projects, with a long list of overrides. Wdyt? Better than nothing.

[sigma]

Oh sorry I misunderstood the problem at hand.
Yeah I guess providing a template might help somewhat, although it just pushes the problem one step further, as you note.

[sigma]

btw I should add that it's entirely possible that the current "solution" expressed in Godeps.json is not an actual solution and that packages would conflict with each other according to their respective constraints (which overrides casually ignore).
I don't know if that's the case currently, but a few weeks back, there was a clear inconsistency for some microsoft-related packages (whether the corresponding constraints were faulty or too broad is another question :))

My point is, in general we shouldn't assume that a godep-snapshot is dep-consistent, so generating one from the other is a risky endeavor by nature.

[sttts]

Godep does no consistency checks. It only checks out what you declare in Godeps.json. So if kubernetes itself (all of it) builds and works with the given revisions, so will the staging repo.

If I understand you correctly, you are referring to constraints in dependencies' Gopkg.toml. The Godep.json solution might now matchs of the dependencies in this sense. This of course can happen.

Do I understand it correctly that the generated Godeps.jsons of all staging repos agree on the revisions? I.e. you don't try to find solutions for each staging dir alone, but one global one.

[sigma]

Right, yes all generated here Godeps.json are consistent, and use the versions from the unified Gopkg.lock

What I'm trying to express is that this Gopkg.lock solution is not necessarily a "real" one, in the sense that it's not the result of the resolution of proper constraints (from kubernetes itself or any of the vendored repository that happens to have a Gopkg.toml too). Instead, it's a forced replica (via overrides) of whatever the current godep solution is, at the time of the conversion.

As such, there can be no expectation (yet) that using any of the kubernetes repositories in the context of a proper dep-managed downstream project (e.g. vendoring client-go) should lead to dep finding a solution of said downstream project since there might not even be a dep-computable solution for the repository itself at this point. And that's also what could happen with your solver problem above (kubernetes/publishing-bot#55)

That's a problem that should gradually disappear after migration to dep, when we replace overrides with constraints, make the dependencies versions evolve, and hopefully converge towards an override-free manifest.
In short, as long as overrides are needed, downstream is screwed and needs to override too :( (just as it is today)

[BenTheElder]

/test pull-kubernetes-node-e2e
verify failures look to mostly be legitimate, though the typechecker may need some updates regarding staging workarounds.

[BenTheElder]

unrelated but: node-e2e is a widespread flake, I've pinged sig-node about looking into this.
/test pull-kubernetes-node-e2e

[sigma]

@BenTheElder thanks for the update. And I agree that the verify failures are legit, I'm looking into them (none are really unexpected at this point)

[cblecker]

@sigma You can't just remove the verify scripts. The verify that the vendor is up to date, and that the staging dependency definition files are up to date with the main file. They would need to be modified to support dep, but the step of verifying vendor is sane is still required.

[sigma]

@cblecker which is why it's a WIP and I added a TODO for myself :)
Just trying to figure out if I have a complete view of what's missing

[cblecker]

Fixing the rest is missing, not removing the test. The test job is supposed to fail until you actually implement a proper, working test.

[sigma]

/retest

[BenTheElder]

garbage collector flakes 👿
/retest

[sigma]

/retest

[rsc]

Please don't do this. I and the rest of the Go team would be happy to work with you to make Kubernetes work well with vgo (eventually just "go") instead of having K-specific tools on the side. If you fork dep you're just taking on more one-off technical load and maintenance instead of being able to take advantage of the rest of the ecosystem.

[sigma]

@rsc I'm personally more than happy to discuss and figure out (and even contribute to) a way forward with vgo. As mentioned in various places, I've been playing with its current incarnation in the context of k8s and experienced no shortage of valuable pain :).
That said, I don't think it's reasonable to (ask to) reject that particular transition before it is clear when vgo will be 1. real, and 2. a good fit for k8s. The godep-induced pain is real, and needs urgent fixing IMO. Waiting only makes the situation worse, and any transition more difficult.
Also, my view expressed above is that this particular transition would actually help a future move to vgo, by virtue of simply starting to care about semver at all. Why do you feel that's not the case?

[rsc]

@sigma I'll set up a time to talk, as we discussed. I just want to reply briefly to the point you raised here, to avoid the appearance of ignoring you.

• It seems to me that writing and maintaining a custom tool is a big deal. Dep is a very complex tool, and I would very much caution against forking and taking on maintenance of a fork.
• The vgo prototype has improved dramatically in the past month. I think it's very close to being able to support Kubernetes. If you want to put time into replacing the use of godep, I think it would make more sense to jump directly to vgo than via a custom dep fork.
• I understand that use of godep has been less than ideal for Kubernetes, but I also know that it's worked well enough for a few years. That would seem to imply to me that it can serve for the couple more months needed to get over to vgo.
• Dep has enough problems with its model that I don't even think I'd suggest moving to unmodified dep as a stepping stone at this point.

[k8s-ci-robot]

@sigma: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[thockin]

@sigma I am not aware of acute pain wrt godep today. The scripts we have wrapping it are slow, and occasionally generate weird deltas (e.g. one more or less character in the hashes) but they work reliably and they are not run THAT often.

@rsc I am glad to hear you even considering k8s' bizarre needs. Is there any documentation or anything to describe the principles/patterns vgo will adopt to handle the mess we have made (e.g. staging/... as a pseudo-vendor'ed thing)?

[sigma]

@thockin for the staging/ sub-repositories, my current understanding is that the strategy would look like:

• each sub-repo is a separate vgo module
• the main repo and each sub-repo would refer to other sub-repositories using the replace mechanism of vgo, to point to the local filesystem (that part fits pretty nicely in the vgo model)
• we would have to maintain the consistency of versions across main and sub-repositories by having vgo resolve dependencies at the top-level, and then use some script to bump each dependency of each sub-repository to the same version as the one at top-level (that part doesn't fit that well)

[sigma]

@thockin @sttts @cblecker so should we close this PR and move towards vgo integration?
There's quite a lot of work that needs to happen there before it can really be used, so keeping those parallel tracks seems wasteful.

[sttts]

so should we close this PR and move towards vgo integration?

I think vgo is the better approach long-term. @rsc's prototype looks pretty convincing. So, I vote for going the vgo route and accepting the godep pain for a bit longer.

[cblecker]

+1 to @sttts

[mattfarina]

We should head down the vgo/go modules route. Why switch twice when we can switch once? Heading that way will also enable us to provide feedback to the vgo team to make sure our use cases are being met.

[thockin]

Should we close this?

[sigma]

seems like we have general agreement, and I'm not pursuing this any further anyway.
If for some reason we need to reconsider down the road, I'll reopen.

/close