From 1038f063e095fc31f60ff6dc58d7fae768dd0f05 Mon Sep 17 00:00:00 2001
From: Alexander Larsson <alexl@redhat.com>
Date: Tue, 11 Oct 2022 16:38:42 +0200
Subject: [PATCH] Add and use libpod/Container.Terminal() helper

This just gets ctr.config.Spec.Process.Terminal with some null checks,
allowing several places that open-coded this to use the helper.

In particular, this helps the code in
pkg/domain/infra/abi/terminal.StartAttachCtr(), that used to do:
`ctr.Spec().Process.Terminal`, which looks fine, but actually causes
a deep json copy in the `ctr.Spec()` call that takes over 3 msec.

[NO NEW TESTS NEEDED] Just minor performance effects

Signed-off-by: Alexander Larsson <alexl@redhat.com>
---
 libpod/container.go                              | 8 ++++++++
 libpod/container_api.go                          | 2 +-
 libpod/kube.go                                   | 2 +-
 libpod/oci_conmon_common.go                      | 9 +++------
 pkg/domain/infra/abi/containers.go               | 6 ++----
 pkg/domain/infra/abi/terminal/terminal_common.go | 3 ++-
 6 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/libpod/container.go b/libpod/container.go
index a4eb33c494..560f9ab9ef 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -680,6 +680,14 @@ func (c *Container) WorkingDir() string {
 	return "/"
 }
 
+// Terminal returns true if the container has a terminal
+func (c *Container) Terminal() bool {
+	if c.config.Spec != nil && c.config.Spec.Process != nil {
+		return c.config.Spec.Process.Terminal
+	}
+	return false
+}
+
 // State Accessors
 // Require locking
 
diff --git a/libpod/container_api.go b/libpod/container_api.go
index be0ca01282..6b03c19009 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -273,7 +273,7 @@ func (c *Container) Attach(streams *define.AttachStreams, keys string, resize <-
 	// Send a SIGWINCH after attach succeeds so that most programs will
 	// redraw the screen for the new attach session.
 	attachRdy := make(chan bool, 1)
-	if c.config.Spec.Process != nil && c.config.Spec.Process.Terminal {
+	if c.Terminal() {
 		go func() {
 			<-attachRdy
 			if err := c.ociRuntime.KillContainer(c, uint(signal.SIGWINCH), false); err != nil {
diff --git a/libpod/kube.go b/libpod/kube.go
index 1f4831006b..b8047a844f 100644
--- a/libpod/kube.go
+++ b/libpod/kube.go
@@ -698,7 +698,7 @@ func containerToV1Container(ctx context.Context, c *Container) (v1.Container, []
 	// container.EnvFromSource =
 	kubeContainer.SecurityContext = kubeSec
 	kubeContainer.StdinOnce = false
-	kubeContainer.TTY = c.config.Spec.Process.Terminal
+	kubeContainer.TTY = c.Terminal()
 
 	if c.config.Spec.Linux != nil &&
 		c.config.Spec.Linux.Resources != nil {
diff --git a/libpod/oci_conmon_common.go b/libpod/oci_conmon_common.go
index cbdbad02d7..ab0c8aabe8 100644
--- a/libpod/oci_conmon_common.go
+++ b/libpod/oci_conmon_common.go
@@ -493,10 +493,7 @@ func socketCloseWrite(conn *net.UnixConn) error {
 // Returns any errors that occurred, and whether the connection was successfully
 // hijacked before that error occurred.
 func (r *ConmonOCIRuntime) HTTPAttach(ctr *Container, req *http.Request, w http.ResponseWriter, streams *HTTPAttachStreams, detachKeys *string, cancel <-chan bool, hijackDone chan<- bool, streamAttach, streamLogs bool) (deferredErr error) {
-	isTerminal := false
-	if ctr.config.Spec.Process != nil {
-		isTerminal = ctr.config.Spec.Process.Terminal
-	}
+	isTerminal := ctr.Terminal()
 
 	if streams != nil {
 		if !streams.Stdin && !streams.Stdout && !streams.Stderr {
@@ -1038,7 +1035,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 		args = append(args, fmt.Sprintf("--sdnotify-socket=%s", ctr.config.SdNotifySocket))
 	}
 
-	if ctr.config.Spec.Process.Terminal {
+	if ctr.Terminal() {
 		args = append(args, "-t")
 	} else if ctr.config.Stdin {
 		args = append(args, "-i")
@@ -1135,7 +1132,7 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
-	if ctr.config.Spec.Process.Terminal {
+	if ctr.Terminal() {
 		cmd.Stderr = &stderrBuf
 	}
 
diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index d88dbd7841..4845749511 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -1690,10 +1690,8 @@ func (ic *ContainerEngine) ContainerClone(ctx context.Context, ctrCloneOpts enti
 		return nil, err
 	}
 
-	conf := c.Config()
-	if conf.Spec != nil && conf.Spec.Process != nil && conf.Spec.Process.Terminal { // if we do not pass term, running ctrs exit
-		spec.Terminal = true
-	}
+	// if we do not pass term, running ctrs exit
+	spec.Terminal = c.Terminal()
 
 	// Print warnings
 	if len(out) > 0 {
diff --git a/pkg/domain/infra/abi/terminal/terminal_common.go b/pkg/domain/infra/abi/terminal/terminal_common.go
index d005959082..3dbdfc9560 100644
--- a/pkg/domain/infra/abi/terminal/terminal_common.go
+++ b/pkg/domain/infra/abi/terminal/terminal_common.go
@@ -49,7 +49,8 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr,
 
 	// Check if we are attached to a terminal. If we are, generate resize
 	// events, and set the terminal to raw mode
-	if haveTerminal && ctr.Spec().Process.Terminal {
+
+	if haveTerminal && ctr.Terminal() {
 		cancel, oldTermState, err := handleTerminalAttach(ctx, resize)
 		if err != nil {
 			return err