New telepresence replace command.

The new `telepresence replace` command simplifies and clarifies
container replacement.

Previously, the `--replace` flag within the `telepresence intercept`
command was used to replace containers. However, this approach
introduced inconsistencies and limitations:

* **Confusion:** Using a flag to modify the core function of a command
  designed for traffic interception led to ambiguity.
* **Inaccurate Behavior:** Replacement was not possible when no incoming
  traffic was intercepted, as the command's design focused on traffic
  routing.

To address these issues, the `--replace` flag within `telepresence
intercept` has been deprecated. The new `telepresence replace` command
provides a dedicated and consistent method for replacing containers,
enhancing clarity and reliability.

Key differences between `replace` and `intercept`:

1. **Scope:** The `replace` command targets and affects an entire
   container, impacting all its traffic, while an `intercept` targets
   specific services and/or service/container ports.
2. **Port Declarations:** Remote ports specified using the `--port` flag
   are container ports.
3. **No Default Port:** A `replace` can occur without intercepting any
   ports.
4. **Container State:** During a `replace`, the original container is no
   longer active within the cluster.

The deprecated `--replace` flag still works, but is hidden from the
`telepresence intercept` command help, and will print a deprecation
warning when used.

Signed-off-by: Thomas Hallgren <[email protected]>

CHANGELOG.yml

@@ -27,6 +27,42 @@ items:
   - version: 2.22.0
     date: (TBD)
     notes:
+      - type: feature
+        title: New telepresence replace command.
+        body: |-
+          The new `telepresence replace` command simplifies and clarifies container replacement.
+          
+          Previously, the `--replace` flag within the `telepresence intercept` command was used to replace containers.
+          However, this approach introduced inconsistencies and limitations:
+          
+          * **Confusion:** Using a flag to modify the core function of a command designed for traffic interception led
+            to ambiguity.
+          * **Inaccurate Behavior:** Replacement was not possible when no incoming traffic was intercepted, as the
+            command's design focused on traffic routing.
+          
+          To address these issues, the `--replace` flag within `telepresence intercept` has been deprecated. The new
+          `telepresence replace` command provides a dedicated and consistent method for replacing containers, enhancing
+          clarity and reliability.
+
+          Key differences between `replace` and `intercept`:
+          
+          1. **Scope:** The `replace` command targets and affects an entire container, impacting all its traffic, while
+             an `intercept` targets specific services and/or service/container ports.
+          2. **Port Declarations:** Remote ports specified using the `--port` flag are container ports. 
+          3. **No Default Port:** A `replace` can occur without intercepting any ports.
+          4. **Container State:** During a `replace`, the original container is no longer active within the cluster.
+          
+          The deprecated `--replace` flag still works, but is hidden from the `telepresence intercept` command help, and
+          will print a deprecation warning when used.
+      - type: feature
+        title: No dormant container present during replace.
+        body: |-
+          Telepresence will no longer inject a dormant container during a `telepresence replace` operation. Instead, the
+          Traffic Agent now directly serves as the replacement container, eliminating the need to forward traffic to the
+          original application container. This simplification offers several advantages when using the `--replace` flag:
+          
+          - **Removal of the init-container:** The need for a separate init-container is no longer necessary.
+          - **Elimination of port renames:** Port renames within the intercepted pod are no longer required.
       - type: feature
         title: One single invocation of the Telepresence intercept command can now intercept multiple ports.
         body: >-
@@ -49,16 +85,6 @@ items:
                values: <namespaces>`.
           ```
         docs: install/manager#static-versus-dynamic-namespace-selection
-      - type: feature
-        title: Removal of the dormant container during intercept with --replace.
-        body: |-
-          During a `telepresence intercept --replace operation`, the previously injected dormant container has been
-          removed. The Traffic Agent now directly serves as the replacement container, eliminating the need to forward
-          traffic to the original application container. This simplification offers several advantages when using the
-          `--replace` flag:
-          
-          - **Removal of the init-container:** The need for a separate init-container is no longer necessary.
-          - **Elimination of port renames:** Port renames within the intercepted pod are no longer required.
       - type: change
         title: Drop deprecated current-cluster-id command.
         body: >-

cmd/traffic/cmd/agent/agent.go

