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

Using ...pods.exec.post() results in Error: Upgrade request required #319

Closed
fefl opened this issue Aug 30, 2018 · 7 comments · Fixed by #329
Closed

Using ...pods.exec.post() results in Error: Upgrade request required #319

fefl opened this issue Aug 30, 2018 · 7 comments · Fixed by #329

Comments

@fefl
Copy link

fefl commented Aug 30, 2018

I am currently trying to use client.api.v1.namespaces(ns).pods(pod).exec.post() to run a command in a pod. Using it looks something like this:

const cmd = await client.api.v1.namespaces(ns).pods(pod).exec.post({ qs: { command: ['nginx', '-s', 'reload'] } });

This results in the following error:

{ Error: Upgrade request required
    at http.request (/app/node_modules/fluent-openapi/lib/loader.js:230:25)
    at Request.request [as _callback] (/app/node_modules/kubernetes-client/lib/request.js:107:14)
    at Request.self.callback (/app/node_modules/request/request.js:185:22)
    at Request.emit (events.js:180:13)
    at Request.<anonymous> (/app/node_modules/request/request.js:1161:10)
    at Request.emit (events.js:180:13)
    at IncomingMessage.<anonymous> (/app/node_modules/request/request.js:1083:12)
    at Object.onceWrapper (events.js:272:13)
    at IncomingMessage.emit (events.js:185:15)
    at endReadableNT (_stream_readable.js:1101:12)
    at process._tickCallback (internal/process/next_tick.js:114:19) code: 400, statusCode: 400 }

This error occurs with Kubernetes v1.9.0 as well as with v1.9.4 and v1.9.5. I used version 6.0.0 of your client.

If there is any more information I can provide you to help me solve this just tell me.

@silasbw
Copy link
Contributor

silasbw commented Sep 1, 2018

kubernetes-client doesn't exec, but it would be nice if it did. it's been on the TODO list for a long time.

This issue discussing how to use exec in node.js might be helpful to you: #19

@gempesaw
Copy link
Contributor

gempesaw commented Sep 6, 2018

Oh, this reminds me, I found a blog post and looked at the Kube server code to see how the Kube API wanted to talk over the websocket channel for the seemingly undocumented exec endpoint. It wants websocket frames where the first byte indicates the stream (stdout/stderr/....something else/out of band-errors) and then the rest of the frame is the command as binary.

I'm happy to open a PR for it, but I don't know what kind of interface kubernetes-client would like to present around the functionality. Like - when you call exec, it returns a websocket connection that you can pass commands to? or just these one-off "execute a command and get a string result back"? Also, I believe it'd require an additional websocket library dependency, if that's a concern...

@silasbw
Copy link
Contributor

silasbw commented Sep 6, 2018

It would be rad to support this somehow because we get a bunch of questions about it.

One incremental way to add support is to handle the upgrade response in exec.post (and actually retry the request with the websocket), and then add something to examples/ that shows how to execute commands using exec.post like you describe above @gempesaw.

That's probably the minimum amount of support in kubernetes-client to actually support exec. We could see how that flies with users, and then decide what to do next based on that feedback. Thoughts @gempesaw? I think folks would be super happy if you contributed some exec support.

@gempesaw
Copy link
Contributor

gempesaw commented Sep 7, 2018

Cool, that sounds like an entirely reasonable first step. I hope to take a look this weekend :)

@gempesaw
Copy link
Contributor

@silasbw hey, where would you expect this functionality to live? if i'm understanding the structure correctly, I'd guess it it belongs in fluent-openapi... as special logic in the post function?

I don't think I can express it as part of a swagger spec - last I checked swagger didn't have built in support for websockets...

@silasbw
Copy link
Contributor

silasbw commented Sep 10, 2018

What if we add code here-ish and handle it in a way similar to 401ing:

https://github.com/godaddy/kubernetes-client/blob/master/lib/request.js#L94

If the response has an upgrade header, we upgrade the connection and retry. We might be able to handle that way without adding endpoint specific logic.

@Stumblinbear
Copy link

Stumblinbear commented Sep 16, 2018

Currently using this bit of code for the functionality, however while commands are instantly executed, it doesn't actually close the socket for ~30 seconds. Not sure why, but it works for now.

async exec(...cmds: string[]) {
    return new Promise((resolve, reject) => {
        var response = '', uri = '';

        uri += 'wss://';
        // Create the URL from config data
        uri += this.client.kConfig.url.split('://', 2)[1];
        // Fetch the raw url from the exec endpoint
        uri += '/' + (client.k.api.v1.namespaces(ns).pod(cmdPod) as any)['splits'].join('/');
        uri += '?stdout=1&stdin=1&stderr=1';

        let newCmds = ['/bin/bash', '-c', '$(' + cmds.join(') && (') + ')'];
        newCmds.forEach(subCmd => uri += `&command=${encodeURIComponent(subCmd)}`);
    
        var ws = new WebSocket(uri, 'base64.channel.k8s.io', {
            ca: this.kConfig.ca,
            key: this.kConfig.key,
            cert: this.kConfig.cert
        });

        ws.on('message', function(data: any) {
            if(data[0].match(/^[0-3]$/)) {
                response += Buffer.from(data.slice(1), 'base64').toString('ascii');
            }
        });

        ws.on('error', (err: Error) => reject(err));
        ws.on('close', () => resolve(response));
    });
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants