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

Ctrl-C doesn't work during 'odo dev' build phase and if build is stuck the only way out is killing the terminal #6196

Closed
dgolovin opened this issue Oct 4, 2022 · 6 comments · Fixed by #6736
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. sprint demo Indicates an issue for which a demo should be recorded and presented at the end of the sprint.
Milestone

Comments

@dgolovin
Copy link
Contributor

dgolovin commented Oct 4, 2022

/kind bug

What versions of software are you using?

Operating System: Linux

Output of odo version: 3.0.0-rc1

How did you run odo exactly?

odo dev for project created with nodejs-angular devfile and starter project
make install command never exit by adding && npm run dev

Actual behavior

Command shows
Building your application in container on cluster (command: install) ...
and no reaction to Ctrl - C

The same behavior is for long build like java devfiles

/  \__     Developing using the java-maven-development Devfile
\__/  \    Namespace: nivologd-dev
/  \__/    odo version: v3.0.0-rc1
\__/

↪ Deploying to the cluster in developer mode
•  Waiting for Kubernetes resources  ...
✓  Added storage m2 to component
⚠  Pod is Pending
✓  Pod is Running
•  Syncing files into the container  ...
✓  Syncing files into the container [1s]
•  Building your application in container on cluster (command: mvn-package)  ...
^C
✓  Building your application in container on cluster (command: mvn-package) [28s]
•  Executing the application (command: run)  ...
-  Forwarding from 127.0.0.1:40001 -> 8080


Watching for changes in the current directory /home/denis/temp/demo1
Press Ctrl+c to exit `odo dev` and delete resources from the cluster

✗  Dev mode interrupted by user
Cleaning resources, please wait
✗  Finished executing the application (command: run) [41s]

If build is long enough and user exits by closing terminal it leaves the cluster with 'inner loop resources' for the component.

Expected behavior

Ctrl - C works at every phase of the 'odo dev'

Any logs, error output, etc?

See above

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 4, 2022
@rm3l rm3l added the needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. label Oct 5, 2022
@rm3l rm3l added needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. and removed needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. labels Nov 2, 2022
@valaparthvi
Copy link
Member

valaparthvi commented Dec 1, 2022

We needed a way to stop the execution of long running processes such as file sync and command execution if Ctrl+C is encountered. After some debugging we came across ExecCmdInContainer which is responsible for all the communications between local system and container, and this is where we need to catch Ctrl+C signal.

// "k8s.io/client-go/tools/remotecommand"


// ExecCMDInContainer execute command in the container of a pod, pass an empty string for containerName to execute in the first container of the pod
func (c *Client) ExecCMDInContainer(containerName, podName string, cmd []string, stdout io.Writer, stderr io.Writer, stdin io.Reader, tty bool) error {

	---Omitted for brevity---

	// Connect to url (constructed from req) using SPDY (HTTP/2) protocol which allows bidirectional streams.
	exec, err := remotecommand.NewSPDYExecutor(config, "POST", req.URL())
	if err != nil {
		return fmt.Errorf("unable execute command via SPDY: %w", err)
	}
	// initialize the transport of the standard shell streams
	err = exec.Stream(remotecommand.StreamOptions{
		Stdin:  stdin,
		Stdout: stdout,
		Stderr: stderr,
		Tty:    tty,
	})
	if err != nil {
		return fmt.Errorf("error while streaming command: %w", err)
	}

	return nil
}

This method uses k8s/client-go to start a stream and closes it only when client closes the connection or the server disconnects. The current library version does not have a way to interpret signals, but the new version(~28 days old) does with kubernetes/kubernetes#103177. A new method StreamWithContext replaces the Stream method which is now marked deprecated.

Updating the library should ideally fix the problem for us, but there's one more catch. We also use openshift/client-go library in odo, which requires to use a certain version of k8s.io/client-go.
If openshift/client-go updates their k8s.io/client-go, we should be able to use the new method, optimistically fixing the issue at hand.

We'll wait on fixing this until we can update the library.

@valaparthvi valaparthvi added status/blocked Denotes an issue or PR that is blocked on something (e.g., issue/PR in different repo) and removed needs-triage Indicates an issue or PR lacks a `triage/*` and requires one. labels Dec 1, 2022
@feloy
Copy link
Contributor

feloy commented Mar 2, 2023

Make some research to check if openshift client-go library includes the PR kubernetes/kubernetes#103177

@feloy
Copy link
Contributor

feloy commented Mar 2, 2023

Also check on podman if it is the same blocking part.

@kadel
Copy link
Member

kadel commented Mar 9, 2023

Make some research to check if openshift client-go library includes the PR kubernetes/kubernetes#103177

this might be a good opportunity to get rid of the openshift-specific code and use only Kubernetes libraries. We should be able to do everything with just k8s libs

@feloy
Copy link
Contributor

feloy commented Mar 9, 2023

This part of the code is using the Kubernetes libraries. The exec.StreamWithContext will be available with PR #6602

@valaparthvi valaparthvi removed the status/blocked Denotes an issue or PR that is blocked on something (e.g., issue/PR in different repo) label Apr 5, 2023
@valaparthvi
Copy link
Member

Removing status/blocked since the required method is now available and we can make another attempt at fixing this.

@rm3l rm3l added this to the v3.10.0 🚀 milestone Apr 6, 2023
@valaparthvi valaparthvi added the sprint demo Indicates an issue for which a demo should be recorded and presented at the end of the sprint. label Apr 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. sprint demo Indicates an issue for which a demo should be recorded and presented at the end of the sprint.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants