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.

Sign up for GitHub

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

Apply update strategies app-wide #2230

Merged
merged 2 commits into from
Feb 1, 2024
Merged

Apply update strategies app-wide #2230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87b1956
Use the state-helper functions in app module tests
pipex Mar 7, 2023
3afcef2
Respect update strategies app-wide instead of at the service level
cywang117 Dec 15, 2023
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
126 changes: 72 additions & 54 deletions src/compose/app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import {
import * as targetStateCache from '../device-state/target-state-cache';
import { getNetworkGateway } from '../lib/docker-utils';
import * as constants from '../lib/constants';

import { getStepsFromStrategy } from './update-strategies';

import { InternalInconsistencyError, isNotFoundError } from '../lib/errors';
import {
getStepsFromStrategy,
getStrategyFromService,
} from './update-strategies';
import { isNotFoundError } from '../lib/errors';
import * as config from '../config';
import { checkTruthy, checkString } from '../lib/validation';
import { checkTruthy } from '../lib/validation';
import { ServiceComposeConfig, DeviceMetadata } from './types/service';
import { pathExistsOnRoot } from '../lib/host-utils';
import { isSupervisor } from '../lib/supervisor-metadata';
Expand Down Expand Up @@ -133,17 +134,8 @@ export class App {
state.containerIds,
);

for (const { current: svc } of removePairs) {
// All removes get a kill action if they're not already stopping
if (svc!.status !== 'Stopping') {
steps.push(generateStep('kill', { current: svc! }));
} else {
steps.push(generateStep('noop', {}));
}
}

// For every service which needs to be updated, update via update strategy.
const servicePairs = updatePairs.concat(installPairs);
const servicePairs = removePairs.concat(updatePairs, installPairs);
steps = steps.concat(
servicePairs
.map((pair) =>
Expand Down Expand Up @@ -511,8 +503,8 @@ export class App {
},
): Nullable<CompositionStep> {
if (current?.status === 'Stopping') {
// Theres a kill step happening already, emit a noop to ensure we stay alive while
// this happens
// There's a kill step happening already, emit a noop to ensure
// we stay alive while this happens
return generateStep('noop', {});
}
if (current?.status === 'Dead') {
Expand All @@ -521,16 +513,14 @@ export class App {
return;
}

const needsDownload = !_.some(
context.availableImages,
const needsDownload = !context.availableImages.some(
(image) =>
image.dockerImageId === target?.config.image ||
imageManager.isSameImage(image, { name: target?.imageName! }),
);

if (needsDownload && context.downloading.includes(target?.imageName!)) {
// The image needs to be downloaded, and it's currently downloading. We simply keep
// the application loop alive
// The image needs to be downloaded, and it's currently downloading.
// We simply keep the application loop alive
return generateStep('noop', {});
}

Expand All @@ -546,47 +536,39 @@ export class App {
context.servicePairs,
);
} else {
if (!target) {
throw new InternalInconsistencyError(
'An empty changing pair passed to generateStepsForService',
);
}

// This service is in both current & target so requires an update,
// or it's a service that's not in target so requires removal
const needsSpecialKill = this.serviceHasNetworkOrVolume(
current,
context.networkPairs,
context.volumePairs,
);

if (
!needsSpecialKill &&
target != null &&
current.isEqualConfig(target, context.containerIds)
) {
// we're only starting/stopping a service
return this.generateContainerStep(current, target);
}

let strategy =
checkString(target.config.labels['io.balena.update.strategy']) || '';
const validStrategies = [
'download-then-kill',
'kill-then-download',
'delete-then-download',
'hand-over',
];

if (!validStrategies.includes(strategy)) {
strategy = 'download-then-kill';
let strategy: string;
let dependenciesMetForStart: boolean;
if (target != null) {
strategy = getStrategyFromService(target);
dependenciesMetForStart = this.dependenciesMetForServiceStart(
target,
context.targetApp,
context.availableImages,
context.networkPairs,
context.volumePairs,
context.servicePairs,
);
} else {
strategy = getStrategyFromService(current);
dependenciesMetForStart = false;
}

const dependenciesMetForStart = this.dependenciesMetForServiceStart(
target,
context.targetApp,
context.availableImages,
context.networkPairs,
context.volumePairs,
context.servicePairs,
);
const dependenciesMetForKill = this.dependenciesMetForServiceKill(
context.targetApp,
context.availableImages,
Expand Down Expand Up @@ -678,7 +660,10 @@ export class App {
volumePairs: Array<ChangingPair<Volume>>,
servicePairs: Array<ChangingPair<Service>>,
): CompositionStep | undefined {
if (needsDownload) {
if (
needsDownload &&
this.dependenciesMetForServiceFetch(target, servicePairs)
) {
// We know the service name exists as it always does for targets
return generateStep('fetch', {
image: imageManager.imageFromService(target),
Expand All @@ -698,6 +683,37 @@ export class App {
}
}

private dependenciesMetForServiceFetch(
target: Service,
servicePairs: Array<ChangingPair<Service>>,
) {
const [servicePairsWithCurrent, servicePairsWithoutCurrent] = _.partition(
servicePairs,
(pair) => pair.current != null,
);

// Target services not in current can be safely fetched
for (const pair of servicePairsWithoutCurrent) {
if (target.serviceName === pair.target!.serviceName) {
return true;
}
}

// Current services should be killed before target
// service fetch depending on update strategy
for (const pair of servicePairsWithCurrent) {
// Prefer target's update strategy if target service exists
const strategy = getStrategyFromService(pair.target ?? pair.current!);
if (
['kill-then-download', 'delete-then-download'].includes(strategy) &&
pair.current!.config.running
) {
return false;
}
}
return true;
}

// TODO: account for volumes-from, networks-from, links, etc
// TODO: support networks instead of only network mode
private dependenciesMetForServiceStart(
Expand Down Expand Up @@ -751,7 +767,7 @@ export class App {
}

// do not start until all images have been downloaded
return this.targetImagesReady(targetApp, availableImages);
return this.targetImagesReady(targetApp.services, availableImages);
}

// Unless the update strategy requires an early kill (i.e kill-then-download,
Expand All @@ -762,12 +778,14 @@ export class App {
targetApp: App,
availableImages: Image[],
) {
// Don't kill any services before all images have been downloaded
return this.targetImagesReady(targetApp, availableImages);
return this.targetImagesReady(targetApp.services, availableImages);
}

private targetImagesReady(targetApp: App, availableImages: Image[]) {
return targetApp.services.every((service) =>
private targetImagesReady(
targetServices: Service[],
availableImages: Image[],
) {
return targetServices.every((service) =>
availableImages.some(
(image) =>
image.dockerImageId === service.config.image ||
Expand Down
38 changes: 28 additions & 10 deletions src/compose/update-strategies.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import * as imageManager from './images';
import Service from './service';

import { InternalInconsistencyError } from '../lib/errors';
import { CompositionStep, generateStep } from './composition-steps';
import { InternalInconsistencyError } from '../lib/errors';
import { checkString } from '../lib/validation';

export interface StrategyContext {
current: Service;
target: Service;
target?: Service;
needsDownload: boolean;
dependenciesMetForStart: boolean;
dependenciesMetForKill: boolean;
Expand All @@ -19,39 +19,57 @@ export function getStepsFromStrategy(
): CompositionStep {
switch (strategy) {
case 'download-then-kill':
if (context.needsDownload) {
if (context.needsDownload && context.target) {
return generateStep('fetch', {
image: imageManager.imageFromService(context.target),
serviceName: context.target.serviceName!,
serviceName: context.target.serviceName,
});
} else if (context.dependenciesMetForKill) {
// We only kill when dependencies are already met, so that we minimize downtime
return generateStep('kill', { current: context.current });
} else {
return { action: 'noop' };
return generateStep('noop', {});
}
case 'kill-then-download':
case 'delete-then-download':
return generateStep('kill', { current: context.current });
case 'hand-over':
if (context.needsDownload) {
if (context.needsDownload && context.target) {
return generateStep('fetch', {
image: imageManager.imageFromService(context.target),
serviceName: context.target.serviceName!,
serviceName: context.target.serviceName,
});
} else if (context.needsSpecialKill && context.dependenciesMetForKill) {
return generateStep('kill', { current: context.current });
} else if (context.dependenciesMetForStart) {
} else if (context.dependenciesMetForStart && context.target) {
return generateStep('handover', {
current: context.current,
target: context.target,
});
} else {
return { action: 'noop' };
return generateStep('noop', {});
}
default:
throw new InternalInconsistencyError(
`Invalid update strategy: ${strategy}`,
);
}
}

export function getStrategyFromService(svc: Service): string {
let strategy =
checkString(svc.config.labels['io.balena.update.strategy']) || '';

const validStrategies = [
'download-then-kill',
'kill-then-download',
'delete-then-download',
'hand-over',
];

if (!validStrategies.includes(strategy)) {
strategy = 'download-then-kill';
}

return strategy;
}
Loading
Loading