Skip to content

Commit

Permalink
[ML] Fix flaky update_groups api test (elastic#161326)
Browse files Browse the repository at this point in the history
Related to elastic#161324 and
elastic#160370
Flaky test runner
https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/2649

I believe the problem lies with the function `cleanMLSavedObjects` only
cleaning up saved objects in the default space and not in any other of
the spaces which jobs or trained models may have been added to.
This causes an intermittent clash where a job's saved object already
exists, but is in a different space. I don't know why this doesn't fail
on every run.
The fix is to update `cleanMLSavedObjects` so it can take a list of
additional space IDs to also clean. Any test which adds jobs or trained
models to spaces other than `default` need to call this function and
supply the list of space IDs it is using.
I've updated every test I could find in this PR.
  • Loading branch information
jgowdyelastic authored and Devon Thomson committed Aug 1, 2023
1 parent ebd47eb commit 2e3adf4
Show file tree
Hide file tree
Showing 18 changed files with 41 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default ({ getService }: FtrProviderContext) => {
afterEach(async () => {
await ml.api.closeAnomalyDetectionJob(jobIdSpace1);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ export default ({ getService }: FtrProviderContext) => {

afterEach(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default ({ getService }: FtrProviderContext) => {
afterEach(async () => {
await ml.api.closeAnomalyDetectionJob(jobIdSpace1);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default ({ getService }: FtrProviderContext) => {

afterEach(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export default ({ getService }: FtrProviderContext) => {
await ml.api.closeAnomalyDetectionJob(jobIdSpace1);
await ml.api.closeAnomalyDetectionJob(jobIdSpace2);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export default ({ getService }: FtrProviderContext) => {

afterEach(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export default ({ getService }: FtrProviderContext) => {
await ml.api.deleteAnomalyDetectionJobES(jobIdSpace1);
await ml.api.deleteAnomalyDetectionJobES(jobIdSpace2);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
4 changes: 4 additions & 0 deletions x-pack/test/api_integration/apis/ml/jobs/jobs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default ({ getService }: FtrProviderContext) => {
const esArchiver = getService('esArchiver');
const supertest = getService('supertestWithoutAuth');
const ml = getService('ml');
const spacesService = getService('spaces');

const idSpace1 = 'space1';

Expand Down Expand Up @@ -95,6 +96,7 @@ export default ({ getService }: FtrProviderContext) => {
before(async () => {
await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/ml/farequote');
await ml.testResources.setKibanaTimeZoneToUTC();
await spacesService.create({ id: idSpace1, name: 'space_one', disabledFeatures: [] });

for (const job of testSetupJobConfigs) {
await ml.api.createAnomalyDetectionJob(job);
Expand Down Expand Up @@ -123,7 +125,9 @@ export default ({ getService }: FtrProviderContext) => {
});

after(async () => {
await spacesService.delete(idSpace1);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
});

it('returns expected list of combined jobs with stats in default space', async () => {
Expand Down
3 changes: 1 addition & 2 deletions x-pack/test/api_integration/apis/ml/jobs/reset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ export default ({ getService }: FtrProviderContext) => {
[MULTI_METRIC_JOB_CONFIG.job_id]: { reset: true, task: 'cannot be predicted' },
};

// Failing: See https://github.com/elastic/kibana/issues/160370
describe.skip('reset_jobs', function () {
describe('reset_jobs', function () {
before(async () => {
await esArchiver.loadIfNeeded('x-pack/test/functional/es_archives/ml/farequote');
await ml.testResources.createIndexPatternIfNeeded('ft_farequote', '@timestamp');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export default ({ getService }: FtrProviderContext) => {
await ml.api.deleteAnomalyDetectionJobES(jobIdSpace1);
await ml.api.deleteAnomalyDetectionJobES(jobIdSpace2);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
1 change: 1 addition & 0 deletions x-pack/test/api_integration/apis/ml/jobs/update_groups.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export default ({ getService }: FtrProviderContext) => {

after(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
});

it('returns expected list of groups after update', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ export default ({ getService }: FtrProviderContext) => {
});

after(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await spacesService.delete(idSpace1);
await spacesService.delete(idSpace2);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
});

it('should see results in current space', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ export default ({ getService }: FtrProviderContext) => {
});

after(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await spacesService.delete(idSpace1);
await spacesService.delete(idSpace2);
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
});

it('should remove AD job from current space', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export default ({ getService }: FtrProviderContext) => {

afterEach(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export default ({ getService }: FtrProviderContext) => {

afterEach(async () => {
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.cleanMLSavedObjects([idSpace1, idSpace2]);
});

after(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await ml.api.deleteAnomalyDetectionJobES(jobId);
}
await spacesService.delete(idSpace1);
await ml.testResources.cleanMLSavedObjects();
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects();
await ml.testResources.deleteIndexPatternByTitle('ft_farequote');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ export default function ({ getService }: FtrProviderContext) {
await spacesService.delete(spaceId);
}
}
await ml.testResources.cleanMLSavedObjects();
await ml.api.cleanMlIndices();
await ml.testResources.cleanMLSavedObjects([spaceIds.idSpace1]);
await ml.testResources.deleteIndexPatternByTitle('ft_farequote');
await ml.testResources.deleteIndexPatternByTitle('ft_ihp_outlier');
});
Expand Down
28 changes: 19 additions & 9 deletions x-pack/test/functional/services/ml/test_resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,14 @@ export function MachineLearningTestResourcesProvider(
log.debug(` > Not found`);
},

async getSavedObjectIdsByType(objectType: SavedObjectType): Promise<string[]> {
async getSavedObjectIdsByType(objectType: SavedObjectType, space?: string): Promise<string[]> {
const savedObjectIds: string[] = [];

log.debug(`Searching for '${objectType}' ...`);
const { body: findResponse, status } = await supertest
.get(`/api/saved_objects/_find?type=${objectType}&per_page=10000`)
.get(
`${space ? `/s/${space}` : ''}/api/saved_objects/_find?type=${objectType}&per_page=10000`
)
.set(getCommonRequestHeader('1'));
mlApi.assertResponseStatusCode(200, status, findResponse);

Expand Down Expand Up @@ -514,17 +516,23 @@ export function MachineLearningTestResourcesProvider(
await this.assertSavedObjectExistsById(id, SavedObjectType.DASHBOARD);
},

async deleteMlSavedObjectByJobId(jobId: string, jobType: JobType) {
async deleteMlSavedObjectByJobId(jobId: string, jobType: JobType, space?: string) {
const savedObjectId = `${jobType}-${jobId}`;
await this.deleteSavedObjectById(savedObjectId, SavedObjectType.ML_JOB, true);
await this.deleteSavedObjectById(savedObjectId, SavedObjectType.ML_JOB, true, space);
},

async cleanMLSavedObjects() {
async cleanMLSavedObjects(additionalSpaces: string[] = []) {
// clean default space
await this.cleanMLJobSavedObjects();
await this.cleanMLTrainedModelsSavedObjects();

for (const space of additionalSpaces) {
await this.cleanMLJobSavedObjects(space);
await this.cleanMLTrainedModelsSavedObjects(space);
}
},

async cleanMLJobSavedObjects() {
async cleanMLJobSavedObjects(space?: string) {
log.debug('Deleting ML job saved objects ...');
const savedObjectIds = await this.getSavedObjectIdsByType(SavedObjectType.ML_JOB);
for (const id of savedObjectIds) {
Expand All @@ -533,10 +541,11 @@ export function MachineLearningTestResourcesProvider(
log.debug('> ML job saved objects deleted.');
},

async cleanMLTrainedModelsSavedObjects() {
async cleanMLTrainedModelsSavedObjects(space?: string) {
log.debug('Deleting ML trained model saved objects ...');
const savedObjectIds = await this.getSavedObjectIdsByType(
SavedObjectType.ML_TRAINED_MODEL_SAVED_OBJECT_TYPE
SavedObjectType.ML_TRAINED_MODEL_SAVED_OBJECT_TYPE,
space
);
for (const id of savedObjectIds) {
if (mlApi.isInternalModelId(id)) {
Expand All @@ -546,7 +555,8 @@ export function MachineLearningTestResourcesProvider(
await this.deleteSavedObjectById(
id,
SavedObjectType.ML_TRAINED_MODEL_SAVED_OBJECT_TYPE,
true
true,
space
);
}
log.debug('> ML trained model saved objects deleted.');
Expand Down

0 comments on commit 2e3adf4

Please sign in to comment.