Skip to content

Commit 088d610

Browse files
nicholas-fedornicholas-fedor
committed
fix(actions): use detached context for container restart operations
- Use detached context in restartStaleContainer to survive parent cancellation - Track container creation separately from start operations in tests - Rename StartOrder to CreateOrder in mock client for accurate tracking
1 parent a8d1298 commit 088d610

6 files changed

Lines changed: 50 additions & 42 deletions

File tree

‎internal/actions/actions_internal_test.go‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2436,8 +2436,9 @@ var _ = ginkgo.Describe("performRollingRestart", func() {
24362436
"container-1": true,
24372437
"container-2": true,
24382438
},
2439-
StopOrder: []string{},
2440-
StartOrder: []string{},
2439+
StopOrder: []string{},
2440+
CreateOrder: []string{},
2441+
StartOrder: []string{},
24412442
},
24422443
false,
24432444
false,
@@ -2455,11 +2456,11 @@ var _ = ginkgo.Describe("performRollingRestart", func() {
24552456
nil,
24562457
)
24572458

2458-
// Verify start order is forward (container-0, container-1, container-2).
2459-
gomega.Expect(client.TestData.StartOrder).To(gomega.HaveLen(3))
2460-
gomega.Expect(client.TestData.StartOrder[0]).To(gomega.Equal("container-0"))
2461-
gomega.Expect(client.TestData.StartOrder[1]).To(gomega.Equal("container-1"))
2462-
gomega.Expect(client.TestData.StartOrder[2]).To(gomega.Equal("container-2"))
2459+
// Verify create order is forward (container-0, container-1, container-2).
2460+
gomega.Expect(client.TestData.CreateOrder).To(gomega.HaveLen(3))
2461+
gomega.Expect(client.TestData.CreateOrder[0]).To(gomega.Equal("container-0"))
2462+
gomega.Expect(client.TestData.CreateOrder[1]).To(gomega.Equal("container-1"))
2463+
gomega.Expect(client.TestData.CreateOrder[2]).To(gomega.Equal("container-2"))
24632464
})
24642465
})
24652466

‎internal/actions/actions_test.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -261,10 +261,10 @@ var _ = ginkgo.Describe("Actions", func() {
261261

262262
gomega.Expect(metric).NotTo(gomega.BeNil())
263263
// Only container1 should be updated, container2 should be skipped due to CurrentContainerID
264-
gomega.Expect(client.TestData.StartOrder).To(gomega.HaveLen(1))
265-
gomega.Expect(client.TestData.StartOrder).
264+
gomega.Expect(client.TestData.CreateOrder).To(gomega.HaveLen(1))
265+
gomega.Expect(client.TestData.CreateOrder).
266266
To(gomega.ContainElement("watchtower-1"))
267-
gomega.Expect(client.TestData.StartOrder).
267+
gomega.Expect(client.TestData.CreateOrder).
268268
To(gomega.Not(gomega.ContainElement("watchtower-2")))
269269
},
270270
)
@@ -1163,16 +1163,16 @@ var _ = ginkgo.Describe("Actions", func() {
11631163
wg.Wait()
11641164

11651165
// Verify scope isolation through separate TestData tracking
1166-
// client1 should only start scope1-watchtower
1167-
gomega.Expect(client1.TestData.StartOrder).
1166+
// client1 should only create scope1-watchtower
1167+
gomega.Expect(client1.TestData.CreateOrder).
11681168
To(gomega.ContainElement("scope1-watchtower"))
1169-
gomega.Expect(client1.TestData.StartOrder).
1169+
gomega.Expect(client1.TestData.CreateOrder).
11701170
NotTo(gomega.ContainElement("scope2-watchtower"))
11711171

1172-
// client2 should only start scope2-watchtower
1173-
gomega.Expect(client2.TestData.StartOrder).
1172+
// client2 should only create scope2-watchtower
1173+
gomega.Expect(client2.TestData.CreateOrder).
11741174
To(gomega.ContainElement("scope2-watchtower"))
1175-
gomega.Expect(client2.TestData.StartOrder).
1175+
gomega.Expect(client2.TestData.CreateOrder).
11761176
NotTo(gomega.ContainElement("scope1-watchtower"))
11771177
},
11781178
)

