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] Do not await setupIngestManager in plugin start #68631

Closed
wants to merge 11 commits into from
6 changes: 3 additions & 3 deletions x-pack/plugins/ingest_manager/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export type IngestManagerSetup = void;
*/
export interface IngestManagerStart {
registerDatasource: typeof registerDatasource;
success: boolean;
success?: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
success?: boolean;
success?: false | undefined;

Is more accurate for what's happening here. start will not return success: true

error?: {
message: string;
};
Expand Down Expand Up @@ -81,8 +81,8 @@ export class IngestManagerPlugin
try {
const permissionsResponse = await core.http.get(appRoutesService.getCheckPermissionsPath());
if (permissionsResponse.success) {
const { isInitialized: success } = await core.http.post(setupRouteService.getSetupPath());
return { success, registerDatasource };
core.http.post(setupRouteService.getSetupPath());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make the POST to setup to kick off the side-effects, but don't hold up kibana

return { registerDatasource };
} else {
throw new Error(permissionsResponse.error);
}
Expand Down
16 changes: 16 additions & 0 deletions x-pack/plugins/ingest_manager/server/routes/epm/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ import {
removeInstallation,
} from '../../services/epm/packages';

import { appContextService } from '../../services';

export const getCategoriesHandler: RequestHandler = async (context, request, response) => {
try {
const res = await getCategories();
Expand Down Expand Up @@ -125,6 +127,20 @@ export const installPackageHandler: RequestHandler<TypeOf<
const { pkgkey } = request.params;
const savedObjectsClient = context.core.savedObjects.client;
const callCluster = context.core.elasticsearch.legacy.client.callAsCurrentUser;

if (appContextService.getIsInitialized()?.status === 'not_started') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An example of an option for a handler or service.

It can check the initialization status and decide how to proceed. It could do any combination of return nothing, return an error, log the issue, await setupIngestManager, etc

return response.customError({
statusCode: 503,
body: 'Ingest Manager is not initialized',
});
}

if (appContextService.getIsInitialized()?.status === 'error') {
return response.customError({
statusCode: 500,
body: 'There was an error setting up Ingest Manager',
});
}
const res = await installPackage({
savedObjectsClient,
pkgkey,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const ingestManagerSetupHandler: RequestHandler = async (context, request
const logger = appContextService.getLogger();
try {
await setupIngestManager(soClient, callCluster);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change

return response.ok({
body: { isInitialized: true },
});
Expand Down
14 changes: 14 additions & 0 deletions x-pack/plugins/ingest_manager/server/services/app_context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,10 @@ import { IngestManagerConfigType } from '../../common';
import { IngestManagerAppContext } from '../plugin';
import { CloudSetup } from '../../../cloud/server';

interface InitializationState {
status: 'not_started' | 'success' | 'error';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just an example. Could expand to include pending or others depending on what we do. Only success | error seem required.

error?: Error;
}
class AppContextService {
private encryptedSavedObjects: EncryptedSavedObjectsClient | undefined;
private security: SecurityPluginSetup | undefined;
Expand All @@ -23,6 +27,7 @@ class AppContextService {
private cloud?: CloudSetup;
private logger: Logger | undefined;
private httpSetup?: HttpServiceSetup;
private initializationState?: InitializationState;

public async start(appContext: IngestManagerAppContext) {
this.encryptedSavedObjects = appContext.encryptedSavedObjects?.getClient();
Expand All @@ -33,6 +38,7 @@ class AppContextService {
this.logger = appContext.logger;
this.kibanaVersion = appContext.kibanaVersion;
this.httpSetup = appContext.httpSetup;
this.initializationState = { status: 'not_started' };

if (appContext.config$) {
this.config$ = appContext.config$;
Expand Down Expand Up @@ -84,6 +90,14 @@ class AppContextService {
return this.savedObjects;
}

public getIsInitialized() {
return this.initializationState;
}

public setIsInitialized(vals: InitializationState) {
this.initializationState = vals;
Comment on lines +97 to +98
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
public setIsInitialized(vals: InitializationState) {
this.initializationState = vals;
public setIsInitialized(state: InitializationState) {
this.initializationState = state;

}

public getIsProductionMode() {
return this.isProductionMode;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import { SavedObjectsClientContract } from 'kibana/server';
import { getInstallation } from './epm/packages';
import { ESIndexPatternService } from '../../server';
import { CallESAsCurrentUser } from '../types';

export class ESIndexPatternSavedObjectService implements ESIndexPatternService {
public async getESIndexPattern(
savedObjectsClient: SavedObjectsClientContract,
callCluster: CallESAsCurrentUser,
pkgName: string,
datasetPath: string
): Promise<string | undefined> {
Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/ingest_manager/server/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/

import { SavedObjectsClientContract } from 'kibana/server';
import { AgentStatus } from '../types';
import { AgentStatus, CallESAsCurrentUser } from '../types';
import * as settingsService from './settings';
export { ESIndexPatternSavedObjectService } from './es_index_pattern';

Expand All @@ -15,6 +15,7 @@ export { ESIndexPatternSavedObjectService } from './es_index_pattern';
export interface ESIndexPatternService {
getESIndexPattern(
savedObjectsClient: SavedObjectsClientContract,
callCluster: CallESAsCurrentUser,
pkgName: string,
datasetPath: string
): Promise<string | undefined>;
Expand Down
119 changes: 63 additions & 56 deletions x-pack/plugins/ingest_manager/server/services/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,67 +34,74 @@ export async function setupIngestManager(
soClient: SavedObjectsClientContract,
callCluster: CallESAsCurrentUser
) {
const [installedPackages, defaultOutput, config] = await 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.

This is just moving the function body inside a try/catch

// packages installed by default
ensureInstalledDefaultPackages(soClient, callCluster),
outputService.ensureDefaultOutput(soClient),
agentConfigService.ensureDefaultAgentConfig(soClient),
ensureDefaultIndices(callCluster),
settingsService.getSettings(soClient).catch((e: any) => {
if (e.isBoom && e.output.statusCode === 404) {
const http = appContextService.getHttpSetup();
const serverInfo = http.getServerInfo();
const basePath = http.basePath;

const cloud = appContextService.getCloud();
const cloudId = cloud?.isCloudEnabled && cloud.cloudId;
const cloudUrl = cloudId && decodeCloudId(cloudId)?.kibanaUrl;
const flagsUrl = appContextService.getConfig()?.fleet?.kibana?.host;
const defaultUrl = url.format({
protocol: serverInfo.protocol,
hostname: serverInfo.host,
port: serverInfo.port,
pathname: basePath.serverBasePath,
});

return settingsService.saveSettings(soClient, {
agent_auto_upgrade: true,
package_auto_upgrade: true,
kibana_url: cloudUrl || flagsUrl || defaultUrl,
});
}

return Promise.reject(e);
}),
]);

// ensure default packages are added to the default conifg
const configWithDatasource = await agentConfigService.get(soClient, config.id, true);
if (!configWithDatasource) {
throw new Error('Config not found');
}
if (
configWithDatasource.datasources.length &&
typeof configWithDatasource.datasources[0] === 'string'
) {
throw new Error('Config not found');
}
for (const installedPackage of installedPackages) {
const packageShouldBeInstalled = DEFAULT_AGENT_CONFIGS_PACKAGES.some(
(packageName) => installedPackage.name === packageName
);
if (!packageShouldBeInstalled) {
continue;
try {
const [installedPackages, defaultOutput, config] = await Promise.all([
// packages installed by default
ensureInstalledDefaultPackages(soClient, callCluster),
outputService.ensureDefaultOutput(soClient),
agentConfigService.ensureDefaultAgentConfig(soClient),
ensureDefaultIndices(callCluster),
settingsService.getSettings(soClient).catch((e: any) => {
if (e.isBoom && e.output.statusCode === 404) {
const http = appContextService.getHttpSetup();
const serverInfo = http.getServerInfo();
const basePath = http.basePath;

const cloud = appContextService.getCloud();
const cloudId = cloud?.isCloudEnabled && cloud.cloudId;
const cloudUrl = cloudId && decodeCloudId(cloudId)?.kibanaUrl;
const flagsUrl = appContextService.getConfig()?.fleet?.kibana?.host;
const defaultUrl = url.format({
protocol: serverInfo.protocol,
hostname: serverInfo.host,
port: serverInfo.port,
pathname: basePath.serverBasePath,
});

return settingsService.saveSettings(soClient, {
agent_auto_upgrade: true,
package_auto_upgrade: true,
kibana_url: cloudUrl || flagsUrl || defaultUrl,
});
}

return Promise.reject(e);
}),
]);

// ensure default packages are added to the default conifg
const configWithDatasource = await agentConfigService.get(soClient, config.id, true);
if (!configWithDatasource) {
throw new Error('Config not found');
}
if (
configWithDatasource.datasources.length &&
typeof configWithDatasource.datasources[0] === 'string'
) {
throw new Error('Config not found');
}
for (const installedPackage of installedPackages) {
const packageShouldBeInstalled = DEFAULT_AGENT_CONFIGS_PACKAGES.some(
(packageName) => installedPackage.name === packageName
);
if (!packageShouldBeInstalled) {
continue;
}

const isInstalled = configWithDatasource.datasources.some((d: Datasource | string) => {
return typeof d !== 'string' && d.package?.name === installedPackage.name;
});
const isInstalled = configWithDatasource.datasources.some((d: Datasource | string) => {
return typeof d !== 'string' && d.package?.name === installedPackage.name;
});

if (!isInstalled) {
await addPackageToConfig(soClient, installedPackage, configWithDatasource, defaultOutput);
if (!isInstalled) {
await addPackageToConfig(soClient, installedPackage, configWithDatasource, defaultOutput);
}
}
} catch (e) {
appContextService.setIsInitialized({ status: 'error', error: e });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Something in setupIngestManager failed. Track the status & error for others

}

appContextService.setIsInitialized({ status: 'success' });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy path

return;
}

export async function setupFleet(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export class IngestIndexPatternRetriever implements IndexPatternRetriever {
try {
const pattern = await this.service.getESIndexPattern(
ctx.core.savedObjects.client,
ctx.core.elasticsearch.legacy.client.callAsCurrentUser,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IngestIndexPatternRetriever.endpointPackageName,
datasetPath
);
Expand Down