Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add missing service container health check #2354

Merged
merged 4 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pkg/container/container_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ type Container interface {
Remove() common.Executor
Close() common.Executor
ReplaceLogWriter(io.Writer, io.Writer) (io.Writer, io.Writer)
GetHealth(ctx context.Context) ContainerHealth
}

// NewDockerBuildExecutorInput the input for the NewDockerBuildExecutor function
Expand All @@ -73,3 +74,11 @@ type NewDockerPullExecutorInput struct {
Username string
Password string
}

type ContainerHealth int

const (
ContainerHealthStarting ContainerHealth = iota
ContainerHealthHealthy
ContainerHealthUnHealthy
)
24 changes: 24 additions & 0 deletions pkg/container/docker_run.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,30 @@
).IfNot(common.Dryrun)
}

func (cr *containerReference) GetHealth(ctx context.Context) ContainerHealth {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uses docker inspect to poll container health like actions/runner

resp, err := cr.cli.ContainerInspect(ctx, cr.id)
logger := common.Logger(ctx)
if err != nil {
logger.Errorf("failed to query container health %s", err)
return ContainerHealthUnHealthy

Check warning on line 177 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L176-L177

Added lines #L176 - L177 were not covered by tests
}
if resp.Config == nil || resp.Config.Healthcheck == nil || resp.State == nil || resp.State.Health == nil || len(resp.Config.Healthcheck.Test) == 1 && strings.EqualFold(resp.Config.Healthcheck.Test[0], "NONE") {
logger.Debugf("no container health check defined")
return ContainerHealthHealthy
}

logger.Infof("container health of %s (%s) is %s", cr.id, resp.Config.Image, resp.State.Health.Status)
switch resp.State.Health.Status {
case "starting":
return ContainerHealthStarting
case "healthy":
return ContainerHealthHealthy
case "unhealthy":
return ContainerHealthUnHealthy

Check warning on line 191 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L190-L191

Added lines #L190 - L191 were not covered by tests
}
return ContainerHealthUnHealthy

Check warning on line 193 in pkg/container/docker_run.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/docker_run.go#L193

Added line #L193 was not covered by tests
}

func (cr *containerReference) ReplaceLogWriter(stdout io.Writer, stderr io.Writer) (io.Writer, io.Writer) {
out := cr.input.Stdout
err := cr.input.Stderr
Expand Down
4 changes: 4 additions & 0 deletions pkg/container/host_environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,10 @@
}
}

func (e *HostEnvironment) GetHealth(ctx context.Context) ContainerHealth {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Service container are not implemented for HostEnvironment, just a stub that is probably never called

return ContainerHealthHealthy

Check warning on line 456 in pkg/container/host_environment.go

View check run for this annotation

Codecov / codecov/patch

pkg/container/host_environment.go#L455-L456

Added lines #L455 - L456 were not covered by tests
}

func (e *HostEnvironment) ReplaceLogWriter(stdout io.Writer, _ io.Writer) (io.Writer, io.Writer) {
org := e.StdOut
e.StdOut = stdout
Expand Down
36 changes: 36 additions & 0 deletions pkg/runner/run_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"runtime"
"strconv"
"strings"
"time"

"github.com/docker/go-connections/nat"
"github.com/nektos/act/pkg/common"
Expand Down Expand Up @@ -420,6 +421,7 @@
Mode: 0o666,
Body: "",
}),
rc.waitForServiceContainers(),
)(ctx)
}
}
Expand Down Expand Up @@ -518,6 +520,40 @@
}
}

func (rc *RunContext) waitForServiceContainer(c container.ExecutionsEnvironment) common.Executor {
return func(ctx context.Context) error {
sctx, cancel := context.WithTimeout(ctx, time.Minute*5)
defer cancel()
health := container.ContainerHealthStarting
delay := time.Second
for i := 0; ; i++ {
health = c.GetHealth(sctx)
if health != container.ContainerHealthStarting || i > 30 {
break
}
time.Sleep(delay)
delay *= 2
if delay > 10*time.Second {
delay = 10 * time.Second
}
Comment on lines +536 to +538
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never poll less than every 10s, not looked up anywhere.
"Fake exponential backoff"

}
if health == container.ContainerHealthHealthy {
return nil
}
return fmt.Errorf("service container failed to start")

Check warning on line 543 in pkg/runner/run_context.go

View check run for this annotation

Codecov / codecov/patch

pkg/runner/run_context.go#L543

Added line #L543 was not covered by tests
}
}

func (rc *RunContext) waitForServiceContainers() common.Executor {
return func(ctx context.Context) error {
execs := []common.Executor{}
for _, c := range rc.ServiceContainers {
execs = append(execs, rc.waitForServiceContainer(c))
}
return common.NewParallelExecutor(len(execs), execs...)(ctx)
}
}

func (rc *RunContext) stopServiceContainers() common.Executor {
return func(ctx context.Context) error {
execs := []common.Executor{}
Expand Down
1 change: 1 addition & 0 deletions pkg/runner/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,7 @@ func TestRunEvent(t *testing.T) {
{workdir, "services-empty-image", "push", "", platforms, secrets},
{workdir, "services-host-network", "push", "", platforms, secrets},
{workdir, "services-with-container", "push", "", platforms, secrets},
{workdir, "mysql-service-container-with-health-check", "push", "", platforms, secrets},

// local remote action overrides
{workdir, "local-remote-action-overrides", "push", "", platforms, secrets},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
name: service-container
on: push
jobs:
service-container-test:
runs-on: ubuntu-latest
container: mysql:8
services:
maindb:
image: mysql:8
env:
MYSQL_DATABASE: dbname
MYSQL_USER: dbuser
MYSQL_PASSWORD: dbpass
MYSQL_RANDOM_ROOT_PASSWORD: yes
options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=3
steps:
- run: mysql -u dbuser -D dbname -pdbpass -h maindb -e "create table T(id INT NOT NULL AUTO_INCREMENT, val VARCHAR(255), PRIMARY KEY (id))"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

currently this workflow fails to run, because maindb is still starting.

The actions runner wait for it to be ready

- run: mysql -u dbuser -D dbname -pdbpass -h maindb -e "insert into T(val) values ('test'),('h')"
- run: mysql -u dbuser -D dbname -pdbpass -h maindb -e "select * from T"
Loading