Update to client-go 10.0.0 and refactor dynamic client usage

[lblackstone]

• Update outdated references to dynamic client functions

Fixes #344
Fixes #281

[hausdorff]

I'm a bit confused on the use of Go modules -- is this required? I'd rather completely use either dep or Go modules if we can, using both is weird to me.

[lblackstone]

@hausdorff I was having a tough time getting it to build with dep, but it worked well with modules. I can keep trying with dep, but I think life would be easier if we start moving to modules.

[lukehoban]

I'm slightly nervous about switching just this one repo over to Go Modules with all the other Pulumi repos still on dep. Seems that consistency across the Pulumi repos will be valuable for folks contributing across them. For example, I believe using Go Modules here means this one repo will need to live outside of GOPATH? @ellismg thoughts?

@lblackstone Is it the case that this is now somehow impossible to make work with dep, or just that we haven't yet been able to get it to solve correctly with dep?

[lblackstone]

@lukehoban I spent several hours yesterday unsuccessfully trying to get this building with dep. The k8s client-go library is notoriously finicky due to the large number of dependencies. I could probably get it working with some more effort, but I would prefer to move to the use of Go modules across the board.

In general, I'd like to move away from building directly on the host to building in a container. Using Docker as the abstraction also allows us to change build tooling in the future without requiring users to care about the toolchain.

Note: You can still use modules within a GOPATH, but the GO111MODULE=on env var must be set (it's default for repos outside of the GOPATH)

[hausdorff]

I'm less concerned than @lukehoban is about using dep in some places and Go modules in others. I'm much more concerned with the fact that (1) Kubernetes itself has not transitioned, and (2) there do not seem to be any "serious" production systems using client-go via Go modules. I'm additionally skeptical (based on my experience with kubespy) that upgrading will be easier. Could be wrong -- but I'd personally want to be sure before we move.

If Go modules really work and dep really doesn't, it seems like we could just use those SHAs in the dep file and it should be solved incidentally?

[lblackstone]

@hausdorff Just chatted with Luke offline about this, and decided to try again with dep using the SHAs from the go.mod file. If I still can't get that working, we'll regroup and decide how heavily to invest in Go modules across the org at this time.

[lblackstone]

I was able to get the provider building again with dep, but now I'm troubleshooting regressions introduced by the upgrade.

[lblackstone]

I'm close to having this working now. The client currently isn't working right for resource kinds that don't have a namespace, but I know what the problem is, and am working on squashing bugs.

I'm also investigating a failure on

pulumi-kubernetes/tests/integration/get/step1/index.ts

Lines 41 to 45 in 69d6a81

	const ctObj = k8s.apiextensions.CustomResource.get("my-new-cron-object-get", {
	apiVersion: "stable.example.com/v1",
	kind: "CronTab",
	id: "my-new-cron-object"
	});

where the preview fails due to the CRD not being registered yet. I'm still unclear if this is a bug in the test, or if I inadvertently changed the semantics there.

[hausdorff]

@lblackstone I think I mentioned this offline, but just to follow up on the integration test before, we could put the .get in a step2/ folder -- no one would ever try to do something like that in real life, but it would give us code coverage for that path.

That said, I'm somewhat confused how this was passing before. :)

[lblackstone]

I squashed and rebased on master in the last force-push.

[lblackstone]

Rebased on master and resolved merge conflict.

[metral]

Thanks for all of the heavy lifting on this PR @lblackstone! The dynamic clientset looks cleaner, and not as repetitive as the disco + pool approach.

Overall LGTM 👍

[lblackstone]

So the discovery interface is pretty cool for introspecting on the k8s API, but it turns out to be really flaky when you're slamming it pretty hard (e.g., the istio test). I added a default look up table back for resource namespacing that will fall back to server-side discovery, and also added retry logic around discovery calls to make it more robust. I was able to spin the istio example up a couple times without error, so I think it's good to go now. 🤞

[lblackstone]

Fixed the istio test issues and seem to have introduced a regression on the Read, probably related to the oldInputs/currentInputs comments in the review. Working on that now.

[hausdorff]

Overall we're getting close. Just a couple more discussion points.

[lblackstone]

Last failure was the python flake. Addressed remaining feedback and this should hopefully be good to go

[lblackstone]

Think this is ready to merge now!

[hausdorff]

Ok, let's ship this. Just logistically, can we file bugs for these comments so we can track them:

• Update to client-go 10.0.0 and refactor dynamic client usage #348 (comment)
• Update to client-go 10.0.0 and refactor dynamic client usage #348 (comment)

One other note to set expectations about this change. This is quite invasive, and we expect a trail of regressions in the coming weeks as we figure out subtle incompatibilities between client-go v9 and v10. An example is that the error types returned under various circumstances seem to have changed, and it's not clear we have fully accounted for this yet. As a result, we've deliberately slated this change to come directly after a minor release, so that we have a long time to vet the changes.

For now, we think this is "good enough" to merge into master, as it gives the Pulumi team the opportunity to run the bits on our own infrastructure.