chore: refinements in container lifecycle hooks (#78)

* feat: make container lifecycle configurable

* chore: simplify setting credentials for pulling images

* chore: refine Create hooks

* chore: proper label name for client version

* fix: lint

* chore: validate just one auth config is returned for the image

* fix: include empty credentials

Author: mdelapenya

client/labels.go

@@ -3,14 +3,14 @@ package client
 import "maps"
 
 const (
-	// LabelBase is the base label for all Docker labels.
+	// LabelBase is the base label for all Docker SDK labels.
 	LabelBase = "com.docker.sdk"
 
-	// LabelLang specifies the language which created the container.
+	// LabelLang specifies the language.
 	LabelLang = LabelBase + ".lang"
 
-	// LabelVersion specifies the version of go-sdk which created the container.
-	LabelVersion = LabelBase + ".version"
+	// LabelVersion specifies the version of go-sdk's client.
+	LabelVersion = LabelBase + ".client"
 )
 
 // sdkLabels is a map of labels that can be used to identify resources

container/container.run.go

@@ -2,16 +2,12 @@ package container
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
-	"github.com/containerd/errdefs"
-	"github.com/containerd/platforms"
-
 	"github.com/docker/docker/api/types/container"
-	apiimage "github.com/docker/docker/api/types/image"
 	apinetwork "github.com/docker/docker/api/types/network"
 	"github.com/docker/go-sdk/client"
-	"github.com/docker/go-sdk/image"
 )
 
 // Run is a convenience function that creates a new container and starts it.
@@ -23,6 +19,17 @@ func Run(ctx context.Context, opts ...ContainerCustomizer) (*Container, error) {
 		started: true,
 	}
 
