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.

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

[Ingest Manager] Better handling of package installation problems #66541

Merged
merged 6 commits into from
May 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 10 additions & 0 deletions x-pack/plugins/ingest_manager/server/routes/setup/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,22 @@ export const createFleetSetupHandler: RequestHandler<
export const ingestManagerSetupHandler: RequestHandler = async (context, request, response) => {
const soClient = context.core.savedObjects.client;
const callCluster = context.core.elasticsearch.adminClient.callAsCurrentUser;
const logger = appContextService.getLogger();
try {
await setupIngestManager(soClient, callCluster);
return response.ok({
body: { isInitialized: true },
});
} catch (e) {
if (e.isBoom) {
logger.error(e.output.payload.message);
return response.customError({
statusCode: e.output.statusCode,
body: { message: e.output.payload.message },
});
}
logger.error(e.message);
logger.error(e.stack);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@neptunian This gives us a stack trace from the handler that's actually pointing at the real location the error occurred. I only added this here for "unknown" errors.

return response.customError({
statusCode: 500,
body: { message: e.message },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import Boom from 'boom';
import { Dataset, RegistryPackage, ElasticsearchAssetType, TemplateRef } from '../../../../types';
import { CallESAsCurrentUser } from '../../../../types';
import { Field, loadFieldsFromYaml, processFields } from '../../fields/field';
Expand All @@ -20,8 +21,8 @@ export const installTemplates = async (
// install any pre-built index template assets,
// atm, this is only the base package's global index templates
// Install component templates first, as they are used by the index templates
installPreBuiltComponentTemplates(pkgName, pkgVersion, callCluster);
installPreBuiltTemplates(pkgName, pkgVersion, callCluster);
await installPreBuiltComponentTemplates(pkgName, pkgVersion, callCluster);
Copy link
Member

Choose a reason for hiding this comment

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

Can we do this two operation concurently inside a Promise.all

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The second depends on the first being completed, so we can't do them concurrently.

await installPreBuiltTemplates(pkgName, pkgVersion, callCluster);

// build templates per dataset from yml files
const datasets = registryPackage.datasets;
Expand Down Expand Up @@ -53,16 +54,7 @@ const installPreBuiltTemplates = async (
pkgVersion,
(entry: Registry.ArchiveEntry) => isTemplate(entry)
);
// templatePaths.forEach(async path => {
// const { file } = Registry.pathParts(path);
// const templateName = file.substr(0, file.lastIndexOf('.'));
// const content = JSON.parse(Registry.getAsset(path).toString('utf8'));
// await callCluster('indices.putTemplate', {
// name: templateName,
// body: content,
// });
// });
templatePaths.forEach(async path => {
const templateInstallPromises = templatePaths.map(async path => {
const { file } = Registry.pathParts(path);
const templateName = file.substr(0, file.lastIndexOf('.'));
const content = JSON.parse(Registry.getAsset(path).toString('utf8'));
Expand Down Expand Up @@ -91,8 +83,15 @@ const installPreBuiltTemplates = async (
// The existing convenience endpoint `indices.putTemplate` only sends to _template,
// which does not support v2 templates.
// See src/core/server/elasticsearch/api_types.ts for available endpoints.
await callCluster('transport.request', callClusterParams);
return callCluster('transport.request', callClusterParams);
});
try {
return await Promise.all(templateInstallPromises);
} catch (e) {
throw new Boom(`Error installing prebuilt index templates ${e.message}`, {
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to start to introduce our domain errors and avoid using Boom ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would prefer to start with that in changes that don't go into 7.8 to keep the changes smaller.

statusCode: 400,
});
}
};

const installPreBuiltComponentTemplates = async (
Expand All @@ -105,7 +104,7 @@ const installPreBuiltComponentTemplates = async (
pkgVersion,
(entry: Registry.ArchiveEntry) => isComponentTemplate(entry)
);
templatePaths.forEach(async path => {
const templateInstallPromises = templatePaths.map(async path => {
const { file } = Registry.pathParts(path);
const templateName = file.substr(0, file.lastIndexOf('.'));
const content = JSON.parse(Registry.getAsset(path).toString('utf8'));
Expand All @@ -124,8 +123,15 @@ const installPreBuiltComponentTemplates = async (
// This uses the catch-all endpoint 'transport.request' because there is no
// convenience endpoint for component templates yet.
// See src/core/server/elasticsearch/api_types.ts for available endpoints.
await callCluster('transport.request', callClusterParams);
return callCluster('transport.request', callClusterParams);
});
try {
return await Promise.all(templateInstallPromises);
} catch (e) {
throw new Boom(`Error installing prebuilt component templates ${e.message}`, {
statusCode: 400,
});
}
};

const isTemplate = ({ path }: Registry.ArchiveEntry) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,17 +73,13 @@ export async function ensureInstalledPackage(options: {
if (installedPackage) {
return installedPackage;
}
// if the requested packaged was not found to be installed, try installing
try {
await installLatestPackage({
savedObjectsClient,
pkgName,
callCluster,
});
return await getInstallation({ savedObjectsClient, pkgName });
} catch (err) {
throw new Error(err.message);
}
// if the requested packaged was not found to be installed, install
await installLatestPackage({
savedObjectsClient,
pkgName,
callCluster,
});
return await getInstallation({ savedObjectsClient, pkgName });
}

export async function installPackage(options: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export async function getResponse(url: string): Promise<Response> {
throw new Boom(response.statusText, { statusCode: response.status });
}
} catch (e) {
throw Boom.boomify(e);
throw new Boom(`Error connecting to package registry: ${e.message}`, { statusCode: 502 });
}
}

Expand Down