Skip to content

Commit

Permalink
[Fleet] Don't fail on errors in 'update' or 'reupdate' operation in /…
Browse files Browse the repository at this point in the history
…setup (#97404)

* Don't fail on, just report, update and reupdate errors.

* Show error toast on update and reupdate errors.

* Don't return empty error array.

* Adjust mock.

* Adjust test.
  • Loading branch information
skh authored Apr 19, 2021
1 parent 17ecb04 commit efcf7d1
Show file tree
Hide file tree
Showing 9 changed files with 72 additions and 14 deletions.
5 changes: 5 additions & 0 deletions x-pack/plugins/fleet/common/types/models/epm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ export enum InstallStatus {
uninstalling = 'uninstalling',
}

export interface DefaultPackagesInstallationError {
installType: InstallType;
error: Error;
}

export type InstallType = 'reinstall' | 'reupdate' | 'rollback' | 'update' | 'install' | 'unknown';
export type InstallSource = 'registry' | 'upload';

Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/fleet/common/types/rest_spec/ingest_setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
* 2.0.
*/

import type { DefaultPackagesInstallationError } from '../models/epm';

export interface PostIngestSetupResponse {
isInitialized: boolean;
preconfigurationError?: { name: string; message: string };
nonFatalPackageUpgradeErrors?: DefaultPackagesInstallationError[];
}
7 changes: 7 additions & 0 deletions x-pack/plugins/fleet/public/applications/fleet/app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,13 @@ export const WithPermissionsAndSetup: React.FC = memo(({ children }) => {
}),
});
}
if (setupResponse.data.nonFatalPackageUpgradeErrors) {
notifications.toasts.addError(setupResponse.data.nonFatalPackageUpgradeErrors, {
title: i18n.translate('xpack.fleet.setup.nonFatalPackageErrorsTitle', {
defaultMessage: 'One or more packages could not be successfully upgraded',
}),
});
}
} catch (err) {
setInitializationError(err);
}
Expand Down
6 changes: 5 additions & 1 deletion x-pack/plugins/fleet/server/routes/setup/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ describe('FleetSetupHandler', () => {

it('POST /setup succeeds w/200 and body of resolved value', async () => {
mockSetupIngestManager.mockImplementation(() =>
Promise.resolve({ isInitialized: true, preconfigurationError: undefined })
Promise.resolve({
isInitialized: true,
preconfigurationError: undefined,
nonFatalPackageUpgradeErrors: [],
})
);
await fleetSetupHandler(context, request, response);

Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/fleet/server/routes/setup/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,14 @@ export const fleetSetupHandler: RequestHandler = async (context, request, respon
try {
const soClient = context.core.savedObjects.client;
const esClient = context.core.elasticsearch.client.asCurrentUser;
const body: PostIngestSetupResponse = { isInitialized: true };
await setupIngestManager(soClient, esClient);
const setupStatus = await setupIngestManager(soClient, esClient);
const body: PostIngestSetupResponse = {
isInitialized: true,
};

if (setupStatus.nonFatalPackageUpgradeErrors.length > 0) {
body.nonFatalPackageUpgradeErrors = setupStatus.nonFatalPackageUpgradeErrors;
}

return response.ok({
body,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ export async function bulkInstallPackages({
skipPostInstall: true,
});
if (installResult.error) {
return { name: packageName, error: installResult.error };
return {
name: packageName,
error: installResult.error,
installType: installResult.installType,
};
} else {
return {
name: packageName,
Expand Down Expand Up @@ -75,7 +79,11 @@ export async function bulkInstallPackages({
const packageName = packagesToInstall[index];
if (result.status === 'fulfilled') {
if (result.value && result.value.error) {
return { name: packageName, error: result.value.error };
return {
name: packageName,
error: result.value.error,
installType: result.value.installType,
};
} else {
return result.value;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ describe('ensureInstalledDefaultPackages', () => {
];
});
const resp = await ensureInstalledDefaultPackages(soClient, jest.fn());
expect(resp).toEqual([mockInstallation.attributes]);
expect(resp.installations).toEqual([mockInstallation.attributes]);
});
it('should throw the first Error it finds', async () => {
class SomeCustomError extends Error {}
Expand Down
28 changes: 24 additions & 4 deletions x-pack/plugins/fleet/server/services/epm/packages/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ import type { ElasticsearchClient, SavedObject, SavedObjectsClientContract } fro
import { generateESIndexPatterns } from '../elasticsearch/template/template';

import { defaultPackages } from '../../../../common';
import type { BulkInstallPackageInfo, InstallablePackage, InstallSource } from '../../../../common';
import type {
BulkInstallPackageInfo,
InstallablePackage,
InstallSource,
DefaultPackagesInstallationError,
} from '../../../../common';
import {
IngestManagerError,
PackageOperationNotSupportedError,
Expand Down Expand Up @@ -45,11 +50,17 @@ import { removeInstallation } from './remove';
import { getPackageSavedObjects } from './get';
import { _installPackage } from './_install_package';

export interface DefaultPackagesInstallationResult {
installations: Installation[];
nonFatalPackageUpgradeErrors: DefaultPackagesInstallationError[];
}

export async function ensureInstalledDefaultPackages(
savedObjectsClient: SavedObjectsClientContract,
esClient: ElasticsearchClient
): Promise<Installation[]> {
): Promise<DefaultPackagesInstallationResult> {
const installations = [];
const nonFatalPackageUpgradeErrors = [];
const bulkResponse = await bulkInstallPackages({
savedObjectsClient,
packagesToInstall: Object.values(defaultPackages),
Expand All @@ -58,19 +69,27 @@ export async function ensureInstalledDefaultPackages(

for (const resp of bulkResponse) {
if (isBulkInstallError(resp)) {
throw resp.error;
if (resp.installType && (resp.installType === 'update' || resp.installType === 'reupdate')) {
nonFatalPackageUpgradeErrors.push({ installType: resp.installType, error: resp.error });
} else {
throw resp.error;
}
} else {
installations.push(getInstallation({ savedObjectsClient, pkgName: resp.name }));
}
}

const retrievedInstallations = await Promise.all(installations);
return retrievedInstallations.map((installation, index) => {
const verifiedInstallations = retrievedInstallations.map((installation, index) => {
if (!installation) {
throw new Error(`could not get installation ${bulkResponse[index].name}`);
}
return installation;
});
return {
installations: verifiedInstallations,
nonFatalPackageUpgradeErrors,
};
}

async function isPackageVersionOrLaterInstalled(options: {
Expand Down Expand Up @@ -181,6 +200,7 @@ export async function handleInstallPackageFailure({
export interface IBulkInstallPackageError {
name: string;
error: Error;
installType?: InstallType;
}
export type BulkInstallResponse = BulkInstallPackageInfo | IBulkInstallPackageError;

Expand Down
13 changes: 9 additions & 4 deletions x-pack/plugins/fleet/server/services/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { i18n } from '@kbn/i18n';

import { DEFAULT_AGENT_POLICIES_PACKAGES, FLEET_SERVER_PACKAGE } from '../../common';

import type { PackagePolicy } from '../../common';
import type { PackagePolicy, DefaultPackagesInstallationError } from '../../common';

import { SO_SEARCH_LIMIT } from '../constants';

Expand All @@ -33,6 +33,7 @@ import { awaitIfFleetServerSetupPending } from './fleet_server';
export interface SetupStatus {
isInitialized: boolean;
preconfigurationError: { name: string; message: string } | undefined;
nonFatalPackageUpgradeErrors: DefaultPackagesInstallationError[];
}

export async function setupIngestManager(
Expand All @@ -46,7 +47,7 @@ async function createSetupSideEffects(
soClient: SavedObjectsClientContract,
esClient: ElasticsearchClient
): Promise<SetupStatus> {
const [installedPackages, defaultOutput] = await Promise.all([
const [defaultPackagesResult, defaultOutput] = await Promise.all([
// packages installed by default
ensureInstalledDefaultPackages(soClient, esClient),
outputService.ensureDefaultOutput(soClient),
Expand Down Expand Up @@ -142,7 +143,7 @@ async function createSetupSideEffects(
);
}

for (const installedPackage of installedPackages) {
for (const installedPackage of defaultPackagesResult.installations) {
const packageShouldBeInstalled = DEFAULT_AGENT_POLICIES_PACKAGES.some(
(packageName) => installedPackage.name === packageName
);
Expand Down Expand Up @@ -172,7 +173,11 @@ async function createSetupSideEffects(

await ensureAgentActionPolicyChangeExists(soClient, esClient);

return { isInitialized: true, preconfigurationError };
return {
isInitialized: true,
preconfigurationError,
nonFatalPackageUpgradeErrors: defaultPackagesResult.nonFatalPackageUpgradeErrors,
};
}

export async function ensureDefaultEnrollmentAPIKeysExists(
Expand Down

0 comments on commit efcf7d1

Please sign in to comment.