search

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

Fix provision check for empty OB returned #203

Closed BlaineEXE closed 3 years ago

BlaineEXE commented 3 years ago

See discussion on comment: https://github.com/kube-object-storage/lib-bucket-provisioner/pull/201#discussion_r546008824

BlaineEXE commented 3 years ago

We needed to do a similar thing in Rook and had to use an external library (github.com/google/go-cmp/cmp) to "diff" objects.

https://github.com/rook/rook/blob/ad24990473a876ba014b11ef61f8d63d46190ec0/pkg/operator/ceph/cluster/predicate.go#L124-L154

I would propose that if we want to check for equality to empty, this must look like

cmp.Diff(v1alpha1.ObjectBucket{}, *ob)
BlaineEXE commented 3 years ago

@travisn is there a reason we used cmp.Diff in Rook and not apiequality.Semantic.DeepEqual? https://stupefied-goodall-e282f7.netlify.app/contributors/devel/api_changes/#update-the-semantic-comparisons https://github.com/kubernetes/apimachinery/blob/0ca7b349afd234e3376e550db2392f41236be77b/pkg/api/equality/semantic.go#L27-L49

update: was it just so that we could get the diff reported in a nice output?

travisn commented 3 years ago

@leseb IIRC you implemented the diff check? I don't recall the details.

BlaineEXE commented 3 years ago

I don't think I did. @leseb do you recall any details about cmp.Diff vs apiequality.Semantic.DeepEqual?

leseb commented 3 years ago

@travisn you actually did add it long ago I remember since I requested it. We use it to get the CR diff changes when the CR is updated. We don't actually use it for equality. We might as well use DeepEqual unless the type cannot be "deep equaled".

travisn commented 3 years ago

Oh did I do that? :) I was thinking there was another change after mine, oh well. We really just need to know if there is any difference so we know to update the pod spec, then print the diff to the log.