ceph: update object bucket provisioner library #6699
Conversation
BlaineEXE
added
object-bucket-claims
|
@acejam how comfortable do you feel building Rook containers locally and testing them in your environment? I believe this should fix #6650 for you though I will have to wait for kube-object-storage/lib-bucket-provisioner#198 to merge before Rook can be patched. If you don't feel comfortable, I can figure out how to get a Rook build onto Dockerhub for you somehow to test if you'd be willing. |
| @@ -74,6 +74,7 @@ func (p Provisioner) Provision(options *apibkt.BucketOptions) (*bktv1alpha1.Obje | |||
|
|
|||
| s3svc, err := cephObject.NewS3Agent(p.accessKeyID, p.secretAccessKey, p.getObjectStoreEndpoint(), true) | |||
| if err != nil { | |||
| p.deleteOBCResourceLogError("") | |||
There was a problem hiding this comment.
Can't we just defer instead of these multiple calls?
There was a problem hiding this comment.
Yes, but we only want to do this on error... I suppose we could do what the lib-bucket-provisioner code does and have an if err != nil block in the deferred function...
There was a problem hiding this comment.
Also given that some of the cleanup calls use p.bucketName I'd rather duplicate the lines and be more clear rather than have a complicated cleanup call. But let me know if you still think we should defer cleanup.
BlaineEXE
force-pushed
the
update-lib-bucket-provisioner
branch
from
2353040
to
6da08a6
Compare
|
I think I still want to update the object integration test to verify that the OBC makes it into the bound state and does not regress to perpetually "Pending" like before. |
BlaineEXE
force-pushed
the
update-lib-bucket-provisioner
branch
from
88c5fe2
to
83f977c
Compare
|
There seems to be a race condition that is getting caught by the integration tests but not my local system. From what I can tell, the OBC should be getting re-resolved after I'm adding extra logging temporarily to the object bucket provisioner library to determine when a new request is being queued and what the OBC looks like when work on it starts. Update: This was a result of forgetting to put the |
BlaineEXE
force-pushed
the
update-lib-bucket-provisioner
branch
from
573ccd4
to
90b89a3
Compare
The library for object bucket provisioning is updated to fix errors during provisioning. Signed-off-by: Blaine Gardner <blaine.gardner@redhat.com>
BlaineEXE
force-pushed
the
update-lib-bucket-provisioner
branch
from
90b89a3
to
f1f89fd
Compare
| // resource being modified to refer to its OB. | ||
| timeToWaitForReversion := 2 * time.Minute | ||
| timeToCheckForReversion := timeBucketCreateVerified.Add(timeToWaitForReversion) | ||
| for time.Now().Before(timeToCheckForReversion) { |
There was a problem hiding this comment.
Is this for loop the same as a single 2 minute sleep? Why not just do this?
time.Sleep(2 * time.Minute)
Actually, we really should not have a two minute sleep in the test. That's pretty expensive when the whole suite is less than 15 minutes. I would suggest we not test for this revert bug in the integration tests.
There was a problem hiding this comment.
I didn't want to just blindly wait 2 minutes, so I check every 5 seconds if it's been 2 minutes since the OBC was created timeBucketCreateVerified.
There was a problem hiding this comment.
The best thing to do might be to make the object and bucket its own test outside of the smoke suite. Maybe a separate PR? I really think we should do a better job of adding regression tests into our integration.
There was a problem hiding this comment.
This is where we really do need a "daily" test suite that will run regression tests that may take a lot longer to run. For development we really need to keep the test suites shorter though.
Ideally we would add a test to prevent every regression that we find. In reality, it's frequently just not feasible to do that, so we have to strike a balance based on the cost of creating the test.
There was a problem hiding this comment.
I'm open to adding this regression in a separate PR with the ability to enable it only with a REGRESSION_TEST env var or something, but I really don't want to just drop the responsibility of writing regression tests. Maybe I can work with Seb to get another GH action to run regressions daily and on release builds?
There was a problem hiding this comment.
Yes, a separate PR sounds good for the tests so we can get this fix into the release.
BlaineEXE
force-pushed
the
update-lib-bucket-provisioner
branch
2 times, most recently
from
89ee2cd
to
3b4ee6c
Compare
ceph: update object bucket provisioner library (bp #6699)
The library for object bucket provisioning is updated to fix errors
during provisioning.
Signed-off-by: Blaine Gardner blaine.gardner@redhat.com
Description of your changes:
Which issue is resolved by this Pull Request:
Resolves #6650
Checklist:
make codegen) has been run to update object specifications, if necessary.