Skip to content

Commit 9d9e8d6

Browse files
authored
chore: various run local fixes (#5459)
Fixes the following issues: - Download the ARM version of session manager plugin in pause container if using ARM binary - Considers all containers "essential" as we did previously - meaning, if a container stops, that is reported as an error - Adds back status messages of what the orchestrator is doing after `Stop()` - Passes `--init` to `docker run` for the pause container so it responds to `docker stop` By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
1 parent c94bba3 commit 9d9e8d6

4 files changed

Lines changed: 177 additions & 78 deletions

File tree

internal/pkg/docker/dockerengine/dockerengine.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ type RunOptions struct {
8787
ContainerNetwork string // Optional. Network mode for the container.
8888
LogOptions RunLogOptions // Optional. Configure logging for output from the container
8989
AddLinuxCapabilities []string // Optional. Adds linux capabilities to the container.
90+
Init bool // Optional. Adds an init process as an entrypoint.
9091
}
9192

9293
// RunLogOptions holds the logging configuration for Run().
@@ -274,6 +275,10 @@ func (in *RunOptions) generateRunArguments() []string {
274275
args = append(args, "--cap-add", cap)
275276
}
276277

278+
if in.Init {
279+
args = append(args, "--init")
280+
}
281+
277282
args = append(args, in.ImageURI)
278283

279284
if in.Command != nil && len(in.Command) > 0 {
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
FROM public.ecr.aws/amazonlinux/amazonlinux:2023
22

33
RUN dnf install -y aws-cli iptables
4-
RUN dnf install -y https://s3.amazonaws.com/session-manager-downloads/plugin/latest/linux_64bit/session-manager-plugin.rpm
4+
5+
ARG ARCH=64bit
6+
# url from https://docs.aws.amazon.com/systems-manager/latest/userguide/install-plugin-linux.html
7+
RUN dnf install -y https://s3.amazonaws.com/session-manager-downloads/plugin/latest/linux_${ARCH}/session-manager-plugin.rpm

internal/pkg/docker/orchestrator/orchestrator.go

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ import (
1212
"maps"
1313
"net"
1414
"os"
15+
"runtime"
1516
"strconv"
17+
"strings"
1618
"sync"
1719
"sync/atomic"
1820
"time"
@@ -283,11 +285,6 @@ func (o *Orchestrator) setupProxyConnections(ctx context.Context, pauseContainer
283285
return fmt.Errorf("modify iptables: %w", err)
284286
}
285287

286-
err = o.docker.Exec(ctx, pauseContainer, io.Discard, "iptables-save")
287-
if err != nil {
288-
return fmt.Errorf("save iptables: %w", err)
289-
}
290-
291288
err = o.docker.Exec(ctx, pauseContainer, io.Discard, "/bin/bash",
292289
"-c", fmt.Sprintf(`echo %s %s >> /etc/hosts`, ip.String(), host.Name))
293290
if err != nil {
@@ -338,10 +335,18 @@ func ipv4Increment(ip net.IP, network *net.IPNet) (net.IP, error) {
338335
}
339336

340337
func (o *Orchestrator) buildPauseContainer(ctx context.Context) error {
338+
arch := "64bit"
339+
if strings.Contains(runtime.GOARCH, "arm") {
340+
arch = "arm64"
341+
}
342+
341343
return o.docker.Build(ctx, &dockerengine.BuildArguments{
342344
URI: pauseCtrURI,
343345
Tags: []string{pauseCtrTag},
344346
DockerfileContent: pauseDockerfile,
347+
Args: map[string]string{
348+
"ARCH": arch,
349+
},
345350
}, os.Stderr)
346351
}
347352

@@ -351,6 +356,7 @@ func (o *Orchestrator) buildPauseContainer(ctx context.Context) error {
351356
func (o *Orchestrator) Stop() {
352357
o.stopOnce.Do(func() {
353358
close(o.stopped)
359+
fmt.Printf("\nStopping task...\n")
354360
o.actions <- &stopAction{}
355361
})
356362
}
@@ -368,9 +374,11 @@ func (a *stopAction) Do(o *Orchestrator) error {
368374
}
369375

370376
// stop pause container
377+
fmt.Printf("Stopping %q\n", "pause")
371378
if err := o.docker.Stop(context.Background(), o.containerID("pause")); err != nil {
372379
errs = append(errs, fmt.Errorf("stop %q: %w", "pause", err))
373380
}
381+
fmt.Printf("Stopped %q\n", "pause")
374382

375383
return errors.Join(errs...)
376384
}
@@ -386,10 +394,12 @@ func (o *Orchestrator) stopTask(ctx context.Context, task Task) error {
386394
for name := range task.Containers {
387395
name := name
388396
go func() {
397+
fmt.Printf("Stopping %q\n", name)
389398
if err := o.docker.Stop(ctx, o.containerID(name)); err != nil {
390399
errCh <- fmt.Errorf("stop %q: %w", name, err)
391400
return
392401
}
402+
fmt.Printf("Stopped %q\n", name)
393403
errCh <- nil
394404
}()
395405
}
@@ -456,6 +466,7 @@ func (o *Orchestrator) pauseRunOptions(t Task) dockerengine.RunOptions {
456466
ContainerPorts: make(map[string]string),
457467
Secrets: t.PauseSecrets,
458468
AddLinuxCapabilities: []string{"NET_ADMIN"},
469+
Init: true,
459470
}
460471

461472
for _, ctr := range t.Containers {
@@ -486,18 +497,22 @@ func (o *Orchestrator) run(taskID int32, opts dockerengine.RunOptions) {
486497
o.wg.Add(1)
487498
go func() {
488499
defer o.wg.Done()
500+
err := o.docker.Run(context.Background(), &opts)
489501

490-
if err := o.docker.Run(context.Background(), &opts); err != nil {
491-
curTaskID := o.curTaskID.Load()
492-
if curTaskID == orchestratorStoppedTaskID {
493-
return
494-
}
502+
// if the orchestrator has already stopped,
503+
// we don't want to report the error
504+
curTaskID := o.curTaskID.Load()
505+
if curTaskID == orchestratorStoppedTaskID {
506+
return
507+
}
495508

496-
// the error is from the pause container
497-
// or from the currently running task
498-
if taskID == pauseCtrTaskID || taskID == curTaskID {
499-
o.runErrs <- fmt.Errorf("run %q: %w", opts.ContainerName, err)
509+
// the error is from the pause container
510+
// or from the currently running task
511+
if taskID == pauseCtrTaskID || taskID == curTaskID {
512+
if err == nil {
513+
err = errors.New("container stopped unexpectedly")
500514
}
515+
o.runErrs <- fmt.Errorf("run %q: %w", opts.ContainerName, err)
501516
}
502517
}()
503518
}

0 commit comments

Comments
 (0)