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

PodOperationsImpl should call listener.onOpen before marking the future as complete #3957

Closed
AdrianFarmadin opened this issue Mar 10, 2022 · 2 comments · Fixed by #3958
Closed
Assignees
Labels
Milestone

Comments

@AdrianFarmadin
Copy link
Contributor

Describe the bug

The buildAsync method in OkHttpWebSocketImpl class is called with ExecWebSocketListener as parameter.
The ExecWebSocketListener is opening outputPipe and errorPipe in onOpen.
If the future is completed before the outputPipe and errorPipe are open, then the consumer can read from the pipes before they are open.
This can cause Pipe not connected exception.

The pipes should be first open and then the future should be marked as complete.

Fabric8 Kubernetes Client version

other (please specify in additional context)

Steps to reproduce

  1. Execute command with PodOperationsImpl and define outPipe and errPipe
  2. Start reading lines from streams immediately after the command is started
ExecWatch watch = client.getPodResource(pod)
                .inContainer(getNameOfFirstContainer())
                .readingOutput(outputStream)
                .readingError(errorStream)
                .exec(args);
// start reading lines from outputStream and errorStream

Expected behavior

No Pipe not connected exception

Runtime

Kubernetes (vanilla)

Kubernetes API Server version

other (please specify in additional context)

Environment

Linux

Fabric8 Kubernetes Client Logs

No response

Additional context

Fabric8 Kubernetes Client version > 5.11
Kubernetes API Server version: v1.20.11

AdrianFarmadin added a commit to AdrianFarmadin/kubernetes-client that referenced this issue Mar 10, 2022
AdrianFarmadin added a commit to AdrianFarmadin/kubernetes-client that referenced this issue Mar 14, 2022
@sunix
Copy link
Collaborator

sunix commented Mar 14, 2022

Hey @AdrianFarmadin
Thanks for reporting.
Are you working on this issue? Shall I assign it to you?

@AdrianFarmadin
Copy link
Contributor Author

Hi @sunix, yes, I'm working on it. You can assign it to me

Pull request: #3958

manusa pushed a commit to AdrianFarmadin/kubernetes-client that referenced this issue Mar 22, 2022
@manusa manusa added bug 5.12.x Backportable tentative labels Mar 22, 2022
@manusa manusa added this to the 6.0.0 milestone Mar 22, 2022
manusa pushed a commit to AdrianFarmadin/kubernetes-client that referenced this issue Mar 22, 2022
manusa pushed a commit to AdrianFarmadin/kubernetes-client that referenced this issue Mar 22, 2022
manusa pushed a commit that referenced this issue Mar 22, 2022
@manusa manusa removed the 5.12.x Backportable tentative label Apr 5, 2022
manusa pushed a commit to manusa/kubernetes-client that referenced this issue Apr 5, 2022
…he connection as open

(cherry picked from commit eaf7a15)
Signed-off-by: Marc Nuri <[email protected]>
manusa pushed a commit to manusa/kubernetes-client that referenced this issue Apr 5, 2022
…he connection as open

(cherry picked from commit eaf7a15)
Signed-off-by: Marc Nuri <[email protected]>
manusa pushed a commit that referenced this issue Apr 5, 2022
…tion as open

(cherry picked from commit eaf7a15)
Signed-off-by: Marc Nuri <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants