Skip to content

Add a client for the pod read-log endpoint #82

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

Closed
dbolkensteyn opened this issue Aug 15, 2018 · 6 comments · Fixed by #83
Closed

Add a client for the pod read-log endpoint #82

dbolkensteyn opened this issue Aug 15, 2018 · 6 comments · Fixed by #83

Comments

@dbolkensteyn
Copy link
Contributor

dbolkensteyn commented Aug 15, 2018

Command-line: kubectl log <pod-name>
Documentation: https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.11/#read-log-pod-v1-core

From what I read so far on kubernetes/kubernetes#13885, the /log endpoints only expects binary.k8s.io as protocol. Without it (but still sending the *.channel.k8s.io ones), a 403 Forbidden status code is returned.

I'll try to submit a PR for this in the upcoming days, suggestions are welcome.

Right now I'm trying to fit this into WebSocketHandler, but there is already a binary handler that expects the first byte of the message to be the channel number, which is not the case for the /log endpoint. See https://github.com/kubernetes-client/javascript/blob/master/node-client/src/web-socket-handler.ts#L89-L94

@dbolkensteyn dbolkensteyn changed the title Add a client for the pod read-log /log endpoint Add a client for the pod read-log endpoint Aug 15, 2018
dbolkensteyn pushed a commit to dbolkensteyn/javascript that referenced this issue Aug 15, 2018
@dbolkensteyn
Copy link
Contributor Author

PR opened, let's discuss there

dbolkensteyn added a commit to dbolkensteyn/javascript that referenced this issue Aug 15, 2018
dbolkensteyn added a commit to dbolkensteyn/javascript that referenced this issue Mar 19, 2019
dbolkensteyn added a commit to dbolkensteyn/javascript that referenced this issue Mar 19, 2019
dbolkensteyn added a commit to dbolkensteyn/javascript that referenced this issue Mar 19, 2019
dbolkensteyn added a commit to dbolkensteyn/javascript that referenced this issue Mar 20, 2019
dbolkensteyn added a commit to dbolkensteyn/javascript that referenced this issue Mar 20, 2019
k8s-ci-robot added a commit that referenced this issue Mar 27, 2019
Fixes #82 Add a client for the pod read-log endpoint
@chadbr
Copy link

chadbr commented Nov 5, 2019

@dbolkensteyn Hey Dinesh -- Can you point me to an example of how to use the Log.log(..) method? I have not been able to figure out how to get something out the of the stream: Writable parameter.

Thanks, Chad

@dbolkensteyn
Copy link
Contributor Author

dbolkensteyn commented Nov 5, 2019

It's been a while but try something along these lines @chadbr:

const myStream = new stream.Writable({
    write(chunk, encoding, callback) {
      console.log(chunk.toString().trimRight())
      // or do something else more useful to you
      callback()
    }
  })

and then pass myStream as the argument.

@chadbr
Copy link

chadbr commented Nov 5, 2019

Thanks @dbolkensteyn -- I'm failing to grasp why we would pass a writable stream instead of getting a readable stream from the log function? I'm assuming the function is there to read log streams?

Sorry for the dumb question...

For example, when I call spawn - I get an object back with the process streams:

    // return this object when stdio option is undefined or not specified
    interface ChildProcessWithoutNullStreams extends ChildProcess {
        stdin: Writable;
        stdout: Readable;
        stderr: Readable;

I would have assumed the log function would return something like `stdout: Readable;'? What am I missing?

@chadbr
Copy link

chadbr commented Nov 5, 2019

As a side note -- I think I might have found a small bug?

applyToRequest is async / returns a promise, and it's not awaited / resolved? I'm not sure if it matters...

        this.config.applyToRequest(requestOptions);

javascript/src/log.ts

Lines 78 to 79 in 9d5757e

this.config.applyToRequest(requestOptions);

Sorry for the noise...

@dbolkensteyn
Copy link
Contributor Author

I don't have a good answer to either of your points as my knowledge of TypeScript is limited. Feel free to further investigate and open new issues (and/or PRs).

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.

2 participants