kube-object-storage / lib-bucket-provisioner

Library for the dynamic provisioning of object store buckets to be used by object store providers.
Apache License 2.0
20 stars 22 forks source link

updating dependecies #185

Closed jackyalbo closed 4 years ago

jackyalbo commented 4 years ago

Update in order to update noobaa-operator to use operator-sdk v0.13.0 and controller-runtime v0.4.0 We update the kubernetes client apis 1.16.2

Following up PR #177

guymguym commented 4 years ago

@screeley44 Please review in order to maintain s3 provisioner.

@travisn Please review - note that we see that rook refers to the library to branch master without a specific commit so this might require to restrict the dependency to previous commit so that it won't break?

https://github.com/rook/rook/blob/97ce25ce03cf2bb7798841dfeacd4d917e9a810f/Gopkg.toml#L133-L136

guymguym commented 4 years ago

Another thing to consider:

operator-sdk 0.13.0 uses the following scaffold for the go.mod file of new operators in which the pinning of kubernetes libraries is in a replace section:

https://github.com/operator-framework/operator-sdk/blob/ba68281353e529aefadb0c86c74979dd2cf69f12/internal/scaffold/go_mod.go#L47-L70

In our case the library uses require and not replace. Using require means that provisioners must use the same exact kubernetes dependencies, while using replace means that provisioners can use other compatible kubernetes dependencies. See https://github.com/golang/go/wiki/Modules#when-should-i-use-the-replace-directive.

Should we switch to replace? @copejon @jeffvance @screeley44 @travisn

copejon commented 4 years ago

Building from @jackyalbo 's branch in rook/ceph and the aws-s3-provisioner results in breaks as a result of the new deps. I'm for making the change, but I think we need to decide now to version the library before making breaking changes.

copejon commented 4 years ago

Per a sidebar with @guymguym , I've set a tag on master:HEAD : kubernetes-v1.14.1. That'll be sufficient for now to distinguish the breaking changes without complicating managing the project.

travisn commented 4 years ago

This should be fine to merge from Rook's perspective. It shouldn't affect Rook until someone intentionally updates the dependencies. The build by default uses the specific hash from the Gopkg.lock rather than pulling the latest master as seen in the Gopkg.toml. But we certainly should move to using the tags so we don't unexpectedly hit issues when the dependencies are refreshed.

guymguym commented 4 years ago

Thanks Travis