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

Adds error from es call to nodes.info to the nodes version compatibility response message #100005

Merged
2 changes: 1 addition & 1 deletion src/core/server/elasticsearch/status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe('calculateStatus', () => {
});
});

it('changes to available with a differemnt message when isCompatible and warningNodes present', async () => {
it('changes to available with a different message when isCompatible and warningNodes present', async () => {
Copy link
Contributor Author

@TinaHeiligers TinaHeiligers May 12, 2021

Choose a reason for hiding this comment

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

cleans up a typo

expect(
await calculateStatus$(
of({
Expand Down
120 changes: 118 additions & 2 deletions src/core/server/elasticsearch/version_check/ensure_es_version.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,28 @@ describe('mapNodesVersionCompatibility', () => {
`"You're running Kibana 5.1.0 with some different versions of Elasticsearch. Update Kibana or Elasticsearch to the same version to prevent compatibility issues: v5.1.1 @ http_address (ip)"`
);
});

it('returns isCompatible=false without an extended message when a requestError is not provided', async () => {
const result = mapNodesVersionCompatibility({ nodes: {} }, KIBANA_VERSION, false);
expect(result.isCompatible).toBe(false);
expect(result.requestError).toBeUndefined();
expect(result.message).toMatchInlineSnapshot(
`"Unable to retrieve version information from Elasticsearch nodes."`
);
});

it('returns isCompatible=false with an extended message when a requestError is present', async () => {
const result = mapNodesVersionCompatibility(
{ nodes: {}, requestError: new Error('connection refused') },
KIBANA_VERSION,
false
);
expect(result.isCompatible).toBe(false);
expect(result.requestError).toBeTruthy();
expect(result.message).toMatchInlineSnapshot(
`"Unable to retrieve version information from Elasticsearch nodes. connection refused"`
);
});
});

describe('pollEsNodesVersion', () => {
Expand All @@ -122,13 +144,13 @@ describe('pollEsNodesVersion', () => {
internalClient.nodes.info.mockImplementationOnce(() => createEsError(error));
};

it('returns iscCompatible=false and keeps polling when a poll request throws', (done) => {
it('returns isCompatible=false and keeps polling when a poll request throws', (done) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cleans up a typo

expect.assertions(3);
const expectedCompatibilityResults = [false, false, true];
jest.clearAllMocks();

nodeInfosSuccessOnce(createNodes('5.1.0', '5.2.0', '5.0.0'));
nodeInfosErrorOnce('mock request error');
nodeInfosErrorOnce(new Error('mock request error'));
nodeInfosSuccessOnce(createNodes('5.1.0', '5.2.0', '5.1.1-Beta1'));

pollEsNodesVersion({
Expand All @@ -148,6 +170,100 @@ describe('pollEsNodesVersion', () => {
});
});

it('returns the error from a failed nodes.info call when a poll request throws', (done) => {
expect.assertions(2);
const expectedCompatibilityResults = [false];
const expectedMessageResults = [
'Unable to retrieve version information from Elasticsearch nodes. mock request error',
];
jest.clearAllMocks();

nodeInfosErrorOnce(new Error('mock request error'));

pollEsNodesVersion({
internalClient,
esVersionCheckInterval: 1,
ignoreVersionMismatch: false,
kibanaVersion: KIBANA_VERSION,
log: mockLogger,
})
.pipe(take(1))
.subscribe({
next: (result) => {
expect(result.isCompatible).toBe(expectedCompatibilityResults.shift());
expect(result.message).toBe(expectedMessageResults.shift());
},
complete: done,
error: done,
});
});

it('only emits if the error from a failed nodes.info call changed from the previous poll', (done) => {
expect.assertions(4);
const expectedCompatibilityResults = [false, false];
const expectedMessageResults = [
'Unable to retrieve version information from Elasticsearch nodes. mock request error',
'Unable to retrieve version information from Elasticsearch nodes. mock request error 2',
];
jest.clearAllMocks();

nodeInfosErrorOnce(new Error('mock request error')); // emit
nodeInfosErrorOnce(new Error('mock request error')); // ignore, same error message
nodeInfosErrorOnce(new Error('mock request error 2')); // emit

pollEsNodesVersion({
internalClient,
esVersionCheckInterval: 1,
ignoreVersionMismatch: false,
kibanaVersion: KIBANA_VERSION,
log: mockLogger,
})
.pipe(take(2))
.subscribe({
next: (result) => {
expect(result.message).toBe(expectedMessageResults.shift());
expect(result.isCompatible).toBe(expectedCompatibilityResults.shift());
},
complete: done,
error: done,
});
});

it('returns isCompatible=false and keeps polling when a poll request throws, only responding again if the error message has changed', (done) => {
expect.assertions(8);
const expectedCompatibilityResults = [false, false, true, false];
const expectedMessageResults = [
'This version of Kibana (v5.1.0) is incompatible with the following Elasticsearch nodes in your cluster: v5.0.0 @ http_address (ip)',
'Unable to retrieve version information from Elasticsearch nodes. mock request error',
"You're running Kibana 5.1.0 with some different versions of Elasticsearch. Update Kibana or Elasticsearch to the same version to prevent compatibility issues: v5.2.0 @ http_address (ip), v5.1.1-Beta1 @ http_address (ip)",
'Unable to retrieve version information from Elasticsearch nodes. mock request error 2',
Copy link
Member

Choose a reason for hiding this comment

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

NIT: can we make this error the same as in line 239 to prove that the same error also emits after having switched earlier to a success reponse?

];
jest.clearAllMocks();

nodeInfosSuccessOnce(createNodes('5.1.0', '5.2.0', '5.0.0')); // emit
nodeInfosErrorOnce(new Error('mock request error')); // emit
nodeInfosErrorOnce(new Error('mock request error')); // ignore
nodeInfosSuccessOnce(createNodes('5.1.0', '5.2.0', '5.1.1-Beta1')); // emit
nodeInfosErrorOnce(new Error('mock request error 2')); // emit

pollEsNodesVersion({
internalClient,
esVersionCheckInterval: 1,
ignoreVersionMismatch: false,
kibanaVersion: KIBANA_VERSION,
log: mockLogger,
})
.pipe(take(4))
.subscribe({
next: (result) => {
expect(result.isCompatible).toBe(expectedCompatibilityResults.shift());
expect(result.message).toBe(expectedMessageResults.shift());
},
complete: done,
error: done,
});
});

it('returns compatibility results', (done) => {
expect.assertions(1);
const nodes = createNodes('5.1.0', '5.2.0', '5.0.0');
Expand Down
37 changes: 26 additions & 11 deletions src/core/server/elasticsearch/version_check/ensure_es_version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface NodesVersionCompatibility {
incompatibleNodes: NodeInfo[];
warningNodes: NodeInfo[];
kibanaVersion: string;
requestError?: Error;
}

function getHumanizedNodeName(node: NodeInfo) {
Expand All @@ -57,22 +58,28 @@ function getHumanizedNodeName(node: NodeInfo) {
}

export function mapNodesVersionCompatibility(
nodesInfo: NodesInfo,
nodesInfoResponse: NodesInfo & { requestError?: Error },
kibanaVersion: string,
ignoreVersionMismatch: boolean
): NodesVersionCompatibility {
if (Object.keys(nodesInfo.nodes ?? {}).length === 0) {
if (Object.keys(nodesInfoResponse.nodes ?? {}).length === 0) {
// Note: If the a requestError is present, the message contains the requestError.message as a suffix
let message = `Unable to retrieve version information from Elasticsearch nodes.`;
if (nodesInfoResponse.requestError) {
message = message + ` ${nodesInfoResponse.requestError.message}`;
}
return {
isCompatible: false,
message: 'Unable to retrieve version information from Elasticsearch nodes.',
message,
incompatibleNodes: [],
warningNodes: [],
kibanaVersion,
requestError: nodesInfoResponse.requestError, // optionally stringify the whole Error or parts of it that we're interested in
};
}
const nodes = Object.keys(nodesInfo.nodes)
const nodes = Object.keys(nodesInfoResponse.nodes)
.sort() // Sorting ensures a stable node ordering for comparison
.map((key) => nodesInfo.nodes[key])
.map((key) => nodesInfoResponse.nodes[key])
.map((node) => Object.assign({}, node, { name: getHumanizedNodeName(node) }));

// Aggregate incompatible ES nodes.
Expand Down Expand Up @@ -112,16 +119,24 @@ export function mapNodesVersionCompatibility(
kibanaVersion,
};
}

// Returns true if NodesVersionCompatibility requestError is the same
function compareNodesInfoErrorMessages(
prev: NodesVersionCompatibility,
curr: NodesVersionCompatibility
): boolean {
return prev.requestError?.message === curr.requestError?.message;
}
// Returns true if two NodesVersionCompatibility entries match
function compareNodes(prev: NodesVersionCompatibility, curr: NodesVersionCompatibility) {
const nodesEqual = (n: NodeInfo, m: NodeInfo) => n.ip === m.ip && n.version === m.version;
// if we pass the full error back in here, we can check to see if it's the same error rather than checking
Copy link
Member

Choose a reason for hiding this comment

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

I think these are dev notes and can be removed now, can they? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes they can.

return (
curr.isCompatible === prev.isCompatible &&
curr.incompatibleNodes.length === prev.incompatibleNodes.length &&
curr.warningNodes.length === prev.warningNodes.length &&
curr.incompatibleNodes.every((node, i) => nodesEqual(node, prev.incompatibleNodes[i])) &&
curr.warningNodes.every((node, i) => nodesEqual(node, prev.warningNodes[i]))
curr.warningNodes.every((node, i) => nodesEqual(node, prev.warningNodes[i])) &&
compareNodesInfoErrorMessages(curr, prev)
);
}

Expand All @@ -142,13 +157,13 @@ export const pollEsNodesVersion = ({
).pipe(
map(({ body }) => body),
catchError((_err) => {
return of({ nodes: {} });
return of({ nodes: {}, requestError: _err }); // I don't know how to map over this now
})
);
}),
map((nodesInfo: NodesInfo) =>
mapNodesVersionCompatibility(nodesInfo, kibanaVersion, ignoreVersionMismatch)
map((nodesInfoResponse: NodesInfo & { requestError?: Error }) =>
mapNodesVersionCompatibility(nodesInfoResponse, kibanaVersion, ignoreVersionMismatch)
),
distinctUntilChanged(compareNodes) // Only emit if there are new nodes or versions
distinctUntilChanged(compareNodes) // Only emit if there are new nodes or versions or if we return an error and that error changes
);
};