Skip to content

Commit d022fbe

Browse files
mxpvmxpv
committed
Address PR comments
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
1 parent 6fa1bb4 commit d022fbe

File tree

2 files changed

+30
-28
lines changed

2 files changed

+30
-28
lines changed

‎runtime/v2/manager.go‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -114,37 +114,37 @@ func init() {
114114
}
115115

116116
func migrateTasks(ic *plugin.InitContext, shimManager *ShimManager) error {
117-
if !shimManager.list.IsEmpty() {
117+
if !shimManager.shims.IsEmpty() {
118118
return nil
119119
}
120120

121121
// Rename below will fail is target directory exists.
122122
// `Root` and `State` dirs expected to be empty at this point (we check that there are no shims loaded above).
123123
// If for some they are not empty, these remove calls will fail (`os.Remove` requires a dir to be empty to succeed).
124124
if err := os.Remove(shimManager.root); err != nil && !os.IsNotExist(err) {
125-
return err
125+
return fmt.Errorf("failed to remove `root` dir: %w", err)
126126
}
127127

128128
if err := os.Remove(shimManager.state); err != nil && !os.IsNotExist(err) {
129-
return err
129+
return fmt.Errorf("failed to remove `state` dir: %w", err)
130130
}
131131

132132
if err := os.Rename(ic.Root, shimManager.root); err != nil {
133133
if os.IsNotExist(err) {
134134
return nil
135135
}
136-
return fmt.Errorf("failed to migrate task `root` directory")
136+
return fmt.Errorf("failed to migrate task `root` directory: %w", err)
137137
}
138138

139139
if err := os.Rename(ic.State, shimManager.state); err != nil {
140140
if os.IsNotExist(err) {
141141
return nil
142142
}
143-
return fmt.Errorf("failed to migrate task `state` directory")
143+
return fmt.Errorf("failed to migrate task `state` directory: %w", err)
144144
}
145145

146146
if err := shimManager.loadExistingTasks(ic.Context); err != nil {
147-
return fmt.Errorf("failed to load tasks after migration")
147+
return fmt.Errorf("failed to load tasks after migration: %w", err)
148148
}
149149

150150
return nil
@@ -173,7 +173,7 @@ func NewShimManager(ctx context.Context, config *ManagerConfig) (*ShimManager, e
173173
state: config.State,
174174
containerdAddress: config.Address,
175175
containerdTTRPCAddress: config.TTRPCAddress,
176-
list: runtime.NewTaskList(),
176+
shims: runtime.NewTaskList(),
177177
events: config.Events,
178178
containers: config.Store,
179179
schedCore: config.SchedCore,
@@ -196,7 +196,7 @@ type ShimManager struct {
196196
containerdAddress string
197197
containerdTTRPCAddress string
198198
schedCore bool
199-
list *runtime.TaskList
199+
shims *runtime.TaskList
200200
events *exchange.Exchange
201201
containers containers.Store
202202
}
@@ -235,7 +235,7 @@ func (m *ShimManager) Start(ctx context.Context, id string, opts runtime.CreateO
235235
task: task.NewTaskClient(shim.client),
236236
}
237237

238-
if err := m.list.Add(ctx, shimTask); err != nil {
238+
if err := m.shims.Add(ctx, shimTask); err != nil {
239239
return nil, errors.Wrap(err, "failed to add task")
240240
}
241241

@@ -262,12 +262,12 @@ func (m *ShimManager) startShim(ctx context.Context, bundle *Bundle, id string,
262262
shim, err := b.Start(ctx, topts, func() {
263263
log.G(ctx).WithField("id", id).Info("shim disconnected")
264264

265-
cleanupAfterDeadShim(context.Background(), id, ns, m.list, m.events, b)
265+
cleanupAfterDeadShim(context.Background(), id, ns, m.shims, m.events, b)
266266
// Remove self from the runtime task list. Even though the cleanupAfterDeadShim()
267267
// would publish taskExit event, but the shim.Delete() would always failed with ttrpc
268268
// disconnect and there is no chance to remove this dead task from runtime task lists.
269269
// Thus it's better to delete it here.
270-
m.list.Delete(ctx, id)
270+
m.shims.Delete(ctx, id)
271271
})
272272
if err != nil {
273273
return nil, errors.Wrap(err, "start failed")
@@ -282,11 +282,11 @@ func (m *ShimManager) cleanupShim(shim *shim) {
282282
defer cancel()
283283

284284
_ = shim.delete(dctx)
285-
m.list.Delete(dctx, shim.ID())
285+
m.shims.Delete(dctx, shim.ID())
286286
}
287287

288288
func (m *ShimManager) Get(ctx context.Context, id string) (ShimProcess, error) {
289-
proc, err := m.list.Get(ctx, id)
289+
proc, err := m.shims.Get(ctx, id)
290290
if err != nil {
291291
return nil, err
292292
}
@@ -296,14 +296,14 @@ func (m *ShimManager) Get(ctx context.Context, id string) (ShimProcess, error) {
296296

297297
// Delete a runtime task
298298
func (m *ShimManager) Delete(ctx context.Context, id string) error {
299-
proc, err := m.list.Get(ctx, id)
299+
proc, err := m.shims.Get(ctx, id)
300300
if err != nil {
301301
return err
302302
}
303303

304304
shimTask := proc.(*shimTask)
305305
err = shimTask.shim.delete(ctx)
306-
m.list.Delete(ctx, id)
306+
m.shims.Delete(ctx, id)
307307

308308
return err
309309
}
@@ -322,13 +322,13 @@ func parsePlatforms(platformStr []string) ([]ocispec.Platform, error) {
322322

323323
// TaskManager wraps task service client on top of shim manager.
324324
type TaskManager struct {
325-
shims *ShimManager
325+
manager *ShimManager
326326
}
327327

328328
// NewTaskManager creates a new task manager instance.
329329
func NewTaskManager(shims *ShimManager) *TaskManager {
330330
return &TaskManager{
331-
shims: shims,
331+
manager: shims,
332332
}
333333
}
334334

@@ -339,19 +339,21 @@ func (m *TaskManager) ID() string {
339339

340340
// Create launches new shim instance and creates new task
341341
func (m *TaskManager) Create(ctx context.Context, taskID string, opts runtime.CreateOpts) (runtime.Task, error) {
342-
process, err := m.shims.Start(ctx, taskID, opts)
342+
process, err := m.manager.Start(ctx, taskID, opts)
343343
if err != nil {
344344
return nil, errors.Wrap(err, "failed to start shim")
345345
}
346346

347+
// Cast to shim task and call task service to create a new container task instance.
348+
// This will not be required once shim service / client implemented.
347349
shim := process.(*shimTask)
348350
t, err := shim.Create(ctx, opts)
349351
if err != nil {
350352
dctx, cancel := timeout.WithContext(context.Background(), cleanupTimeout)
351353
defer cancel()
352354

353355
_, errShim := shim.delete(dctx, func(ctx context.Context, id string) {
354-
m.shims.list.Delete(ctx, id)
356+
m.manager.shims.Delete(ctx, id)
355357
})
356358

357359
if errShim != nil {
@@ -372,24 +374,24 @@ func (m *TaskManager) Create(ctx context.Context, taskID string, opts runtime.Cr
372374

373375
// Get a specific task
374376
func (m *TaskManager) Get(ctx context.Context, id string) (runtime.Task, error) {
375-
return m.shims.list.Get(ctx, id)
377+
return m.manager.shims.Get(ctx, id)
376378
}
377379

378380
// Tasks lists all tasks
379381
func (m *TaskManager) Tasks(ctx context.Context, all bool) ([]runtime.Task, error) {
380-
return m.shims.list.GetAll(ctx, all)
382+
return m.manager.shims.GetAll(ctx, all)
381383
}
382384

383385
// Delete deletes the task and shim instance
384386
func (m *TaskManager) Delete(ctx context.Context, taskID string) (*runtime.Exit, error) {
385-
item, err := m.shims.list.Get(ctx, taskID)
387+
item, err := m.manager.shims.Get(ctx, taskID)
386388
if err != nil {
387389
return nil, err
388390
}
389391

390392
shimTask := item.(*shimTask)
391393
exit, err := shimTask.delete(ctx, func(ctx context.Context, id string) {
392-
m.shims.list.Delete(ctx, id)
394+
m.manager.shims.Delete(ctx, id)
393395
})
394396

395397
if err != nil {

‎runtime/v2/shim_load.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,15 @@ func (m *ShimManager) loadShims(ctx context.Context) error {
107107
shim, err := loadShim(ctx, bundle, func() {
108108
log.G(ctx).WithField("id", id).Info("shim disconnected")
109109

110-
cleanupAfterDeadShim(context.Background(), id, ns, m.list, m.events, binaryCall)
110+
cleanupAfterDeadShim(context.Background(), id, ns, m.shims, m.events, binaryCall)
111111
// Remove self from the runtime task list.
112-
m.list.Delete(ctx, id)
112+
m.shims.Delete(ctx, id)
113113
})
114114
if err != nil {
115-
cleanupAfterDeadShim(ctx, id, ns, m.list, m.events, binaryCall)
115+
cleanupAfterDeadShim(ctx, id, ns, m.shims, m.events, binaryCall)
116116
continue
117117
}
118-
m.list.Add(ctx, shim)
118+
m.shims.Add(ctx, shim)
119119
}
120120
return nil
121121
}
@@ -133,7 +133,7 @@ func (m *ShimManager) cleanupWorkDirs(ctx context.Context) error {
133133
// if the task was not loaded, cleanup and empty working directory
134134
// this can happen on a reboot where /run for the bundle state is cleaned up
135135
// but that persistent working dir is left
136-
if _, err := m.list.Get(ctx, d.Name()); err != nil {
136+
if _, err := m.shims.Get(ctx, d.Name()); err != nil {
137137
path := filepath.Join(m.root, ns, d.Name())
138138
if err := os.RemoveAll(path); err != nil {
139139
log.G(ctx).WithError(err).Errorf("cleanup working dir %s", path)

0 commit comments

Comments
 (0)