Skip to content

Commit 930c33b

Browse files
committed
Fix race condition in duplicate InstallPlan prevention
Move the listInstallPlans() call to after the muInstallPlan lock is acquired in ensureInstallPlan(). Previously, the check for existing install plans happened before acquiring the lock, which meant that worker 2 could check for install plans before worker 1 had a chance to create one, even with the lock in place. This fixes the race condition by ensuring that the sequence is: 1. Worker 1 acquires lock 2. Worker 1 checks for existing install plans 3. Worker 1 creates new install plan (if needed) 4. Worker 1 releases lock 5. Worker 2 acquires lock 6. Worker 2 checks for existing install plans (now sees worker 1's) 7. Worker 2 reuses existing install plan 8. Worker 2 releases lock Without this fix, both workers could see no existing install plans before either acquired the lock, leading to duplicate install plans being created for the same subscription.
1 parent 971d680 commit 930c33b

File tree

1 file changed

+8
-6
lines changed

1 file changed

+8
-6
lines changed

pkg/controller/operators/catalog/operator.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1654,12 +1654,6 @@ func (o *Operator) ensureInstallPlan(logger *logrus.Entry, namespace string, gen
16541654
return nil, nil
16551655
}
16561656

1657-
// Check if any existing installplans are creating the same resources
1658-
installPlans, err := o.listInstallPlans(namespace)
1659-
if err != nil {
1660-
return nil, err
1661-
}
1662-
16631657
// There are multiple(2) worker threads process the namespaceQueue.
16641658
// Both worker can work at the same time when 2 separate updates are made for the namespace.
16651659
// The following sequence causes 2 installplans are created for a subscription
@@ -1680,6 +1674,14 @@ func (o *Operator) ensureInstallPlan(logger *logrus.Entry, namespace string, gen
16801674
o.muInstallPlan.Lock()
16811675
defer o.muInstallPlan.Unlock()
16821676

1677+
// Check if any existing installplans are creating the same resources
1678+
// This must be done AFTER acquiring the lock to ensure worker 2 sees
1679+
// the installplan created by worker 1
1680+
installPlans, err := o.listInstallPlans(namespace)
1681+
if err != nil {
1682+
return nil, err
1683+
}
1684+
16831685
for _, installPlan := range installPlans {
16841686
if installPlan.Spec.Generation == gen {
16851687
return reference.GetReference(installPlan)

0 commit comments

Comments
 (0)