search

observatorium / thanos-receive-controller

Kubernetes controller to automatically configure Thanos receive hashrings
Apache License 2.0
93 stars 42 forks source link

fix downstream build issue #61

Closed clyang82 closed 3 years ago

clyang82 commented 3 years ago

Fix this issue in downstream build:

The command "go mod download" failed with: go: k8s.io/client-go@v12.0.0+incompatible: reading http://cachito-athens/k8s.io/client-go/@v/v12.0.0+incompatible.mod: 404 Not Found
clyang82 commented 3 years ago

/assign @kakkoyun

squat commented 3 years ago

This seems like a downstream problem with the go module cache bring used by CI/CD. It would be better to fix there than push it upstream. If it absolutely needs to be here, then why use "replace" at all? Client go is not a transitive dependency so we could directly select the correct version to install, no?

clyang82 commented 3 years ago

You know that the downstream just have a dockerfile and reuse the upstream code. so it is hard to fix in downstream side. It seems there has dependency so cannot directly select the correct version. that is why use "replace" here. Actually, we did it in the past if you check the history of go.mod. Thanks.

squat commented 3 years ago

Regarding the usage of replace vs simple version change: 👍

Regarding the upstream code change: I understand that downstream just uses the dockerfile, but it's not a problem with the dependency or the code here; it's a problem with the downstream build environment, specifically the build environment's go module cache. We should ideally fix this there.

clyang82 commented 3 years ago

@arewm Is it possible to fix the go module cache issue in downstream side? or is it a limitation of cachito? Thanks.

arewm commented 3 years ago

It is not possible to fix this downstream since Cachito is pointed to a git url/commit upstream and then operates on the files that it finds there without our interaction. While there is theoretically a way that we can do this, it is not currently exposed.

@clyang82, the only other alternative if this PR does not get accepted is to fork the repo so that we can make the change ourselves (at least for the short term).

squat commented 3 years ago

This is too bad, but I think we can merge this in the short term. cc @observatorium/maintainers

bwplotka commented 3 years ago

Especially given no quick workaround (:

clyang82 commented 3 years ago

Thanks all for your help. Great teamwork.