ExternalVendorCode/balena-supervisor
1
0
Fork 0
mirror of https://github.com/balena-os/balena-supervisor.git synced 2024-12-19 13:47:54 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Simplify getting images for cleanup

Browse Source 
getImagesForCleanup used to query the Engine for the Supervisor
image, which is unnecessary given that the Supervisor has access
to constants.supervisorImage. Thus, this Engine query is removed.

The method is simplified and made more clear, and
imageManager.isCleanupNeeded doesn't need to be stubbed in tests.

Change-type: patch
Signed-off-by: Christina Ying Wang <christina@balena.io>
This commit is contained in:
Christina Ying Wang 2022-11-15 19:26:41 -08:00
parent 631e560137
commit 8174ea9643
2 changed files with 70 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
src/compose/images.ts
View File

@ -465,72 +465,71 @@ export const save = async (image: Image): Promise<void> => {
await markAsSupervised(image);
};
async function getImagesForCleanup(): Promise<string[]> {
const images: string[] = [];
// TODO: remove after we agree on what to do for Supervisor image cleanup after hup
const getSupervisorRepos = (imageName: string) => {
const supervisorRepos = new Set<string>();
supervisorRepos.add(imageName);
// If we're on the new balena/ARCH-supervisor image, add legacy image.
// If the image name is legacy, the `replace` will have no effect.
supervisorRepos.add(imageName.replace(/^balena/, 'resin'));
return [...supervisorRepos];
};
const supervisorImageInfo = dockerUtils.getRegistryAndName(
// TODO: same as above, we no longer use tags to identify supervisors
const isSupervisorRepoTag = ({
repoTag,
svRepos,
svTag,
}: {
repoTag: string;
svRepos: ReturnType<typeof getSupervisorRepos>;
svTag?: string;
}) => {
const { imageName, tagName } = dockerUtils.getRegistryAndName(repoTag);
return svRepos.some((repo) => imageName === repo) && tagName !== svTag;
};
async function getImagesForCleanup(): Promise<Array<Docker.ImageInfo['Id']>> {
// Get Supervisor image metadata for exclusion from image cleanup
const { imageName: svImage, tagName: svTag } = dockerUtils.getRegistryAndName(
constants.supervisorImage,
);
const [supervisorImage, usedImageIds] = await Promise.all([
docker.getImage(constants.supervisorImage).inspect(),
db
.models('image')
.select('dockerImageId')
.then((vals) => vals.map((img: Image) => img.dockerImageId)),
]);
const svRepos = getSupervisorRepos(svImage);
// TODO: remove after we agree on what to do for
// supervisor image cleanup after hup
const supervisorRepos = [supervisorImageInfo.imageName];
// If we're on the new balena/ARCH-supervisor image
if (_.startsWith(supervisorImageInfo.imageName, 'balena/')) {
supervisorRepos.push(
supervisorImageInfo.imageName.replace(/^balena/, 'resin'),
);
}
// TODO: same as above, we no longer use tags to identify supervisors
const isSupervisorRepoTag = ({
imageName,
tagName,
}: {
imageName: string;
tagName?: string;
}) => {
return (
_.some(supervisorRepos, (repo) => imageName === repo) &&
tagName !== supervisorImageInfo.tagName
);
};
const usedImageIds: string[] = await db
.models('image')
.select('dockerImageId')
.then((vals) => vals.map(({ dockerImageId }: Image) => dockerImageId));
const dockerImages = await docker.listImages({ digests: true });
const imagesToCleanup = new Set<Docker.ImageInfo['Id']>();
for (const image of dockerImages) {
// Cleanup should remove truly dangling images (i.e dangling and with no digests)
if (isDangling(image) && !_.includes(usedImageIds, image.Id)) {
images.push(image.Id);
} else if (!_.isEmpty(image.RepoTags) && image.Id !== supervisorImage.Id) {
// We also remove images from the supervisor repository with a different tag
for (const tag of image.RepoTags) {
const imageNameComponents = dockerUtils.getRegistryAndName(tag);
// If
if (isSupervisorRepoTag(imageNameComponents)) {
images.push(image.Id);
}
if (isDangling(image) && !usedImageIds.includes(image.Id)) {
imagesToCleanup.add(image.Id);
continue;
}
// We also remove images from the Supervisor repository with a different tag
for (const repoTag of image.RepoTags || []) {
if (isSupervisorRepoTag({ repoTag, svRepos, svTag })) {
imagesToCleanup.add(image.Id);
}
}
}
return _(images)
.uniq()
.filter(
(image) =>
imageCleanupFailures[image] == null ||
Date.now() - imageCleanupFailures[image] >
constants.imageCleanupErrorIgnoreTimeout,
)
.value();
return [...imagesToCleanup].filter(
(image) =>
imageCleanupFailures[image] == null ||
Date.now() - imageCleanupFailures[image] >
constants.imageCleanupErrorIgnoreTimeout,
);
}
export const isCleanupNeeded = async () =>
(await getImagesForCleanup()).length > 0;
// Look for an image in the engine with registry/image as reference (tag)
// for images with deltas this should return unless there is some inconsistency
// and the tag was deleted.
@ -606,10 +605,6 @@ export async function inspectByName(imageName: string) {
);
}
export async function isCleanupNeeded() {
return !_.isEmpty(await getImagesForCleanup());
}
export async function cleanup() {
const images = await getImagesForCleanup();
for (const image of images) {

33
test/integration/compose/application-manager.spec.ts
View File

@ -1,6 +1,5 @@
import { expect } from 'chai';
import * as sinon from 'sinon';
import { stub } from 'sinon';
import * as Docker from 'dockerode';
import App from '~/src/compose/app';
import * as applicationManager from '~/src/compose/application-manager';
@ -14,6 +13,7 @@ import { ServiceComposeConfig } from '~/src/compose/types/service';
import Volume from '~/src/compose/volume';
import { InstancedAppState } from '~/src/types/state';
import * as config from '~/src/config';
import { createDockerImage } from '~/test-lib/docker-helper';
const DEFAULT_NETWORK = Network.fromComposeObject('default', 1, 'appuuid', {});
@ -171,9 +171,6 @@ function createCurrentState({
// the app spec, remove that redundancy to simplify the tests
describe('compose/application-manager', () => {
before(async () => {
// Stub methods that depend on external dependencies
stub(imageManager, 'isCleanupNeeded');
// Service.fromComposeObject gets api keys from the database
// which also depend on the local mode. This ensures the database
// is initialized. This can be removed when ApplicationManager and Service
@ -182,17 +179,10 @@ describe('compose/application-manager', () => {
});
beforeEach(async () => {
// Do not check for cleanup images by default
(imageManager.isCleanupNeeded as sinon.SinonStub).resolves(false);
// Set up network by default
await networkManager.ensureSupervisorNetwork();
});
after(() => {
// Restore stubs
(imageManager.isCleanupNeeded as sinon.SinonStub).restore();
});
afterEach(async () => {
// Delete any created networks
const docker = new Docker();
@ -927,8 +917,21 @@ describe('compose/application-manager', () => {
});
it('should infer a cleanup step when a cleanup is required', async () => {
// Stub the image manager function
(imageManager.isCleanupNeeded as sinon.SinonStub).resolves(true);
// Create a dangling image; this is done by building an image again with
// some slightly different metadata, leaving the old image with no metadata.
const docker = new Docker();
const dockerImageIdOne = await createDockerImage(
'some-image:some-tag',
['io.balena.testing=1'],
docker,
);
const dockerImageIdTwo = await createDockerImage(
'some-image:some-tag',
['io.balena.testing=2'],
docker,
);
// Remove the tagged image, leaving only the dangling image
await docker.getImage(dockerImageIdTwo).remove();
const targetApps = createApps(
{
@ -958,6 +961,8 @@ describe('compose/application-manager', () => {
action: 'cleanup',
});
expect(nextSteps).to.have.lengthOf(0);
await docker.getImage(dockerImageIdOne).remove();
});
it('should infer that an image should be removed if it is no longer referenced in current or target state (only target)', async () => {
@ -1188,7 +1193,7 @@ describe('compose/application-manager', () => {
).to.have.lengthOf(1);
});
describe('getting applications current state', () => {
describe("getting application's current state", () => {
let getImagesState: sinon.SinonStub;
let getServicesState: sinon.SinonStub;