Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

dep can't upgrade from client-go release-4.0 to release-5.0 #1265

Closed
davecheney opened this issue Oct 13, 2017 · 18 comments
Closed

dep can't upgrade from client-go release-4.0 to release-5.0 #1265

davecheney opened this issue Oct 13, 2017 · 18 comments

Comments

@davecheney
Copy link
Contributor

What version of dep are you using (dep version)?

v0.3.1-122-g98f625e

What dep command did you run?

The scenario is this.

  1. check out https://github.com/davecheney/dep-k8s-example
  2. dep ensure && go build
  3. Edit Gopkg.toml and apply this patch to upgrade from the release-4.0 branch to release-5.0. (The revision for apimachinery is taken from https://github.com/kubernetes/client-go/blob/release-5.0/Godeps/Godeps.json)
lucky(~/src/github.com/davecheney/dep-k8s-example) % git diff
diff --git a/Gopkg.toml b/Gopkg.toml
index 58ae22c..1b91f62 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -8,11 +8,11 @@

[[constraint]]
 name="k8s.io/client-go"
-  branch="release-4.0"
+  branch="release-5.0"

[[override]]
 name="k8s.io/apimachinery"
-  revision="917740426ad66ff818da4809990480bcc0786a77"
+  revision="9d38e20d609d27e00d4ec18f7b9db67105a2bde0"

[[override]]
 name="github.com/ugorji/go"
  1. Run dep ensure to fetch the new dependencies.

What did you expect to see?

I expected dep ensure to complete successfully.

What did you see instead?

lucky(~/src/github.com/davecheney/dep-k8s-example) % dep ensure                                                                                        
ensure Solve(): No versions of k8s.io/client-go met constraints:
        release-4.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v4.0.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v3.0.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v2.0.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v1.5.1: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v1.5.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v1.4.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v4.0.0-beta.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v3.0.0-beta.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v2.0.0-alpha.1: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        v2.0.0-alpha.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        master: Could not introduce k8s.io/client-go@master, as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-1.4: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-1.5: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-2.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-3.0: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.
        release-5.0: Could not introduce k8s.io/[email protected], as its subpackage k8s.io/client-go/pkg/api/v1 is missing. (Package is required by (root).)
        revert-14-1.5: Could not introduce k8s.io/[email protected], as it is not allowed by constraint release-5.0 from project github.com/davecheney/dep-k8s-example.

dep cannot update to the release-5.0 branch because it would fail to compile. Superficially that is helpful, but unless dep upgrades the client-go dependency, I cannot fix the build breakage from the major api bump.

How can I use dep to fetch release-5.0 so I can fix the build breakage?

@sdboyer
Copy link
Member

sdboyer commented Oct 13, 2017

man, you just keep on bumping into these fundamental questions 😜 gotta love k8s, drawing out all the warts.

the simple answer for what you can do right now would be that you add k8s.io/client-go/pkg/api/v1 to the ignored list in Gopkg.toml. i think that'll let you make it through - it just won't care that that package doesn't exist. you can then remove that ignored directive once you've eliminated all instances of that import. please do let me know if that doesn't work.

this is a tip of a bigger iceberg, though. these sort of additional checks that dep does give it the characteristics of a strongly mean-reverting system - one where wild fluctuations tend to be dampened out, back towards some (working) midpoint. this tends to be good the overwhelming majority of the time, but can put the user in an awkward position when they need to make large transitions, as you do here.

the biggest original problem in this class this was the "how do i add a new dependency?" problem chicken-or-egg problem, now addressed by dep ensure -add. but your issue here is a good example of that same sort of "how do i get from here to there?" gap. and, if we end up adding type analysis, that gap will become much more pronounced.

my plan has been that, if we ever get to doing type analysis, it would absolutely 💯 need to come with some sort of -version-checks-only flag, which can be included in order to tell the solver to disable anything other than the essential version satisfiability checks. i don't think it's strictly necessary now, for cases like this, as recourse exists with ignored. but eventually, such a flag would let users live in these "in between" places, both for this case, and the likely much more common case of creating some temporary type mismatches while making slightly larger changes.

@justinsb
Copy link

justinsb commented Oct 14, 2017

Does this imply that the versions determined by dep depend not just on Gopkg.toml, but on application code as well?

@sdboyer
Copy link
Member

sdboyer commented Oct 14, 2017 via email

@justinsb
Copy link

Can you explain what we're trying to do here? I guess that for dependencies that don't define their dependencies we have to do this to infer their dependencies somehow. But for a user application using dep, we should make them declare their direct dependencies.

I'd suggest we should also warn about dependencies that are not good go-citizens, and do not include a Gopkg.toml.

Taking that to extremes (if we're trying to "fill in the gaps" until all libraries include a Gopkg.toml): disable all additional analysis entirely by default (including for transitive dependencies); we expect all libraries to declare a Gopkg.toml file. Then we have a flag --legacy which must be passed to enable dependency inference, and it suggests filing a github issue against the legacy library. Eventually --legacy inference would be removed from dep, likely moved to a plugin.

@ianlewis
Copy link

As an aside I had similar issues recently with apimachinery and it turns out that having a copy of it in your GOPATH could also cause issues. Albeit in my case, it might have been a poorly or partially installed version. Once I cleared it out it worked.

I think yours is a separate problem but it just might change things with a clean GOPATH.

@sdboyer
Copy link
Member

sdboyer commented Oct 24, 2017

@justinsb even if we were to make the changes you're proposing, it would have little practical bearing on this issue - but making such changes is not on the table:

Can you explain what we're trying to do here? I guess that for dependencies that don't define their dependencies we have to do this to infer their dependencies somehow.

they do define their dependencies - that's what import statements in the source code are. asking the user to duplicate that information into Gopkg.toml is tedious and invites the possibility of conflicting information, so we don't do it. the package management committee settled on this last year.

consider just how much information we would have to record in e.g. k8s' Gopkg.toml if we wanted to make it possible for a user to import a single package within k8s, and get only the direct external dependencies of that package, and of all the other internal k8s packages transitively imported by that package. (we'd have to duplicate both the package structure and the import statements in their entirety)

But for a user application using dep, we should make them declare their direct dependencies.

we literally can't make anyone do anything. there is no central point of control, of publishing, that could be used to enforce such a rule - and intentionally excluding segments of the ecosystem is a non-starter.

I'd suggest we should also warn about dependencies that are not good go-citizens, and do not include a Gopkg.toml.

we're a long way off from this, if we ever do it. and we're unlikely to, as if your library relies solely on stdlib, there's no reason at all to use dep.

@ianlewis if accurate, this is very concerning - the only time that GOPATH should matter in any way to dep is if you dep init -gopath. outside of that, i can't even conceive of how cleaning out GOPATH/src could change dep's behavior.

@ianlewis
Copy link

ianlewis commented Oct 25, 2017

@sdboyer Yah, I though so too. Drove me bonkers trying to figure it out. I got errors like this:

$ mkdir -p $GOPATH/src/test
$ cd $GOPATH/src/test
$ curl -o main.go https://raw.githubusercontent.com/kubernetes/client-go/release-4.0/examples/out-of-cluster-client-configuration/main.go
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  2535  100  2535    0     0  14419      0 --:--:-- --:--:-- --:--:-- 14485

$ dep init
No versions of k8s.io/apimachinery met constraints:
	master: unable to update checked out version: : command failed: [git checkout 3b05bbfa0a45413bfa184edbf9af617e277962fb]: exit status 1
	release-1.6: unable to update checked out version: : command failed: [git checkout 35be0db31bd6e5635ef6366708dc2b137392f6a2]: exit status 1
	release-1.7: unable to update checked out version: : command failed: [git checkout 8ab5f3d8a330c2e9baaf84e39042db8d49034ae2]: exit status 1
	release-1.8: unable to update checked out version: : command failed: [git checkout 9d38e20d609d27e00d4ec18f7b9db67105a2bde0]: exit status 1

I'll see if I can find some time soon to repo it in a more meaningful way.

@sdboyer
Copy link
Member

sdboyer commented Oct 25, 2017

@ianlewis oooh ooh cache corruption cache corruption. could you try compiling dep from #1279 and seeing if that magically fixes it?

could you tar up $GOPATH/pkg/dep/sources/https---github.com-kubernetes-apimachinery and make it available so i can see the state of the repo? or, just running git status in that dir and seeing if anything seems odd/there's a dirty tree or untracked files, etc. do that, then #1279 😄

this is getting off-topic, though - let's please carry that discussion on in slack, or another issue. @davecheney, did the ignored approach work for you, at least for now?

@ncdc
Copy link

ncdc commented Oct 25, 2017

@sdboyer I've been trying the ignored approach too (for https://github.com/heptio/ark). I initially did what you suggested, and reran dep ensure -update k8s.io/client-go k8s.io/apimachinery k8s.io/kubernetes, which then listed another package that it had trouble with, so I kept adding to the ignored list until dep would run without complaining. But now I can't actually get it to pull in dependencies that are new to client-go v5.0.0. If I comment out the ignored list, it continues to complain about packages via transitive dependencies, e.g.:

v5.0.0: "k8s.io/client-go/discovery" transitively (through 1 packages) imports "k8s.io/client-go/pkg/api/v1", which contains malformed code: no package exists at "k8s.io/client-go/pkg/api/v1"

I don't have any references in the Ark repo to k8s.io/client-go/pkg/api* anywhere. Help?

@sdboyer
Copy link
Member

sdboyer commented Oct 25, 2017

@ncdc yeah, this story still needs improving 😢 it's not a common case, but it's definitely exacerbated by the complexity of k8s.

to be clear, though:

which then listed another package that it had trouble with,

these are packages from release-4.0 that no longer exist in release-5.0, right? those are the only ones i was imagining should go into ignored. if so, this:

But now I can't actually get it to pull in dependencies that are new to client-go v5.0.0.

confuses me...oh, wait! that'd just be because you don't actually import those yet. right.

OK, here's my suggestion for a virtuous loop that probably still won't be pleasant, but should at least get you through this with less hair-pulling:

  1. identify a package that has disappeared in the new version, and add it to the ignored list
  2. if that package has a corresponding package(s) in the new version, add those to the required list, including a comment about the old one it's replacing
  3. repeat 1-2 until solving goes through
  4. work through the list of each ignored package in your code, searching for instances of the import statement pointing at it and refactoring the corresponding file(s) to point to the new package(s), which you can handily reference b/c of the comments you made
  5. remove the entry from ignored, and corresponding required entries
  6. repeat 4-5 until you're totally converted

so, i'm imagining your WIP Gopkg.toml looks something like this:

ignored = [
  "something/we/actually/ignore/all/the/time",
# START REFACTORING LIST, ALL THESE ARE GONE WHEN WE'RE DONE
  "importpath/to/old/pkg1",
  "importpath/to/old/pkg2",
]

required = [
  "importpath/to/new/pkg1", # from "importpath/to/old/pkg1"
  "importpath/to/new/pkg2a", # from "importpath/to/old/pkg2"
  "importpath/to/new/pkg2b", # from "importpath/to/old/pkg2"
]

@ncdc
Copy link

ncdc commented Oct 25, 2017

these are packages from release-4.0 that no longer exist in release-5.0, right? those are the only ones i was imagining should go into ignored

Actually, no. Some of the packages listed are no longer in 5.0, but others are. Weird...

I'll give your suggestion a try and see how it goes. Will report back later. Thanks!

@ncdc
Copy link

ncdc commented Oct 25, 2017

@sdboyer something was messed up with my cache. Once I removed ~/go/pkg/dep/sources/https---github.com-kubernetes-client--go, I was able to get past my issues and have it download the new transitive dependencies. I'm still fixing compilation issues, but I'm optimistic I got past my main problem.

@sdboyer
Copy link
Member

sdboyer commented Oct 25, 2017

@ncdc uuuugh. hopefully #1279 will make the incidence of these cache issues much, much less frequent. but that's still us fighting blind in the dark, after the fact - i would love to figure out a way to better manage/report/something when these problems occur, rather than waiting for them to blow up. which is tricky, given that they almost entirely occur as a result of handling signals.

i want to merge that PR soon so that we can have people test it in the wild for a while before release. it's an adaptive failure mechanism, so the most we can really test it for is that it doesn't have unintended consequences on the happy path, or misbehaviors on certain known failure paths.

@ncdc
Copy link

ncdc commented Oct 25, 2017

@sdboyer in my case, I'm pretty sure the cached copy of client-go wasn't dirty. I'm not 100% positive, but I know I didn't cd in there and muck with it. It had the v5.0.0 tag checked out, but it most definitely did not match a manual clone & checkout of that tag. Very odd.

@sdboyer
Copy link
Member

sdboyer commented Oct 25, 2017

@ncdc any chance that upstream folks had force-pushed that tag at some point?

@ncdc
Copy link

ncdc commented Oct 25, 2017

@sdboyer can't say for sure, but we don't think so.

@ianlewis
Copy link

@sdboyer Sounds good. Will follow up.

@sdboyer
Copy link
Member

sdboyer commented Nov 14, 2017

i'm gonna close this one out, as we at least have an adequate workaround (even if not ideal)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants