Skip to content

Commit 99368ce

Browse files
authored
Merge pull request #822 from davidgamero/davidgamero/log-migration
convert log to use fetch streaming
2 parents 44f5170 + e333b4d commit 99368ce

File tree

2 files changed

+86
-31
lines changed

2 files changed

+86
-31
lines changed

src/log.ts

Lines changed: 49 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
import request = require('request');
22
import { Writable } from 'stream';
33
import { KubeConfig } from './config';
4-
import { HttpError, ObjectSerializer } from './gen/api';
5-
4+
import { ApiException } from './api';
5+
import { RequestOptions } from 'https';
6+
import { V1Status } from './gen';
7+
import fetch from 'node-fetch'
8+
import { URL, URLSearchParams } from 'url';
69
export interface LogOptions {
710
/**
811
* Follow the log stream of the pod. Defaults to false.
@@ -44,6 +47,26 @@ export interface LogOptions {
4447
timestamps?: boolean;
4548
}
4649

50+
export function AddOptionsToSearchParams(options: LogOptions | undefined, searchParams: URLSearchParams) {
51+
if (!options) {
52+
return
53+
}
54+
searchParams.append('follow', options?.follow?.toString() || 'false');
55+
if (options?.limitBytes) {
56+
searchParams.set('limitBytes', options.limitBytes.toString());
57+
}
58+
searchParams.set('pretty', options?.follow?.toString() || 'false');
59+
searchParams.set('previous', options?.previous?.toString() || 'false');
60+
if (options?.sinceSeconds) {
61+
searchParams.set('sinceSeconds', options?.sinceSeconds?.toString() || 'false');
62+
}
63+
if (options?.tailLines) {
64+
searchParams.set('tailLines', options?.tailLines?.toString() || 'false');
65+
}
66+
searchParams.set('timestamps', options?.timestamps?.toString() || 'false');
67+
return searchParams
68+
}
69+
4770
export class Log {
4871
public config: KubeConfig;
4972

@@ -90,38 +113,33 @@ export class Log {
90113
}
91114
const url = cluster.server + path;
92115

93-
const requestOptions: request.Options = {
94-
method: 'GET',
95-
qs: {
96-
...options,
97-
container: containerName,
98-
},
99-
uri: url,
100-
};
101-
await this.config.applyToRequest(requestOptions);
102-
103-
return new Promise((resolve, reject) => {
104-
const req = request(requestOptions, (error, response, body) => {
105-
if (error) {
106-
reject(error);
107-
done(error);
108-
} else if (response.statusCode !== 200) {
109-
try {
110-
const deserializedBody = ObjectSerializer.deserialize(JSON.parse(body), 'V1Status');
111-
reject(new HttpError(response, deserializedBody, response.statusCode));
112-
} catch (e) {
113-
reject(new HttpError(response, body, response.statusCode));
116+
const requestOptions: RequestOptions = {};
117+
118+
const requestURL = new URL(cluster.server + path);
119+
120+
var searchParams = requestURL.searchParams;
121+
AddOptionsToSearchParams(options, searchParams);
122+
123+
await this.config.applytoHTTPSOptions(requestOptions);
124+
125+
const req = await fetch(requestURL.toString(), requestOptions)
126+
.then(response => {
127+
const status = response.status;
128+
if (status === 200) {
129+
response.body.pipe(stream) // TODO: the follow search param still has the stream close prematurely based on my testing
130+
} else if (status === 500) {
131+
const v1status = response.body as V1Status;
132+
const v1code = v1status.code;
133+
const v1message = v1status.message;
134+
if (v1code !== undefined && v1message !== undefined) {
135+
throw new ApiException<undefined | V1Status>(v1code, v1message, v1status, response.headers.raw())
114136
}
115-
done(body);
116137
} else {
117-
done(null);
118-
}
119-
}).on('response', (response) => {
120-
if (response.statusCode === 200) {
121-
req.pipe(stream);
122-
resolve(req);
138+
throw new ApiException<undefined>(status, "Error occurred in log request", undefined, response.headers.raw())
123139
}
140+
}).catch((err) => {
141+
throw new ApiException<undefined>(err, "Error occurred in log request", undefined, err)
124142
});
125-
});
143+
return req;
126144
}
127145
}

src/log_test.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { expect } from 'chai';
2+
import { AddOptionsToSearchParams, LogOptions } from './log';
3+
import { URLSearchParams } from 'url';
4+
5+
describe('Log', () => {
6+
describe('AddOptionsToSearchParams', () => {
7+
it('should add options to search params', () => {
8+
const searchParams = new URLSearchParams();
9+
const options: LogOptions = {
10+
follow: true,
11+
limitBytes: 100,
12+
pretty: true,
13+
previous: true,
14+
sinceSeconds: 1,
15+
tailLines: 1,
16+
timestamps: true,
17+
};
18+
AddOptionsToSearchParams(options, searchParams);
19+
expect(searchParams.get('follow')).to.equal('true');
20+
expect(searchParams.get('limitBytes')).to.equal('100');
21+
expect(searchParams.get('pretty')).to.equal('true');
22+
expect(searchParams.get('previous')).to.equal('true');
23+
expect(searchParams.get('sinceSeconds')).to.equal('1');
24+
expect(searchParams.get('tailLines')).to.equal('1');
25+
expect(searchParams.get('timestamps')).to.equal('true');
26+
});
27+
it('should use defaults for', () => {
28+
const searchParams = new URLSearchParams();
29+
const options: LogOptions = {};
30+
AddOptionsToSearchParams(options, searchParams);
31+
expect(searchParams.get('follow')).to.equal('false');
32+
expect(searchParams.get('pretty')).to.equal('false');
33+
expect(searchParams.get('previous')).to.equal('false');
34+
expect(searchParams.get('timestamps')).to.equal('false');
35+
});
36+
})
37+
});

0 commit comments

Comments
 (0)