From 2c999f1ecb87a6688c19502f0f594e002104e951 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Tue, 2 Aug 2022 16:18:57 +0200 Subject: pkg/autoupdate: update unit-by-unit Change the auto-update logic to update unit-by-unit rather by policy. This allows for, in theory now and in practice later, to have mutliple containers run in a single systemd unit and update them in sequence before restarting the unit. [NO NEW TESTS NEEDED] - should not change behavior. Signed-off-by: Valentin Rothberg --- pkg/autoupdate/autoupdate.go | 106 ++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 52 deletions(-) (limited to 'pkg') diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go index 58e919493..0f9df9327 100644 --- a/pkg/autoupdate/autoupdate.go +++ b/pkg/autoupdate/autoupdate.go @@ -50,17 +50,14 @@ var supportedPolicies = map[string]Policy{ "local": PolicyLocalImage, } -// policyMappers assembles update tasks by policy -type policyMapper map[Policy][]*task - // updater includes shared state for auto-updating one or more containers. type updater struct { - conn *dbus.Conn - idToImage map[string]*libimage.Image - imageToPolicyMapper map[string]policyMapper - options *entities.AutoUpdateOptions - updatedRawImages map[string]bool - runtime *libpod.Runtime + conn *dbus.Conn + idToImage map[string]*libimage.Image + unitToTasks map[string][]*task + options *entities.AutoUpdateOptions + updatedRawImages map[string]bool + runtime *libpod.Runtime } // task includes data and state for updating a container @@ -152,26 +149,22 @@ func (u *updater) assembleImageMap(ctx context.Context) error { // It returns a slice of successfully restarted systemd units and a slice of // errors encountered during auto update. func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) { + // Note that (most) errors are non-fatal such that a single + // misconfigured container does not prevent others from being updated + // (which could be a security threat). + auto := updater{ options: &options, runtime: runtime, updatedRawImages: make(map[string]bool), } - // Assemble a map `image ID -> *libimage.Image` that we can consult - // later on for lookups. - if err := auto.assembleImageMap(ctx); err != nil { - return nil, []error{err} - } - - // Create a map from `image ID -> []*Container`. - if errs := auto.imageContainersMap(); len(errs) > 0 { - return nil, errs - } + // Find auto-update tasks and assemble them by unit. + errors := auto.assembleTasks(ctx) // Nothing to do. - if len(auto.imageToPolicyMapper) == 0 { - return nil, nil + if len(auto.unitToTasks) == 0 { + return nil, errors } // Connect to DBUS. @@ -187,27 +180,28 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A // Update all images/container according to their auto-update policy. var allReports []*entities.AutoUpdateReport - var errs []error - for imageID, policyMapper := range auto.imageToPolicyMapper { - if _, exists := auto.idToImage[imageID]; !exists { - errs = append(errs, fmt.Errorf("container image ID %q not found in local storage", imageID)) - return nil, errs + for unit, tasks := range auto.unitToTasks { + // Sanity check: we'll support that in the future. + if len(tasks) != 1 { + errors = append(errors, fmt.Errorf("only 1 task per unit supported but unit %s has %d", unit, len(tasks))) + return nil, errors } - for _, task := range policyMapper[PolicyRegistryImage] { - report, err := auto.updateRegistry(ctx, task) - if err != nil { - errs = append(errs, err) - } - if report != nil { - allReports = append(allReports, report) + for _, task := range tasks { + var report *entities.AutoUpdateReport + var reportError error + + switch task.policy { + case PolicyRegistryImage: + report, reportError = auto.updateRegistry(ctx, task) + case PolicyLocalImage: + report, reportError = auto.updateLocally(ctx, task) + default: + reportError = fmt.Errorf("unexpected auto-update policy %s for container %s", task.policy, task.container.ID()) } - } - for _, task := range policyMapper[PolicyLocalImage] { - report, err := auto.updateLocally(ctx, task) - if err != nil { - errs = append(errs, err) + if reportError != nil { + errors = append(errors, reportError) } if report != nil { allReports = append(allReports, report) @@ -215,7 +209,7 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A } } - return allReports, errs + return allReports, errors } // updateRegistry updates the image/container according to the "registry" policy. @@ -372,15 +366,21 @@ func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container, } } -// imageContainersMap generates a map[image ID] -> [containers using the image] -// of all containers with a valid auto-update policy. -func (u *updater) imageContainersMap() []error { +// assembleTasks assembles update tasks per unit and populates a mapping from +// `unit -> []*task` such that multiple containers _can_ run in a single unit. +func (u *updater) assembleTasks(ctx context.Context) []error { + // Assemble a map `image ID -> *libimage.Image` that we can consult + // later on for lookups. + if err := u.assembleImageMap(ctx); err != nil { + return []error{err} + } + allContainers, err := u.runtime.GetAllContainers() if err != nil { return []error{err} } - u.imageToPolicyMapper = make(map[string]policyMapper) + u.unitToTasks = make(map[string][]*task) errors := []error{} for _, c := range allContainers { @@ -395,6 +395,8 @@ func (u *updater) imageContainersMap() []error { continue } + // Check the container's auto-update policy which is configured + // as a label. labels := ctr.Labels() value, exists := labels[Label] if !exists { @@ -405,18 +407,19 @@ func (u *updater) imageContainersMap() []error { errors = append(errors, err) continue } - - // Skip labels not related to autoupdate if policy == PolicyDefault { continue } - id, _ := ctr.Image() - policyMap, exists := u.imageToPolicyMapper[id] + // Make sure the container runs in a systemd unit which is + // stored as a label at container creation. + unit, exists := labels[systemdDefine.EnvVariable] if !exists { - policyMap = make(map[Policy][]*task) + errors = append(errors, fmt.Errorf("auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable)) + continue } + id, _ := ctr.Image() image, exists := u.idToImage[id] if !exists { err := fmt.Errorf("internal error: no image found for ID %s", id) @@ -424,16 +427,15 @@ func (u *updater) imageContainersMap() []error { continue } - unit, _ := labels[systemdDefine.EnvVariable] - t := task{ container: ctr, policy: policy, image: image, unit: unit, } - policyMap[policy] = append(policyMap[policy], &t) - u.imageToPolicyMapper[id] = policyMap + + // Add the task to the unit. + u.unitToTasks[unit] = append(u.unitToTasks[unit], &t) } return errors -- cgit v1.2.3-54-g00ecf