‎internal/actions/mocks/client.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ type TestData struct {
5252
RemoveImageError error // Error to return from RemoveImageByID (for testing).
5353
FailedImageIDs []types.ImageID // List of image IDs that should fail removal.
5454
StopOrder []string // Order in which containers were stopped.
55+
CreateOrder []string // Order in which containers were created.
5556
StartOrder []string // Order in which containers were started.
5657
SimulatedLatency time.Duration // Simulated latency for operations (default 0 for fast tests, set for context cancellation tests).
5758
LastContainerChain string // Last container chain passed to CreateEphemeralOrchestrator.
@@ -213,7 +214,7 @@ func (client MockClient) CreateContainer(ctx context.Context, c types.Container)
213214
return "", client.TestData.CreateContainerError
214215
}
215216

216-
client.TestData.StartOrder = append(client.TestData.StartOrder, c.Name())
217+
client.TestData.CreateOrder = append(client.TestData.CreateOrder, c.Name())
217218

218219
return c.ID(), nil
219220
}

‎internal/actions/update.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,7 +1775,8 @@ func restartStaleContainer(
17751775
}
17761776

17771777
// Create the new container with updated configuration.
1778-
newContainerID, err := client.CreateContainer(ctx, sourceContainer)
1778+
//nolint:contextcheck // Using detached context intentionally to survive parent cancellation
1779+
newContainerID, err := client.CreateContainer(detachedCtx, sourceContainer)
17791780
if err != nil {
17801781
logrus.WithFields(fields).
17811782
WithError(err).
@@ -1796,7 +1797,8 @@ func restartStaleContainer(
17961797
logrus.WithFields(fields).
17971798
Debug("Starting container with updated configuration")
17981799

1799-
err = client.StartContainerByID(ctx, newContainerID)
1800+
//nolint:contextcheck // Using detached context intentionally to survive parent cancellation
1801+
err = client.StartContainerByID(detachedCtx, newContainerID)
18001802
if err != nil {
18011803
logrus.WithFields(fields).
18021804
WithError(err).

‎internal/actions/update_dependencies_test.go‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -415,8 +415,9 @@ var _ = ginkgo.Describe("the update action", func() {
415415
"container-b": true,
416416
"container-c": true,
417417
},
418-
StopOrder: []string{},
419-
StartOrder: []string{},
418+
StopOrder: []string{},
419+
CreateOrder: []string{},
420+
StartOrder: []string{},
420421
},
421422
false,
422423
false,
@@ -432,8 +433,8 @@ var _ = ginkgo.Describe("the update action", func() {
432433
// Verify stop order: dependents first (reverse dependency order)
433434
gomega.Expect(client.TestData.StopOrder).
434435
To(gomega.Equal([]string{"container-a", "container-b", "container-c"}))
435-
// Verify start order: dependencies first
436-
gomega.Expect(client.TestData.StartOrder).
436+
// Verify create order: dependencies first
437+
gomega.Expect(client.TestData.CreateOrder).
437438
To(gomega.Equal([]string{"container-c", "container-b", "container-a"}))
438439
})
439440
})
@@ -1297,8 +1298,9 @@ var _ = ginkgo.Describe("the update action", func() {
12971298
"app-2": false,
12981299
"app-3": false,
12991300
},
1300-
StopOrder: []string{},
1301-
StartOrder: []string{},
1301+
StopOrder: []string{},
1302+
CreateOrder: []string{},
1303+
StartOrder: []string{},
13021304
},
13031305
false,
13041306
false,
@@ -1330,13 +1332,13 @@ var _ = ginkgo.Describe("the update action", func() {
13301332
// db should be last in stop order
13311333
gomega.Expect(stopOrder[len(stopOrder)-1]).To(gomega.Equal("db"))
13321334

1333-
// Verify start order: db first, then dependents
1334-
startOrder := client.TestData.StartOrder
1335-
gomega.Expect(startOrder).To(gomega.HaveLen(4))
1336-
gomega.Expect(startOrder[0]).To(gomega.Equal("db"))
1337-
gomega.Expect(startOrder).To(gomega.ContainElement("app-1"))
1338-
gomega.Expect(startOrder).To(gomega.ContainElement("app-2"))
1339-
gomega.Expect(startOrder).To(gomega.ContainElement("app-3"))
1335+
// Verify create order: db first, then dependents
1336+
createOrder := client.TestData.CreateOrder
1337+
gomega.Expect(createOrder).To(gomega.HaveLen(4))
1338+
gomega.Expect(createOrder[0]).To(gomega.Equal("db"))
1339+
gomega.Expect(createOrder).To(gomega.ContainElement("app-1"))
1340+
gomega.Expect(createOrder).To(gomega.ContainElement("app-2"))
1341+
gomega.Expect(createOrder).To(gomega.ContainElement("app-3"))
13401342
})
13411343
})
13421344

