cluster-api-provider-openstack icon indicating copy to clipboard operation
cluster-api-provider-openstack copied to clipboard

Dependency problem

Open cwrau opened this issue 1 year ago • 13 comments

/kind bug

What steps did you take and what happened:

We are using CAPO as a go dependency to build our own operator on top of it (creating projects, users, ...). After updating from 0.7.3 to 0.8.0, go fails with:

go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid

What did you expect to happen:

The version bump would just work.

Anything else you would like to add:

We fixed it on our end by adding replaces for all of these to version 1.27.2

Environment:

  • Cluster API Provider OpenStack version (Or git rev-parse HEAD if manually built): 0.8.0
  • Cluster-API version: 1.5.1
  • OpenStack version: N/A
  • Minikube/KIND version: N/A
  • Kubernetes version (use kubectl version): 1.27.2
  • OS (e.g. from /etc/os-release): N/A

cwrau avatar Feb 19 '24 10:02 cwrau

I suspect you may be running into a diamond dependencies problem. CAPO 0.8 imports k8s 1.28. Are you importing some other library which requires 1.27, or are you directly importing 1.27. If the former, you could try to find a newer version of it which uses 1.28. If the latter you could bump your own deps to use 1.28.

We're unlikely to be able to help you in 0.8, but I'm interested in your use case because we might be able to improve it in future releases. Are you only using /api, or are you also using other packages? I've long been aware that we have vastly too much code in /api and therefore a bunch of unnecessary dependencies there. I'd be interested in exploring ways to clean that up to improve use cases like yours. I think we're always going to end up with some k8s deps, though, because we use k8s types. I'd be interested to know how other libraries with k8s deps have addressed this, and if we could do the same.

mdbooth avatar Feb 19 '24 13:02 mdbooth

I suspect you may be running into a diamond dependencies problem. CAPO 0.8 imports k8s 1.28

Really? go mod graph shows sigs.k8s.io/[email protected] k8s.io/[email protected], I thought this meant you're using 1.27.2? And https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/v0.8.0/go.mod also says 1.27.2

Are you importing some other library which requires 1.27, or are you directly importing 1.27. If the former, you could try to find a newer version of it which uses 1.28. If the latter you could bump your own deps to use 1.28.

Not as far as I can tell, this is in our go.mod;

require (
	github.com/blang/semver v3.5.1+incompatible
	github.com/go-logr/logr v1.4.1
	github.com/gophercloud/gophercloud v1.7.0
	github.com/gophercloud/utils v0.0.0-20231010081019-80377eca5d56
	github.com/onsi/ginkgo/v2 v2.14.0
	github.com/onsi/gomega v1.30.0
	github.com/pkg/errors v0.9.1
	github.com/prometheus/client_golang v1.18.0
	github.com/spf13/pflag v1.0.5
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.48.0
	go.opentelemetry.io/otel v1.23.1
	go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.23.1
	go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.23.0
	go.opentelemetry.io/otel/sdk v1.23.1
	go.opentelemetry.io/otel/trace v1.23.1
	gopkg.in/yaml.v2 v2.4.0
	k8s.io/api v0.29.0
	k8s.io/apiextensions-apiserver v0.29.0
	k8s.io/apimachinery v0.29.0
	k8s.io/client-go v0.29.0
	k8s.io/component-base v0.29.0
	k8s.io/klog/v2 v2.110.1
	k8s.io/utils v0.0.0-20230726121419-3b25d923346b
	sigs.k8s.io/cluster-api v1.5.1
	sigs.k8s.io/cluster-api-provider-openstack v0.8.0
	sigs.k8s.io/controller-runtime v0.17.2
)