+	// initialize the validate functions with the default ones
+	def.validateFuncs = []func() error{
+		func() error {
+			if def.image == "" {
+				return errors.New("image is required")
+			}
+			return nil
+		},
+		def.validateMounts,
+	}
+
 	for _, opt := range opts {
 		if err := opt.Customize(&def); err != nil {
 			return nil, fmt.Errorf("customize: %w", err)
@@ -51,54 +58,6 @@ func Run(ctx context.Context, opts ...ContainerCustomizer) (*Container, error) {
 		DefaultLoggingHook,
 	}
 
-	for _, is := range def.imageSubstitutors {
-		modifiedTag, err := is.Substitute(def.image)
-		if err != nil {
-			return nil, fmt.Errorf("failed to substitute image %s with %s: %w", def.image, is.Description(), err)
-		}
-
-		if modifiedTag != def.image {
-			def.dockerClient.Logger().Info("Replacing image", "description", is.Description(), "from", def.image, "to", modifiedTag)
-			def.image = modifiedTag
-		}
-	}
-
-	var platform *platforms.Platform
-
-	if def.imagePlatform != "" {
-		p, err := platforms.Parse(def.imagePlatform)
-		if err != nil {
-			return nil, fmt.Errorf("invalid platform %s: %w", def.imagePlatform, err)
-		}
-		platform = &p
-	}
-
-	var shouldPullImage bool
-
-	if def.alwaysPullImage {
-		shouldPullImage = true // If requested always attempt to pull image
-	} else {
-		img, err := def.dockerClient.ImageInspect(ctx, def.image)
-		if err != nil {
-			if !errdefs.IsNotFound(err) {
-				return nil, err
-			}
-			shouldPullImage = true
-		}
-		if platform != nil && (img.Architecture != platform.Architecture || img.Os != platform.OS) {
-			shouldPullImage = true
-		}
-	}
-
-	if shouldPullImage {
-		pullOpt := apiimage.PullOptions{
-			Platform: def.imagePlatform, // may be empty
-		}
-		if err := image.Pull(ctx, def.image, image.WithPullClient(def.dockerClient), image.WithPullOptions(pullOpt)); err != nil {
-			return nil, err
-		}
-	}
-
 	def.labels[moduleLabel] = Version()
 
 	dockerInput := &container.Config{
@@ -131,7 +90,25 @@ func Run(ctx context.Context, opts ...ContainerCustomizer) (*Container, error) {
 		return nil, err
 	}
 
-	resp, err := def.dockerClient.ContainerCreate(ctx, dockerInput, hostConfig, networkingConfig, platform, def.name)
+	// Image substitution must be done after the creating hook has been called,
+	// as the image could have been overridden in there.
+	for _, is := range def.imageSubstitutors {
+		modifiedTag, err := is.Substitute(def.image)
+		if err != nil {
+			return nil, fmt.Errorf("failed to substitute image %s with %s: %w", def.image, is.Description(), err)
+		}
+
+		if modifiedTag != def.image {
+			def.dockerClient.Logger().Info("Replacing image", "description", is.Description(), "from", def.image, "to", modifiedTag)
+			def.image = modifiedTag
+		}
+	}
+
+	// Update the image name in the docker input after the creating hook has been called,
+	// as it could have been overridden in there.
+	dockerInput.Image = def.image
+
+	resp, err := def.dockerClient.ContainerCreate(ctx, dockerInput, hostConfig, networkingConfig, def.platform, def.name)
 	if err != nil {
 		return nil, fmt.Errorf("container create: %w", err)
 	}

container/definition.go

@@ -5,6 +5,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/containerd/platforms"
+
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/go-sdk/client"
@@ -45,6 +47,9 @@ type Definition struct {
 	// hostConfigModifier the modifier for the host config before container creation
 	hostConfigModifier func(*container.HostConfig)
 
+	// validateFuncs the functions to validate the definition.
+	validateFuncs []func() error
+
 	// imageSubstitutors the image substitutors to use for the container.
 	imageSubstitutors []ImageSubstitutor
 
@@ -72,6 +77,10 @@ type Definition struct {
 	// imagePlatform the platform of the image
 	imagePlatform string
 
+	// platform the platform of the container.
+	// Used to override the platform of the image when building the container.
+	platform *platforms.Platform
+
 	// name the name of the container.
 	name string
 
@@ -84,15 +93,14 @@ type Definition struct {
 
 // validate validates the definition.
 func (d *Definition) validate() error {
-	if d.image == "" {
-		return errors.New("image is required")
-	}
-
-	if err := d.validateMounts(); err != nil {
-		return fmt.Errorf("validate mounts: %w", err)
+	var errs []error
+	for _, fn := range d.validateFuncs {
+		if err := fn(); err != nil {
+			errs = append(errs, err)
+		}
 	}
 
-	return nil
+	return errors.Join(errs...)
 }
 
 // DockerClient returns the docker client used by the definition.
@@ -105,6 +113,20 @@ func (d *Definition) Image() string {
 	return d.image
 }
 
+// ImageSubstitutors returns the image substitutors used by the definition.
+func (d *Definition) ImageSubstitutors() []ImageSubstitutor {
+	return d.imageSubstitutors
+}
+
+// Labels returns the labels used by the definition.
+func (d *Definition) Labels() map[string]string {
+	if d.labels == nil {
+		d.labels = make(map[string]string)
+	}
+
+	return d.labels
+}
+
 // Name returns the name of the container.
 func (d *Definition) Name() string {
 	return d.name

container/lifecycle.go

@@ -3,14 +3,20 @@ package container
 import (
 	"context"
 	"errors"
+	"fmt"
 	"io"
 	"log/slog"
 	"reflect"
 	"strings"
 	"time"
 
+	"github.com/containerd/errdefs"
+	"github.com/containerd/platforms"
+
+	apiimage "github.com/docker/docker/api/types/image"
 	"github.com/docker/go-sdk/container/exec"
 	"github.com/docker/go-sdk/container/wait"
+	"github.com/docker/go-sdk/image"
 )
 
 type LifecycleHooks struct {
@@ -188,6 +194,10 @@ func combineContainerHooks(defaultHooks, userDefinedHooks []LifecycleHooks) Life
 		}
 	}
 
+	// Always append the default pull hook after any other pre-create hook.
+	// User could have defined a build hook in which the image has not been defined yet.
+	hooks.PreCreates = append(hooks.PreCreates, defaultPullHook...)
+
 	return hooks
 }
 
@@ -202,13 +212,13 @@ func applyContainerHooks(ctx context.Context, hooks []ContainerHook, ctr *Contai
 }
 
 func applyDefinitionHooks(ctx context.Context, hooks []DefinitionHook, def *Definition) error {
-	var errs []error
 	for _, hook := range hooks {
 		if err := hook(ctx, def); err != nil {
-			errs = append(errs, err)
+			return fmt.Errorf("apply definition hook: %w", err)
 		}
 	}
-	return errors.Join(errs...)
+
+	return nil
 }
 
 // applyLifecycleHooks calls hook on all LifecycleHooks.
@@ -258,3 +268,48 @@ func (c *Container) applyLifecycleHooks(ctx context.Context, logError bool, hook
 
 	return nil
 }
+
+// defaultPullHook is a hook that will pull the image if it is not present or if the platform is different.
+// It must be used as a [DefinitionHook] and not as a [ContainerHook] because it needs to be executed before the container is created.
+var defaultPullHook = []DefinitionHook{
+	func(ctx context.Context, def *Definition) error {
+		var platform *platforms.Platform
+
+		if def.imagePlatform != "" {
+			p, err := platforms.Parse(def.imagePlatform)
+			if err != nil {
+				return fmt.Errorf("invalid platform %s: %w", def.imagePlatform, err)
+			}
+			platform = &p
+			def.platform = platform
+		}
+
+		var shouldPullImage bool
+
+		if def.alwaysPullImage {
+			shouldPullImage = true // If requested always attempt to pull image
+		} else {
+			img, err := def.dockerClient.ImageInspect(ctx, def.image)
+			if err != nil {
+				if !errdefs.IsNotFound(err) {
+					return err
+				}
+				shouldPullImage = true
+			}
+			if platform != nil && (img.Architecture != platform.Architecture || img.Os != platform.OS) {
+				shouldPullImage = true
+			}
+		}
+
+		if shouldPullImage {
+			pullOpt := apiimage.PullOptions{
+				Platform: def.imagePlatform, // may be empty
+			}
+			if err := image.Pull(ctx, def.image, image.WithPullClient(def.dockerClient), image.WithPullOptions(pullOpt)); err != nil {
+				return err
+			}
+		}
+
+		return nil
+	},
+}

container/lifecycle_test.go

@@ -7,6 +7,8 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+
+	"github.com/docker/go-sdk/client"
 )
 
 func TestCombineLifecycleHooks(t *testing.T) {
@@ -51,7 +53,16 @@ func TestCombineLifecycleHooks(t *testing.T) {
 	// call all the hooks in the right order, honouring the lifecycle
 
 	def := Definition{
+		image:          nginxAlpineImage,
 		lifecycleHooks: []LifecycleHooks{combineContainerHooks(defaultHooks, userDefinedHooks)},
+		// define a docker client to avoid the need to initialize the client
+		dockerClient: client.DefaultClient,
+		// avoid validation errors
+		validateFuncs: []func() error{
+			func() error {
+				return nil
+			},
+		},
 	}
 	err := def.creatingHook(context.Background())
 	require.NoError(t, err)