Merge pull request juju#17052 from SimonRichardson/objectstore-failure

juju#17052

Fixes intermittent failure within the objectstore S3 tests. The changes move the internal state call until we've started up the object store. Otherwise, there is a race between the test code and the implementation around what gets called or not.

<!-- Why this change is needed and what it does. -->

## Checklist

- [x] Code style: imports ordered, good names, simple structure, etc

## QA steps

```sh
$ go test -v ./internal/objectstore -check.v -count=10
```

internal/objectstore/base_test.go

@@ -78,7 +78,10 @@ func (s *baseObjectStoreSuite) TestLocking(c *gc.C) {
 
 	s.expectClaimRelease("hash")
 	s.expectExtendDuration(time.Second)
-	s.expectClockAfter(make(chan time.Time))
+
+	// The clock after might not be called, as we might schedule the goroutine
+	// fast enough, that the second goroutine isn't called.
+	s.maybeExpectClockAfter(make(chan time.Time))
 
 	w := &baseObjectStore{
 		claimer: s.claimer,
@@ -335,6 +338,12 @@ func (s *baseObjectStoreSuite) setupMocks(c *gc.C) *gomock.Controller {
 	return ctrl
 }
 
+func (s *baseObjectStoreSuite) maybeExpectClockAfter(ch chan time.Time) {
+	s.clock.EXPECT().After(gomock.Any()).DoAndReturn(func(dur time.Duration) <-chan time.Time {
+		return ch
+	}).AnyTimes()
+}
+
 func (s *baseObjectStoreSuite) expectClockAfter(ch chan time.Time) {
 	s.clock.EXPECT().After(gomock.Any()).DoAndReturn(func(dur time.Duration) <-chan time.Time {
 		return ch

internal/objectstore/s3objectstore.go

@@ -274,9 +274,6 @@ func (t *s3ObjectStore) Remove(ctx context.Context, path string) error {
 }
 
 func (t *s3ObjectStore) loop() error {
-	// Report the initial started state.
-	t.reportInternalState(stateStarted)
-
 	// Ensure the namespace directory exists, along with the tmp directory.
 	if err := t.ensureDirectories(); err != nil {
 		return errors.Annotatef(err, "ensuring file store directories exist")
@@ -327,6 +324,9 @@ func (t *s3ObjectStore) loop() error {
 		fileFallback = useFileAccessor
 	}
 
+	// Report the initial started state.
+	t.reportInternalState(stateStarted)
+
 	// Sequence the get request with the put, remove requests.
 	for {
 		select {