Merge pull request #11781 from vrothberg/spec

podman run - avoid calls to JSONDeepCopy

9 files changed, 39 insertions, 18 deletions

diff --git a/libpod/container.go b/libpod/container.go
index 5c56ff036..4d15c04c5 100644
--- a/libpod/container.go
+++ b/libpod/container.go
@@ -774,9 +774,9 @@ func (c *Container) ExecSessions() ([]string, error) {
 	return ids, nil
 }
 
-// ExecSession retrieves detailed information on a single active exec session in
-// a container
-func (c *Container) ExecSession(id string) (*ExecSession, error) {
+// execSessionNoCopy returns the associated exec session to id.
+// Note that the session is not a deep copy.
+func (c *Container) execSessionNoCopy(id string) (*ExecSession, error) {
 	if !c.batched {
 		c.lock.Lock()
 		defer c.lock.Unlock()
@@ -791,6 +791,17 @@ func (c *Container) ExecSession(id string) (*ExecSession, error) {
 		return nil, errors.Wrapf(define.ErrNoSuchExecSession, "no exec session with ID %s found in container %s", id, c.ID())
 	}
 
+	return session, nil
+}
+
+// ExecSession retrieves detailed information on a single active exec session in
+// a container
+func (c *Container) ExecSession(id string) (*ExecSession, error) {
+	session, err := c.execSessionNoCopy(id)
+	if err != nil {
+		return nil, err
+	}
+
 	returnSession := new(ExecSession)
 	if err := JSONDeepCopy(session, returnSession); err != nil {
 		return nil, errors.Wrapf(err, "error copying contents of container %s exec session %s", c.ID(), session.ID())
@@ -1095,7 +1106,7 @@ func (c *Container) AutoRemove() bool {
 	if spec.Annotations == nil {
 		return false
 	}
-	return c.Spec().Annotations[define.InspectAnnotationAutoremove] == define.InspectResponseTrue
+	return spec.Annotations[define.InspectAnnotationAutoremove] == define.InspectResponseTrue
 }
 
 // Timezone returns the timezone configured inside the container.
diff --git a/libpod/container_exec.go b/libpod/container_exec.go
index 1cb45a118..f99fb7d3f 100644
--- a/libpod/container_exec.go
+++ b/libpod/container_exec.go
@@ -747,7 +747,7 @@ func (c *Container) Exec(config *ExecConfig, streams *define.AttachStreams, resi
 		return -1, err
 	}
 
-	session, err := c.ExecSession(sessionID)
+	session, err := c.execSessionNoCopy(sessionID)
 	if err != nil {
 		if errors.Cause(err) == define.ErrNoSuchExecSession {
 			// TODO: If a proper Context is ever plumbed in here, we
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index e81f2ec5f..3f9738411 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -2004,7 +2004,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s
 				}
 				return nil, err
 			}
-			ociHooks, err := manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
+			ociHooks, err := manager.Hooks(config, c.config.Spec.Annotations, len(c.config.UserVolumes) > 0)
 			if err != nil {
 				return nil, err
 			}
@@ -2021,7 +2021,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s
 			return nil, err
 		}
 
-		allHooks, err = manager.Hooks(config, c.Spec().Annotations, len(c.config.UserVolumes) > 0)
+		allHooks, err = manager.Hooks(config, c.config.Spec.Annotations, len(c.config.UserVolumes) > 0)
 		if err != nil {
 			return nil, err
 		}
diff --git a/libpod/kube.go b/libpod/kube.go
index 69c90c233..57d99f3ef 100644
--- a/libpod/kube.go
+++ b/libpod/kube.go
@@ -426,7 +426,7 @@ func containerToV1Container(ctx context.Context, c *Container) (v1.Container, []
 	// NOTE: a privileged container mounts all of /dev/*.
 	if !c.Privileged() && len(c.config.Spec.Linux.Devices) > 0 {
 		// TODO Enable when we can support devices and their names
-		kubeContainer.VolumeDevices = generateKubeVolumeDeviceFromLinuxDevice(c.Spec().Linux.Devices)
+		kubeContainer.VolumeDevices = generateKubeVolumeDeviceFromLinuxDevice(c.config.Spec.Linux.Devices)
 		return kubeContainer, kubeVolumes, nil, errors.Wrapf(define.ErrNotImplemented, "linux devices")
 	}
 
diff --git a/libpod/runtime.go b/libpod/runtime.go
index 161d5a533..27885bf5c 100644
--- a/libpod/runtime.go
+++ b/libpod/runtime.go
@@ -706,19 +706,32 @@ func (r *Runtime) TmpDir() (string, error) {
 	return r.config.Engine.TmpDir, nil
 }
 
-// GetConfig returns a copy of the configuration used by the runtime
-func (r *Runtime) GetConfig() (*config.Config, error) {
+// GetConfig returns the configuration used by the runtime.
+// Note that the returned value is not a copy and must hence
+// only be used in a reading fashion.
+func (r *Runtime) GetConfigNoCopy() (*config.Config, error) {
 	r.lock.RLock()
 	defer r.lock.RUnlock()
 
 	if !r.valid {
 		return nil, define.ErrRuntimeStopped
 	}
+	return r.config, nil
+}
+
+// GetConfig returns a copy of the configuration used by the runtime.
+// Please use GetConfigNoCopy() in case you only want to read from
+// but not write to the returned config.
+func (r *Runtime) GetConfig() (*config.Config, error) {
+	rtConfig, err := r.GetConfigNoCopy()
+	if err != nil {
+		return nil, err
+	}
 
 	config := new(config.Config)
 
 	// Copy so the caller won't be able to modify the actual config
-	if err := JSONDeepCopy(r.config, config); err != nil {
+	if err := JSONDeepCopy(rtConfig, config); err != nil {
 		return nil, errors.Wrapf(err, "error copying config")
 	}
 
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index 93bfdd54b..7cda9aa8b 100644
--- a/libpod/runtime_ctr.go
+++ b/libpod/runtime_ctr.go
@@ -193,10 +193,7 @@ func (r *Runtime) initContainerVariables(rSpec *spec.Spec, config *ContainerConf
 		ctr.config.LogPath = ""
 	}
 
-	ctr.config.Spec = new(spec.Spec)
-	if err := JSONDeepCopy(rSpec, ctr.config.Spec); err != nil {
-		return nil, errors.Wrapf(err, "error copying runtime spec while creating container")
-	}
+	ctr.config.Spec = rSpec
 	ctr.config.CreatedTime = time.Now()
 
 	ctr.state.BindMounts = make(map[string]string)
diff --git a/pkg/domain/infra/abi/secrets.go b/pkg/domain/infra/abi/secrets.go
index 2bf8eaae3..34c230e75 100644
--- a/pkg/domain/infra/abi/secrets.go
+++ b/pkg/domain/infra/abi/secrets.go
@@ -21,7 +21,7 @@ func (ic *ContainerEngine) SecretCreate(ctx context.Context, name string, reader
 
 	// set defaults from config for the case they are not set by an upper layer
 	// (-> i.e. tests that talk directly to the api)
-	cfg, err := ic.Libpod.GetConfig()
+	cfg, err := ic.Libpod.GetConfigNoCopy()
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/specgen/generate/container.go b/pkg/specgen/generate/container.go
index ae26807a9..71b882510 100644
--- a/pkg/specgen/generate/container.go
+++ b/pkg/specgen/generate/container.go
@@ -54,7 +54,7 @@ func CompleteSpec(ctx context.Context, r *libpod.Runtime, s *specgen.SpecGenerat
 		}
 	}
 
-	rtc, err := r.GetConfig()
+	rtc, err := r.GetConfigNoCopy()
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/specgen/generate/container_create.go b/pkg/specgen/generate/container_create.go
index b6263332e..6100e7a5b 100644
--- a/pkg/specgen/generate/container_create.go
+++ b/pkg/specgen/generate/container_create.go
@@ -23,7 +23,7 @@ import (
 // Returns the created, container and any warnings resulting from creating the
 // container, or an error.
 func MakeContainer(ctx context.Context, rt *libpod.Runtime, s *specgen.SpecGenerator) (*spec.Spec, *specgen.SpecGenerator, []libpod.CtrCreateOption, error) {
-	rtc, err := rt.GetConfig()
+	rtc, err := rt.GetConfigNoCopy()
 	if err != nil {
 		return nil, nil, nil, err
 	}