@@ -158,7 +158,7 @@ func sidecar(ctx context.Context, s State, info *rpc.AgentInfo) error {
 	ac := s.AgentConfig()
 	for _, cn := range ac.Containers {
 		ci := info.Containers[cn.Name]
-		s.AddContainerState(cn.Name, NewContainerState(ci.MountPoint, ci.Environment))
+		s.AddContainerState(cn.Name, NewContainerState(s, cn, ci.MountPoint, ci.Environment))
 
 		// Group the container's intercepts by agent port
 		icStates := make(map[agentconfig.PortAndProto][]*agentconfig.Intercept, len(cn.Intercepts))

cmd/traffic/cmd/agent/containerstate.go

@@ -1,21 +1,65 @@
 package agent
 
+import (
+	"context"
+
+	"github.com/datawire/dlib/dlog"
+	"github.com/telepresenceio/telepresence/rpc/v2/manager"
+	"github.com/telepresenceio/telepresence/v2/pkg/agentconfig"
+)
+
 type containerState struct {
+	State
+	container  *agentconfig.Container
 	mountPoint string
 	env        map[string]string
 }
 
-func (c containerState) MountPoint() string {
+func (c *containerState) MountPoint() string {
 	return c.mountPoint
 }
 
-func (c containerState) Env() map[string]string {
+func (c *containerState) Env() map[string]string {
 	return c.env
 }
 
+func (c *containerState) Name() string {
+	return c.container.Name
+}
+
+func (c *containerState) Replace() bool {
+	return bool(c.container.Replace)
+}
+
+// HandleIntercepts on the containerState takes care of intercepts that just replaces a container and do not declare
+// any ports. Without port declarations, there will be no Intercept entries for an fwdState to handle.
+func (c *containerState) HandleIntercepts(ctx context.Context, iis []*manager.InterceptInfo) (rs []*manager.ReviewInterceptRequest) {
+	for _, ii := range iis {
+		if ii.Disposition == manager.InterceptDispositionType_WAITING {
+			spec := ii.Spec
+			dlog.Debugf(ctx, "container %s checking replace intercept %q(%d) %s", c.Name(), spec.ContainerName, spec.ContainerPort, spec.Name)
+			if c.Replace() && c.Name() == spec.ContainerName && spec.ContainerPort == 0 {
+				dlog.Debugf(ctx, "container %s handling replace intercept %s", c.Name(), spec.Name)
+				rs = append(rs, &manager.ReviewInterceptRequest{
+					Id:          ii.Id,
+					Disposition: manager.InterceptDispositionType_ACTIVE,
+					PodIp:       c.PodIP(),
+					SftpPort:    int32(c.SftpPort()),
+					FtpPort:     int32(c.FtpPort()),
+					MountPoint:  c.MountPoint(),
+					Environment: c.Env(),
+				})
+			}
+		}
+	}
+	return rs
+}
+
 // NewContainerState creates a ContainerState that provides the environment variables and the mount point for a container.
-func NewContainerState(mountPoint string, env map[string]string) ContainerState {
+func NewContainerState(s State, cn *agentconfig.Container, mountPoint string, env map[string]string) ContainerState {
 	return &containerState{
+		State:      s,
+		container:  cn,
 		mountPoint: mountPoint,
 		env:        env,
 	}

cmd/traffic/cmd/agent/intercepttarget.go

@@ -39,9 +39,11 @@ func NewInterceptTarget(ics []*agentconfig.Intercept) InterceptTarget {
 }
 
 func (cp InterceptTarget) MatchForSpec(spec *manager.InterceptSpec) bool {
-	for _, ic := range cp {
-		if agentconfig.SpecMatchesIntercept(spec, ic) {
-			return true
+	if cnPort := uint16(spec.ContainerPort); cnPort > 0 {
+		for _, ic := range cp {
+			if cnPort == ic.ContainerPort {
+				return true
+			}
 		}
 	}
 	return false

cmd/traffic/cmd/agent/state.go

@@ -37,6 +37,9 @@ type State interface {
 }
 
 type ContainerState interface {
+	State
+	Name() string
+	Replace() bool
 	MountPoint() string
 	Env() map[string]string
 }
@@ -123,6 +126,9 @@ func (s *state) HandleIntercepts(ctx context.Context, iis []*manager.InterceptIn
 		}
 		rs = append(rs, ist.HandleIntercepts(ctx, ms)...)
 	}
+	for _, cn := range s.containerStates {
+		rs = append(rs, cn.HandleIntercepts(ctx, iis)...)
+	}
 	return rs
 }
 

cmd/traffic/cmd/agent/state_test.go

@@ -40,7 +40,7 @@ func makeFS(t *testing.T, ctx context.Context) (forwarder.Interceptor, agent.Sta
 	s := agent.NewState(c)
 	cn := c.AgentConfig().Containers[0]
 	cnMountPoint := filepath.Join(agentconfig.ExportsMountPoint, filepath.Base(cn.MountPoint))
-	s.AddContainerState(cn.Name, agent.NewContainerState(cnMountPoint, map[string]string{}))
+	s.AddContainerState(cn.Name, agent.NewContainerState(s, cn, cnMountPoint, map[string]string{}))
 	s.AddInterceptState(s.NewInterceptState(f, agent.NewInterceptTarget(cn.Intercepts), cn.Name))
 	return f, s
 }
@@ -81,6 +81,7 @@ func TestState_HandleIntercepts(t *testing.T) {
 				Namespace:      namespace,
 				ServiceName:    serviceName,
 				PortIdentifier: "http",
+				ContainerPort:  8080,
 				TargetPort:     8080,
 			},
 			Id: "intercept-01",
@@ -94,6 +95,7 @@ func TestState_HandleIntercepts(t *testing.T) {
 				Namespace:      namespace,
 				ServiceName:    serviceName,
 				PortIdentifier: "http",
+				ContainerPort:  8080,
 				TargetPort:     8080,
 			},
 			Id: "intercept-02",

cmd/traffic/cmd/manager/state/intercept.go

@@ -57,10 +57,10 @@ func (s *state) PrepareIntercept(
 	}()
 
 	interceptError := func(err error) (*rpc.PreparedIntercept, error) {
+		dlog.Errorf(ctx, "PrepareIntercept error %v", err)
 		if _, ok := status.FromError(err); ok {
 			return nil, err
 		}
-		dlog.Errorf(ctx, "PrepareIntercept error %v", err)
 		return &rpc.PreparedIntercept{Error: err.Error(), ErrorCategory: int32(errcat.GetCategory(err))}, nil
 	}
 
@@ -78,34 +78,65 @@ func (s *state) PrepareIntercept(
 	if err != nil {
 		return interceptError(err)
 	}
-	cn, ic, err := findIntercept(ac, spec)
+
+	pi = &rpc.PreparedIntercept{
+		Namespace:     ac.Namespace,
+		AgentImage:    ac.AgentImage,
+		WorkloadKind:  ac.WorkloadKind,
+		ContainerName: spec.ContainerName,
+		ServiceName:   spec.ServiceName,
+	}
+
+	var cn *agentconfig.Container
+	if spec.NoDefaultPort {
+		if cn, err = findAgent(ac, spec); err == nil {
+			pi.ContainerName = cn.Name
+			pi.ServiceName = ""
+		}
+		if spec.PortIdentifier != "" {
+			err = s.preparePorts(ac, cr, pi)
+		}
+	} else {
+		err = s.preparePorts(ac, cr, pi)
+	}
+
 	if err != nil {
-		return interceptError(err)
+		return interceptError(errcat.User.New(err))
 	}
+	return pi, nil
+}
+
+func (s *state) preparePorts(ac *agentconfig.Sidecar, cr *rpc.CreateInterceptRequest, pi *rpc.PreparedIntercept) error {
+	spec := cr.InterceptSpec
+	cn, ic, err := findIntercept2(ac, pi.ServiceName, pi.ContainerName, agentconfig.PortIdentifier(spec.PortIdentifier))
+	if err != nil {
+		return err
+	}
+
 	uniqueContainerPorts := make(map[agentconfig.PortAndProto]struct{})
 	uniqueContainerPorts[agentconfig.PortAndProto{Proto: ic.Protocol, Port: ic.ContainerPort}] = struct{}{}
 
 	var podPorts []string
 	if len(spec.PodPorts) > 0 {
-		podPorts = make([]string, len(spec.PodPorts))
 		uniqueTargets := make(map[agentconfig.PortAndProto]struct{})
 		uniqueTargets[agentconfig.PortAndProto{Proto: ic.Protocol, Port: uint16(spec.TargetPort)}] = struct{}{}
+		podPorts = make([]string, len(spec.PodPorts))
 		for i, pms := range spec.PodPorts {
 			pm := agentconfig.PortMapping(pms)
 			_, pmIc, err := findIntercept2(ac, spec.ServiceName, spec.ContainerName, pm.From())
 			if err != nil {
-				return interceptError(err)
+				return err
 			}
 
 			to := pm.To()
 			if _, ok := uniqueTargets[to]; ok {
-				return interceptError(errcat.User.Newf("multiple port definitions targeting %s", to))
+				return fmt.Errorf("multiple port definitions targeting %s", to)
 			}
 			uniqueTargets[to] = struct{}{}
 
 			from := agentconfig.PortAndProto{Proto: pmIc.Protocol, Port: pmIc.ContainerPort}
 			if _, ok := uniqueContainerPorts[from]; ok {
-				return interceptError(errcat.User.Newf("multiple port definitions using container port %s", from))
+				return fmt.Errorf("multiple port definitions using container port %s", from)
 			}
 			uniqueContainerPorts[from] = struct{}{}
 
@@ -133,24 +164,19 @@ func (s *state) PrepareIntercept(
 							client = cps[3]
 						}
 					}
-					return interceptError(errcat.User.Newf("container port %d is already intercepted by %s, intercept %s", cp.Port, client, name))
+					return fmt.Errorf("container port %d is already intercepted by %s, intercept %s", cp.Port, client, name)
 				}
 			}
 		}
 	}
-
-	return &rpc.PreparedIntercept{
-		Namespace:       ac.Namespace,
-		ServiceUid:      string(ic.ServiceUID),
-		ServicePortName: ic.ServicePortName,
-		ContainerName:   cn.Name,
-		Protocol:        string(ic.Protocol),
-		ContainerPort:   int32(ic.ContainerPort),
-		ServicePort:     int32(ic.ServicePort),
-		AgentImage:      ac.AgentImage,
-		WorkloadKind:    ac.WorkloadKind,
-		PodPorts:        podPorts,
-	}, nil
+	pi.ContainerName = cn.Name
+	pi.ServiceUid = string(ic.ServiceUID)
+	pi.ServicePortName = ic.ServicePortName
+	pi.Protocol = string(ic.Protocol)
+	pi.ContainerPort = int32(ic.ContainerPort)
+	pi.ServicePort = int32(ic.ServicePort)
+	pi.PodPorts = podPorts
+	return nil
 }
 
 func (s *state) AddIntercept(ctx context.Context, cir *rpc.CreateInterceptRequest) (*rpc.ClientInfo, *rpc.InterceptInfo, error) {
@@ -779,6 +805,21 @@ func unmarshalConfigMapEntry(y string, name, namespace string) (agentconfig.Side
 	return scx, nil
 }
 
+// findAgent finds the container configuration that matches the given InterceptSpec.
+func findAgent(ac *agentconfig.Sidecar, spec *rpc.InterceptSpec) (foundCN *agentconfig.Container, err error) {
+	if spec.ContainerName == "" {
+		if len(ac.Containers) == 1 {
+			return ac.Containers[0], nil
+		}
+	}
+	for _, cn := range ac.Containers {
+		if spec.ContainerName == cn.Name {
+			return cn, nil
+		}
+	}
+	return nil, errcat.User.Newf("intercept %s does not uniquely identify a container in %s %s.%s", spec.Name, ac.WorkloadKind, ac.WorkloadName, ac.Namespace)
+}
+
 // findIntercept finds the intercept configuration that matches the given InterceptSpec's service/service port or container port.
 func findIntercept(ac *agentconfig.Sidecar, spec *rpc.InterceptSpec) (foundCN *agentconfig.Container, foundIC *agentconfig.Intercept, err error) {
 	return findIntercept2(ac, spec.ServiceName, spec.ContainerName, agentconfig.PortIdentifier(spec.PortIdentifier))