And if I do for dep in k8s.io/cloud-provider k8s.io/controller-manager k8s.io/cri-api k8s.io/csi-translation-lib k8s.io/dynamic-resource-allocation k8s.io/kube-aggregator k8s.io/kube-controller-manager k8s.io/kube-proxy k8s.io/kube-scheduler k8s.io/kubelet k8s.io/legacy-cloud-providers k8s.io/mount-utils k8s.io/pod-security-admission k8s.io/sample-apiserver; do echo ---; echo $dep; go mod why $dep; done it returns main module does not need package k8s.io/sample-apiserver for all of them

I'm interested in your use case because we might be able to improve it in future releases. Are you only using /api, or are you also using other packages?

Yeah, just using /api

cwrau avatar Feb 19 '24 13:02 cwrau

Ok, now I got another (related?) problem 😅

When I clone this repo, do nothing and just go list -json -m -mod=readonly all I get similar errors;

go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
Reproduce
podman-run golang:1.22
root@aedf476f9310:/go# cd
root@aedf476f9310:~# git clone https://github.com/kubernetes-sigs/cluster-api-provider-openstack
Cloning into 'cluster-api-provider-openstack'...
remote: Enumerating objects: 38363, done.
remote: Counting objects: 100% (8862/8862), done.
remote: Compressing objects: 100% (2416/2416), done.
Receiving objects:   2% (768/38363)4/38363)
remote: Total 38363 (delta 7290), reused 6506 (delta 6441), pack-reused 29501
Receiving objects: 100% (38363/38363), 47.08 MiB | 14.68 MiB/s, done.
Resolving deltas: 100% (18547/18547), done.
root@aedf476f9310:~# cd cluster-api-provider-openstack/
root@aedf476f9310:~/cluster-api-provider-openstack# go list -json -m -mod=readonly all
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0
go: k8s.io/[email protected]: invalid version: unknown revision v0.0.0

cwrau avatar Feb 21 '24 10:02 cwrau

I suspect you may be running into a diamond dependencies problem. CAPO 0.8 imports k8s 1.28

Really? go mod graph shows sigs.k8s.io/[email protected] k8s.io/[email protected], I thought this meant you're using 1.27.2? And https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/v0.8.0/go.mod also says 1.27.2

My local checkout of release-0.8 was messed up 🤦 That explains some other things...

You're quite right, it was 1.27.2.

mdbooth avatar Feb 21 '24 11:02 mdbooth

Honestly I don't know. If you get to the bottom of it and you think we could improve anything in the repo, please let us know (PRs would be even better!).

For a while I have wanted to remove all code from /api to make it easier to import. It would still inevitably have some k8s deps, but I think we could theoretically minimise it to core types. As it is it pulls in all sorts, e.g. controller-runtime, and from there the world. I'm encouraged that somebody has finally asked for it! Unfortunately I don't think I'll have time to work on it any time soon, though. Again, if this is something you think you could do, I would do my best to help those PRs land.

mdbooth avatar Feb 21 '24 11:02 mdbooth

Honestly I don't know. If you get to the bottom of it and you think we could improve anything in the repo, please let us know (PRs would be even better!).

For a while I have wanted to remove all code from /api to make it easier to import. It would still inevitably have some k8s deps, but I think we could theoretically minimise it to core types. As it is it pulls in all sorts, e.g. controller-runtime, and from there the world. I'm encouraged that somebody has finally asked for it! Unfortunately I don't think I'll have time to work on it any time soon, though. Again, if this is something you think you could do, I would do my best to help those PRs land.

I don't know if this is something specific to using this as a dependency, as I can't even work with the project itself, see https://github.com/kubernetes-sigs/cluster-api-provider-openstack/issues/1891#issuecomment-1956398726

Maybe if that gets resolved, this will just work as well?

cwrau avatar Feb 21 '24 11:02 cwrau

Honestly I don't know. If you get to the bottom of it and you think we could improve anything in the repo, please let us know (PRs would be even better!). For a while I have wanted to remove all code from /api to make it easier to import. It would still inevitably have some k8s deps, but I think we could theoretically minimise it to core types. As it is it pulls in all sorts, e.g. controller-runtime, and from there the world. I'm encouraged that somebody has finally asked for it! Unfortunately I don't think I'll have time to work on it any time soon, though. Again, if this is something you think you could do, I would do my best to help those PRs land.

