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

[Fleet] Fix isPackageVersionOrLaterInstalled to check for installed package #176532

Merged
merged 3 commits into from
Feb 9, 2024
Merged

[Fleet] Fix isPackageVersionOrLaterInstalled to check for installed package #176532

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
374b5ef
[Fleet] Fix isPackageVersionOrLaterInstalled to make sure package is …
nchaulet Feb 8, 2024
be35eb8
add tests and refacto
nchaulet Feb 8, 2024
81f4125
Merge branch 'main' into feature-fix-isPackageVersionOrLaterInstalled
kibanamachine Feb 8, 2024
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
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/services/epm/packages/bulk_install_packages.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export async function bulkInstallPackages({
result: {
assets: [...installedEs, ...installedKibana],
status: 'already_installed',
installType: installedPackageResult.installType,
installType: 'unknown',
} as InstallResult,
};
}
Expand Down
108 changes: 108 additions & 0 deletions x-pack/plugins/fleet/server/services/epm/packages/install.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
handleInstallPackageFailure,
installAssetsForInputPackagePolicy,
installPackage,
isPackageVersionOrLaterInstalled,
} from './install';
import * as install from './_install_package';
import { getBundledPackageByPkgKey } from './bundled_packages';
Expand Down Expand Up @@ -715,3 +716,110 @@ describe('handleInstallPackageFailure', () => {
);
});
});

describe('isPackageVersionOrLaterInstalled', () => {
beforeEach(() => {
jest.mocked(getInstallationObject).mockReset();
});
it('should return true if package is installed in the same version as expected', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
jest.mocked(getInstallationObject).mockResolvedValueOnce({
attributes: { name: 'test', version: '1.0.0', install_status: 'installed' },
} as any);
const res = await isPackageVersionOrLaterInstalled({
savedObjectsClient,
pkgName: 'test',
pkgVersion: '1.0.0',
});

expect(res).toEqual(
expect.objectContaining({
package: expect.objectContaining({
name: 'test',
version: '1.0.0',
install_status: 'installed',
}),
})
);
});

it('should return true if package is installed in an higher version as expected', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
jest.mocked(getInstallationObject).mockResolvedValueOnce({
attributes: { name: 'test', version: '1.2.0', install_status: 'installed' },
} as any);
const res = await isPackageVersionOrLaterInstalled({
savedObjectsClient,
pkgName: 'test',
pkgVersion: '1.0.0',
});

expect(res).toEqual(
expect.objectContaining({
package: expect.objectContaining({
name: 'test',
version: '1.2.0',
install_status: 'installed',
}),
})
);
});

it('should return false if package is installed in an lower version as expected', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
jest.mocked(getInstallationObject).mockResolvedValueOnce({
attributes: { name: 'test', version: '0.9.0', install_status: 'installed' },
} as any);
const res = await isPackageVersionOrLaterInstalled({
savedObjectsClient,
pkgName: 'test',
pkgVersion: '1.0.0',
});

expect(res).toEqual(false);
});

it('should retry if package is currently installing', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
jest.mocked(getInstallationObject).mockResolvedValueOnce({
attributes: { name: 'test', version: '1.0.0', install_status: 'installing' },
} as any);
jest.mocked(getInstallationObject).mockResolvedValueOnce({
attributes: { name: 'test', version: '1.0.0', install_status: 'installing' },
} as any);
jest.mocked(getInstallationObject).mockResolvedValueOnce({
attributes: { name: 'test', version: '1.0.0', install_status: 'installed' },
} as any);

const res = await isPackageVersionOrLaterInstalled({
savedObjectsClient,
pkgName: 'test',
pkgVersion: '1.0.0',
});

expect(res).toEqual(
expect.objectContaining({
package: expect.objectContaining({
name: 'test',
version: '1.0.0',
install_status: 'installed',
}),
})
);

expect(getInstallationObject).toBeCalledTimes(3);
});

it('should throw on unexpected error', async () => {
const savedObjectsClient = savedObjectsClientMock.create();
jest.mocked(getInstallationObject).mockRejectedValueOnce(new Error('test unexpected error'));

const res = isPackageVersionOrLaterInstalled({
savedObjectsClient,
pkgName: 'test',
pkgVersion: '1.0.0',
});

await expect(res).rejects.toThrowError('test unexpected error');
});
});
61 changes: 45 additions & 16 deletions x-pack/plugins/fleet/server/services/epm/packages/install.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ import { addErrorToLatestFailedAttempts } from './install_errors_helpers';
import { installIndexTemplatesAndPipelines } from './install_index_template_pipeline';
import { optimisticallyAddEsAssetReferences } from './es_assets_reference';

const MAX_ENSURE_INSTALL_TIME = 60 * 1000;

export async function isPackageInstalled(options: {
savedObjectsClient: SavedObjectsClientContract;
pkgName: string;
Expand All @@ -95,27 +97,53 @@ export async function isPackageInstalled(options: {
return installedPackage !== undefined;
}

// Error used to retry in isPackageVersionOrLaterInstalled
class CurrentlyInstallingError extends Error {}

/**
* Check if a package is currently installed,
* if the package is currently installing it will retry until MAX_ENSURE_INSTALL_TIME is reached
*/
export async function isPackageVersionOrLaterInstalled(options: {
savedObjectsClient: SavedObjectsClientContract;
pkgName: string;
pkgVersion: string;
}): Promise<{ package: Installation; installType: InstallType } | false> {
const { savedObjectsClient, pkgName, pkgVersion } = options;
const installedPackageObject = await getInstallationObject({ savedObjectsClient, pkgName });
const installedPackage = installedPackageObject?.attributes;
if (
installedPackage &&
(installedPackage.version === pkgVersion || semverLt(pkgVersion, installedPackage.version))
) {
let installType: InstallType;
try {
installType = getInstallType({ pkgVersion, installedPkg: installedPackageObject });
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the installType here as it seems not used by that function consumers, and it seems that function should not return it as it's not creating an install but checking if one exist

} catch (e) {
installType = 'unknown';
}): Promise<{ package: Installation } | false> {
return pRetry(
async () => {
const { savedObjectsClient, pkgName, pkgVersion } = options;
const installedPackageObject = await getInstallationObject({ savedObjectsClient, pkgName });
const installedPackage = installedPackageObject?.attributes;
if (
installedPackage &&
(installedPackage.version === pkgVersion || semverLt(pkgVersion, installedPackage.version))
) {
if (installedPackage.install_status === 'installing') {
throw new CurrentlyInstallingError(
`Package ${pkgName}-${pkgVersion} is currently installing`
);
} else if (installedPackage.install_status === 'install_failed') {
return false;
}

return { package: installedPackage };
}
return false;
},
{
maxRetryTime: MAX_ENSURE_INSTALL_TIME,
onFailedAttempt: (error) => {
if (!(error instanceof CurrentlyInstallingError)) {
throw error;
}
},
}
return { package: installedPackage, installType };
}
return false;
).catch((err): false => {
if (err instanceof CurrentlyInstallingError) {
return false;
}
throw err;
});
}

export async function ensureInstalledPackage(options: {
Expand Down Expand Up @@ -147,6 +175,7 @@ export async function ensureInstalledPackage(options: {
pkgName: pkgKeyProps.name,
pkgVersion: pkgKeyProps.version,
});

if (installedPackageResult) {
return installedPackageResult.package;
}
Expand Down