improve locking strategy

Signed-off-by: Grant Linville <[email protected]>

Author: g-linville

pkg/credentials/store.go

@@ -218,33 +218,48 @@ func (s *Store) List(_ context.Context) ([]Credential, error) {
 }
 
 func (s *Store) RecreateAll(_ context.Context) error {
-	s.recreateAllLock.Lock()
-	defer s.recreateAllLock.Unlock()
-
 	store, err := s.getStore()
 	if err != nil {
 		return err
 	}
 
+	// We repeatedly lock and unlock the mutex in this function to give other threads a chance to talk to the credential store.
+	// It can take several minutes to recreate the credentials if there are hundreds of them, and we don't want to
+	// block all other threads while we do that.
+	// New credentials might be created after our GetAll, but they will be created with the current encryption configuration,
+	// so it's okay that they are skipped by this function.
+
+	s.recreateAllLock.Lock()
 	all, err := store.GetAll()
+	s.recreateAllLock.Unlock()
 	if err != nil {
 		return err
 	}
 
 	// Loop through and recreate each individual credential.
 	for serverAddress := range all {
+		s.recreateAllLock.Lock()
 		authConfig, err := store.Get(serverAddress)
 		if err != nil {
+			s.recreateAllLock.Unlock()
+
+			if IsCredentialsNotFoundError(err) {
+				// This can happen if the credential was deleted between the GetAll and the Get by another thread.
+				continue
+			}
 			return err
 		}
 
 		if err := store.Erase(serverAddress); err != nil {
+			s.recreateAllLock.Unlock()
 			return err
 		}
 
 		if err := store.Store(authConfig); err != nil {
+			s.recreateAllLock.Unlock()
 			return err
 		}
+		s.recreateAllLock.Unlock()
 	}
 
 	return nil