I don't know if this is something specific to using this as a dependency, as I can't even work with the project itself, see #1891 (comment)

Maybe if that gets resolved, this will just work as well?

Is that actually a problem, though? It builds, so presumably everything required is resolved.

In other news, many aspects of go mod are just voodoo to me.

mdbooth avatar Feb 21 '24 11:02 mdbooth

Honestly I don't know. If you get to the bottom of it and you think we could improve anything in the repo, please let us know (PRs would be even better!). For a while I have wanted to remove all code from /api to make it easier to import. It would still inevitably have some k8s deps, but I think we could theoretically minimise it to core types. As it is it pulls in all sorts, e.g. controller-runtime, and from there the world. I'm encouraged that somebody has finally asked for it! Unfortunately I don't think I'll have time to work on it any time soon, though. Again, if this is something you think you could do, I would do my best to help those PRs land.

I don't know if this is something specific to using this as a dependency, as I can't even work with the project itself, see #1891 (comment) Maybe if that gets resolved, this will just work as well?

Is that actually a problem, though? It builds, so presumably everything required is resolved.

I would say so, I can't even open IntelliJ without running into this similar problem, sounds related to me. It builds locally for me as well, but all other projects, even cluster-api itself, open without problems.

So something is not quite right

In other news, many aspects of go mod are just voodoo to me.

Samesies

cwrau avatar Feb 21 '24 11:02 cwrau

I think the issue may be this https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/82b31226bd8f1058da790f14d496ad9db8f51377/go.mod#L28 Or perhaps rather that we do not have a separate go.mod for some things. That I think we should have. If I compare with CAPI, they have separate go.mod for test and hack/tools. We should probably have one for the test folder at least

lentzi90 avatar Feb 21 '24 11:02 lentzi90

I think the issue may be this

https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/82b31226bd8f1058da790f14d496ad9db8f51377/go.mod#L28

I think we get that from CAPI: https://github.com/kubernetes-sigs/cluster-api/blob/0045baf02ce542fad0f96e42ab2decc41822256e/go.mod#L48

Or perhaps rather that we do not have a separate go.mod for some things. That I think we should have. If I compare with CAPI, they have separate go.mod for test and hack/tools. We should probably have one for the test folder at least

I'm not sure the separate test module would be worth quite so much to us. Unlike CAPI we don't expect anybody to import our tests. A separate module for /api might make sense, though.

mdbooth avatar Feb 21 '24 11:02 mdbooth

Please see kubernetes/cloud-provider-openstack#2490 for a discussion about this. https://github.com/kayrus/cloud-provider-openstack/commit/c128d6909366a2b195e7428da940405f0c14f024 proposes an approach to solve this.

dulek avatar Feb 28 '24 17:02 dulek

Thanks for the reference @dulek ! Based on that discussion I think our issue is https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/7b202d18cc228b7e2fdbaafba3f8674223bcbb73/go.mod#L27 which is used in exactly one place: https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/7b202d18cc228b7e2fdbaafba3f8674223bcbb73/pkg/utils/conversion/restore.go#L27 https://github.com/kubernetes-sigs/cluster-api-provider-openstack/blob/7b202d18cc228b7e2fdbaafba3f8674223bcbb73/pkg/utils/conversion/restore.go#L126

Since k8s.io/kubernetes is not meant to be used as a dependency I wonder if we could find another way to hash the objects?

lentzi90 avatar Feb 29 '24 06:02 lentzi90

The method isn't exactly complicated, IMO we should just copy it: https://github.com/kubernetes/kubernetes/blob/v1.29.2/pkg/util/hash/hash.go#L26-L32

dulek avatar Feb 29 '24 10:02 dulek