@@ -1743,8 +1745,9 @@ var _ = ginkgo.Describe("the update action", func() {
17431745
"c-service2": true,
17441746
"d-service3": true,
17451747
},
1746-
StopOrder: []string{},
1747-
StartOrder: []string{},
1748+
StopOrder: []string{},
1749+
CreateOrder: []string{},
1750+
StartOrder: []string{},
17481751
},
17491752
false,
17501753
false,
@@ -1765,8 +1768,8 @@ var _ = ginkgo.Describe("the update action", func() {
17651768
gomega.Expect(client.TestData.StopOrder).
17661769
To(gomega.Equal([]string{"c-service2", "b-service1", "d-service3"}))
17671770

1768-
// Verify start order: dependency order
1769-
gomega.Expect(client.TestData.StartOrder).
1771+
// Verify create order: dependency order
1772+
gomega.Expect(client.TestData.CreateOrder).
17701773
To(gomega.Equal([]string{"d-service3", "b-service1", "c-service2"}))
17711774

17721775
// Verify cleanup for updated containers

‎internal/actions/update_restart_test.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,8 +1299,9 @@ var _ = ginkgo.Describe("the update action", func() {
12991299
"priority-b": false,
13001300
"priority-a": false,
13011301
},
1302-
StopOrder: []string{},
1303-
StartOrder: []string{},
1302+
StopOrder: []string{},
1303+
CreateOrder: []string{},
1304+
StartOrder: []string{},
13041305
},
13051306
false,
13061307
false,
@@ -1321,10 +1322,10 @@ var _ = ginkgo.Describe("the update action", func() {
13211322
gomega.Expect(report.Updated()).To(gomega.HaveLen(1))
13221323
gomega.Expect(report.Restarted()).To(gomega.HaveLen(2))
13231324

1324-
// Verify start order respects dependencies (dependencies first)
1325-
gomega.Expect(client.TestData.StartOrder).To(gomega.ContainElement("priority-c"))
1326-
gomega.Expect(client.TestData.StartOrder).To(gomega.ContainElement("priority-b"))
1327-
gomega.Expect(client.TestData.StartOrder).To(gomega.ContainElement("priority-a"))
1325+
// Verify create order respects dependencies (dependencies first)
1326+
gomega.Expect(client.TestData.CreateOrder).To(gomega.ContainElement("priority-c"))
1327+
gomega.Expect(client.TestData.CreateOrder).To(gomega.ContainElement("priority-b"))
1328+
gomega.Expect(client.TestData.CreateOrder).To(gomega.ContainElement("priority-a"))
13281329
})
13291330

13301331
ginkgo.It("should handle mixed update types in restart sequences", func() {

0 commit comments

Comments
 (0)