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

Zookeeper should not include replace directives and use newer k8s deps #450

Closed
tmarcu opened this issue Mar 29, 2022 · 0 comments · Fixed by #449
Closed

Zookeeper should not include replace directives and use newer k8s deps #450

tmarcu opened this issue Mar 29, 2022 · 0 comments · Fixed by #449

Comments

@tmarcu
Copy link
Contributor

tmarcu commented Mar 29, 2022

Description

Go uses minimum version selection (MVS) when figuring out shared dependencies. Usually this does the right thing, but in cases where replace directives are used, the MVS algorithm doesn't work. For the most part, replace should be temporarily used, or only used in development.
A package that has a replace directive will override any deps from the imported package, and any replace from the import are ignored.
However, this creates an issue if the imported package uses a replace and doesn't define a valid require for it still. Then the importing package must also use the same replace directive in order to compile. This creates a situation where every package in the chain needs to be synced to the same replace imports, and becomes difficult to upgrade later on. Remove all replace blocks that aren't absolutely required to build zk (note sometimes this is unavoidable as some k8s/docker libs have broken go.mods with replace in them).

After this, upgrading k8s versions should become much simpler and less items should be pinned. Packages that import zk won't have to pin to the exact same version.

Importance

Blocks upgrading bookkeeper, pravega-operator, and others.

Location

Multiple places.

Suggestions for an improvement

Update code to not include any deprecated/removed libraries (already merged, e2e tests are now compatible with latest)
Remove all deps
re-run go mod tidy
Set any k8s deps to the specific version using:

go get -d -u -t k8s.io/[email protected]

do not use a replace block, this will still set it accurately in the go.mod:

require (
        // other deps
	k8s.io/api v0.23.1
	k8s.io/apimachinery v0.23.1
	k8s.io/client-go v0.23.1
	sigs.k8s.io/controller-runtime v0.11.1
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant