Merge branch 'release/v2' into pkg-import

CHANGELOG.yml

@@ -33,6 +33,13 @@ items:
   - version: 2.16.0
     date: (TBD)
     notes:
+      - type: bugfix
+        title: Stop lingering daemon container
+        body: >-
+          When using <code>telepresence connect --docker</code>, a lingering container could be present, causing errors
+          like &quot;The container name NN is already in use by container XX ...&quot;. When this happens, the connect
+          logic will now give the container some time to stop and then call <code>docker stop NN</code> to stop it
+          before retrying to start it.
       - type: bugfix
         title: Add file locking to the Telepresence cache
         body: >-

integration_test/intercept_env_test.go

@@ -2,6 +2,7 @@ package integration
 
 import (
 	"os"
+	"path/filepath"
 
 	"github.com/telepresenceio/telepresence/v2/integration_test/itest"
 )
@@ -16,35 +17,33 @@ func (s *interceptEnvSuite) SuiteName() string {
 }
 
 func init() {
-	itest.AddTrafficManagerSuite("", func(h itest.NamespacePair) itest.TestingSuite {
+	itest.AddNamespacePairSuite("", func(h itest.NamespacePair) itest.TestingSuite {
 		return &interceptEnvSuite{Suite: itest.Suite{Harness: h}, NamespacePair: h}
 	})
 }
 
-func (s *interceptEnvSuite) TearDownTest() {
-	itest.TelepresenceQuitOk(s.Context())
-}
-
 func (s *interceptEnvSuite) Test_ExcludeVariables() {
 	// given
 	ctx := s.Context()
-	err := s.TelepresenceHelmInstall(ctx, true, "--set", "intercept.environment.excluded={DATABASE_HOST,DATABASE_PASSWORD}")
-	s.Assert().NoError(err)
-	s.ApplyApp(ctx, "echo_with_env", "deploy/echo-easy")
+	err := s.TelepresenceHelmInstall(ctx, false, "--set", "intercept.environment.excluded={DATABASE_HOST,DATABASE_PASSWORD}")
+	s.Require().NoError(err)
+	defer s.UninstallTrafficManager(ctx, s.ManagerNamespace())
 
+	s.ApplyApp(ctx, "echo_with_env", "deploy/echo-easy")
 	defer s.DeleteSvcAndWorkload(ctx, "deploy", "echo-easy")
-	defer os.RemoveAll("echo.env") //nolint:errcheck // dont need to catch the err
+
+	helloEnv := filepath.Join(s.T().TempDir(), "echo.env")
 
 	// when
 	s.TelepresenceConnect(ctx)
-	itest.TelepresenceOk(ctx, "intercept", "echo-easy", "--env-file", "echo.env")
+	itest.TelepresenceOk(ctx, "intercept", "echo-easy", "--env-file", helloEnv)
 
 	// then
-	file, err := os.ReadFile("echo.env")
-	s.Assert().NoError(err)
+	file, err := os.ReadFile(helloEnv)
+	s.Require().NoError(err)
 
-	s.Assert().NotContains(string(file), "DATABASE_HOST")
-	s.Assert().NotContains(string(file), "DATABASE_PASSWORD")
-	s.Assert().Contains(string(file), "TEST=DATA")
-	s.Assert().Contains(string(file), "INTERCEPT=ENV")
+	s.NotContains(string(file), "DATABASE_HOST")
+	s.NotContains(string(file), "DATABASE_PASSWORD")
+	s.Contains(string(file), "TEST=DATA")
+	s.Contains(string(file), "INTERCEPT=ENV")
 }

pkg/client/cli/connect/connector.go

@@ -58,8 +58,8 @@ func UserDaemonDisconnect(ctx context.Context, quitDaemons bool) (err error) {
 		}
 		// Disconnect is not implemented so daemon predates 2.4.9. Force a quit
 	}
-	if _, err = ud.Quit(ctx, &emptypb.Empty{}); err == nil || status.Code(err) == codes.Unavailable {
-		err = socket.WaitUntilVanishes("user daemon", socket.UserDaemonPath(ctx), 5*time.Second)
+	if _, err = ud.Quit(ctx, &emptypb.Empty{}); !ud.Remote() && (err == nil || status.Code(err) == codes.Unavailable) {
+		_ = socket.WaitUntilVanishes("user daemon", socket.UserDaemonPath(ctx), 5*time.Second)
 	}
 	if err != nil && status.Code(err) == codes.Unavailable {
 		if quitDaemons {
@@ -165,29 +165,36 @@ func newUserDaemon(conn *grpc.ClientConn, daemonID *daemon.Identifier) *daemon.U
 	}
 }
 
-func ensureUserDaemon(ctx context.Context, required bool) (context.Context, error) {
+func EnsureUserDaemon(ctx context.Context, required bool) (context.Context, error) {
+	var err error
+	defer func() {
+		// The RootDaemon must be started if the UserClient was started
+		if err == nil {
+			err = ensureRootDaemonRunning(ctx)
+		}
+	}()
+
 	if daemon.GetUserClient(ctx) != nil {
 		return ctx, nil
 	}
 	var ud *daemon.UserClient
 	if addr := client.GetEnv(ctx).UserDaemonAddress; addr != "" {
 		// Assume that the user daemon is running and connect to it using the given address instead of using a socket.
 		// NOTE: The UserDaemonAddress does not imply that the daemon runs in Docker
-		conn, err := grpc.DialContext(ctx, addr,
+		var conn *grpc.ClientConn
+		conn, err = grpc.DialContext(ctx, addr,
 			grpc.WithTransportCredentials(insecure.NewCredentials()),
 			grpc.WithNoProxy(),
 			grpc.WithBlock(),
 			grpc.FailOnNonTempDialError(true))
-		if err != nil {
-			return ctx, err
+		if err == nil {
+			ud = newUserDaemon(conn, nil)
 		}
-		ud = newUserDaemon(conn, nil)
 	} else {
-		var err error
 		ctx, ud, err = launchConnectorDaemon(ctx, client.GetExe(), required)
-		if err != nil {
-			return ctx, err
-		}
+	}
+	if err != nil {
+		return ctx, err
 	}
 	return daemon.WithUserClient(ctx, ud), nil
 }
@@ -197,26 +204,24 @@ func ensureDaemonVersion(ctx context.Context) error {
 	return versionCheck(ctx, client.GetExe(), daemon.GetUserClient(ctx))
 }
 
-func ensureSession(cmd *cobra.Command, required bool) error {
-	ctx := cmd.Context()
+func EnsureSession(ctx context.Context, useLine string, required bool) (context.Context, error) {
 	if daemon.GetSession(ctx) != nil {
-		return nil
+		return ctx, nil
 	}
-	s, err := connectSession(cmd, daemon.GetUserClient(ctx), daemon.GetRequest(ctx), required)
+	s, err := connectSession(ctx, useLine, daemon.GetUserClient(ctx), daemon.GetRequest(ctx), required)
 	if err != nil {
-		return err
+		return ctx, err
 	}
 	if s == nil {
-		return nil
+		return ctx, nil
 	}
 	if dns := s.Info.GetDaemonStatus().GetOutboundConfig().GetDns(); dns != nil && dns.Error != "" {
 		ioutil.Printf(output.Err(ctx), "Warning: %s\n", dns.Error)
 	}
-	cmd.SetContext(daemon.WithSession(ctx, s))
-	return nil
+	return daemon.WithSession(ctx, s), nil
 }
 
-func connectSession(cmd *cobra.Command, userD *daemon.UserClient, request *daemon.Request, required bool) (*daemon.Session, error) {
+func connectSession(ctx context.Context, useLine string, userD *daemon.UserClient, request *daemon.Request, required bool) (*daemon.Session, error) {
 	var ci *connector.ConnectInfo
 	var err error
 	if userD.Remote() {
@@ -225,7 +230,6 @@ func connectSession(cmd *cobra.Command, userD *daemon.UserClient, request *daemo
 		delete(request.Environment, "-KUBECONFIG")
 	}
 	cat := errcat.Unknown
-	ctx := cmd.Context()
 
 	session := func(ci *connector.ConnectInfo, started bool) *daemon.Session {
 		// Update the request from the connect info.
@@ -270,13 +274,14 @@ func connectSession(cmd *cobra.Command, userD *daemon.UserClient, request *daemo
 			_, _ = fmt.Fprintf(output.Info(ctx),
 				`Warning: You are executing the %q command without a preceding "telepresence connect", causing an implicit `+
 					"connect to take place. The implicit connect behavior is deprecated and will be removed in a future release.\n",
-				cmd.UseLine())
+				useLine)
 		}
 	}
 
 	if !required {
 		return nil, nil
 	}
+
 	if ci, err = userD.Connect(ctx, &request.ConnectRequest); err != nil {
 		return nil, err
 	}

pkg/client/cli/connect/init_command.go

@@ -43,22 +43,14 @@ func CommandInitializer(cmd *cobra.Command) (err error) {
 			flags.DeprecationIfChanged(cmd, global.FlagDocker, "use telepresence connect to initiate the connection")
 			flags.DeprecationIfChanged(cmd, global.FlagContext, "use telepresence connect to initiate the connection")
 		}
-		if ctx, err = ensureUserDaemon(ctx, v == ann.Required); err != nil {
+		if ctx, err = EnsureUserDaemon(ctx, v == ann.Required); err != nil {
 			if v == ann.Optional && (err == ErrNoUserDaemon || errcat.GetCategory(err) == errcat.Config) {
 				// This is OK, but further initialization is not possible
 				err = nil
 			}
 			return err
 		}
 		cmd.SetContext(ctx)
-
-		// RootDaemon == Optional means that the RootDaemon must be started if
-		// the UserClient was started
-		if _, ok := as[ann.RootDaemon]; ok {
-			if err = ensureRootDaemonRunning(ctx); err != nil {
-				return err
-			}
-		}
 	} else {
 		// The rest requires a user daemon
 		return nil
@@ -70,9 +62,11 @@ func CommandInitializer(cmd *cobra.Command) (err error) {
 	}
 
 	if v := as[ann.Session]; v == ann.Optional || v == ann.Required {
-		if err = ensureSession(cmd, v == ann.Required); err != nil {
+		ctx, err = EnsureSession(ctx, cmd.UseLine(), v == ann.Required)
+		if err != nil {
 			return err
 		}
+		cmd.SetContext(ctx)
 	}
 	return nil
 }

pkg/client/cli/daemon/request.go

@@ -152,13 +152,8 @@ func GetRequest(ctx context.Context) *Request {
 }
 
 func WithDefaultRequest(ctx context.Context, cmd *cobra.Command) (context.Context, error) {
-	cr := Request{
-		ConnectRequest: connector.ConnectRequest{
-			KubeFlags: make(map[string]string),
-		},
-		Implicit:   true,
-		kubeConfig: genericclioptions.NewConfigFlags(false),
-	}
+	cr := NewDefaultRequest()
+	cr.Implicit = true
 	cr.kubeConfig.Context = nil // --context is global
 
 	// Handle deprecated namespace flag, but allow it in the list command.
@@ -172,8 +167,22 @@ func WithDefaultRequest(ctx context.Context, cmd *cobra.Command) (context.Contex
 	if err := cr.setGlobalConnectFlags(cmd); err != nil {
 		return ctx, err
 	}
+	return WithRequest(ctx, cr), nil
+}
+
+func WithRequest(ctx context.Context, cr *Request) context.Context {
+	return context.WithValue(ctx, requestKey{}, cr)
+}
+
+func NewDefaultRequest() *Request {
+	cr := Request{
+		ConnectRequest: connector.ConnectRequest{
+			KubeFlags: make(map[string]string),
+		},
+		kubeConfig: genericclioptions.NewConfigFlags(false),
+	}
 	cr.addKubeconfigEnv()
-	return context.WithValue(ctx, requestKey{}, &cr), nil
+	return &cr
 }
 
 func GetKubeStartingConfig(cmd *cobra.Command) (*api.Config, error) {

pkg/client/docker/daemon.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/clientcmd/api"
 
+	"github.com/datawire/dlib/dexec"
 	"github.com/datawire/dlib/dlog"
 	"github.com/datawire/dlib/dtime"
 	"github.com/telepresenceio/telepresence/v2/pkg/authenticator/patcher"
@@ -258,11 +259,10 @@ func enableK8SAuthenticator(ctx context.Context, daemonID *daemon.Identifier) er
 	if cr.Implicit {
 		return nil
 	}
-	if kkf, ok := cr.KubeFlags["kubeconfig"]; ok && strings.HasPrefix(kkf, dockerTpCache) {
+	if kkf, ok := cr.ContainerKubeFlagOverrides["kubeconfig"]; ok && strings.HasPrefix(kkf, dockerTpCache) {
 		// Been there, done that
 		return nil
 	}
-	dlog.Debugf(ctx, "kubeflags = %v", cr.KubeFlags)
 	configFlags, err := client.ConfigFlags(cr.KubeFlags)
 	if err != nil {
 		return err
@@ -274,7 +274,7 @@ func enableK8SAuthenticator(ctx context.Context, daemonID *daemon.Identifier) er
 	}
 
 	configFiles := loader.ConfigAccess().GetLoadingPrecedence()
-	dlog.Debugf(ctx, "config = %v", configFiles)
+	dlog.Debugf(ctx, "host kubeconfig = %v", configFiles)
 	config, err := loader.RawConfig()
 	if err != nil {
 		return err
@@ -330,7 +330,10 @@ func enableK8SAuthenticator(ctx context.Context, daemonID *daemon.Identifier) er
 	}
 
 	// Concatenate using "/". This will be used in linux
-	cr.KubeFlags["kubeconfig"] = fmt.Sprintf("%s/%s/%s", dockerTpCache, kubeConfigs, kubeConfigFile)
+	if cr.ContainerKubeFlagOverrides == nil {
+		cr.ContainerKubeFlagOverrides = make(map[string]string)
+	}
+	cr.ContainerKubeFlagOverrides["kubeconfig"] = fmt.Sprintf("%s/%s/%s", dockerTpCache, kubeConfigs, kubeConfigFile)
 	return nil
 }
 
@@ -430,17 +433,27 @@ func LaunchDaemon(ctx context.Context, daemonID *daemon.Identifier) (conn *grpc.
 	allArgs = append(allArgs, opts...)
 	allArgs = append(allArgs, image)
 	allArgs = append(allArgs, args...)
+	stopAttempted := false
 	for i := 1; ; i++ {
 		_, err = tryLaunch(ctx, daemonID, addr.Port, allArgs)
 		if err != nil {
-			if i < 6 && strings.Contains(err.Error(), "already in use by container") {
-				// This may happen if the daemon has died (and hence, we never discovered it), but
-				// the container still hasn't died. Let's sleep for a short while and retry.
+			if !strings.Contains(err.Error(), "already in use by container") {
+				return nil, errcat.NoDaemonLogs.New(err)
+			}
+			// This may happen if the daemon has died (and hence, we never discovered it), but
+			// the container still hasn't died. Let's sleep for a short while and retry.
+			if i < 6 {
 				dtime.SleepWithContext(ctx, time.Duration(i)*200*time.Millisecond)
-				dlog.Debugf(ctx, "retry after: %v", err)
 				continue
 			}
-			return nil, errcat.NoDaemonLogs.New(err)
+			if stopAttempted {
+				return nil, err
+			}
+			// Container is still alive. Try and stop it.
+			stopContainer(ctx, daemonID)
+			stopAttempted = true
+			i = 1
+			continue
 		}
 		break
 	}
@@ -528,6 +541,14 @@ func detectKind(cns []types.ContainerJSON, hostAddrPort netip.AddrPort) (string,
 	return "", ""
 }
 
+func stopContainer(ctx context.Context, daemonID *daemon.Identifier) {
+	args := []string{"stop", daemonID.ContainerName()}
+	dlog.Debug(ctx, shellquote.ShellString("docker", args))
+	if _, err := proc.CaptureErr(dexec.CommandContext(ctx, "docker", args...)); err != nil {
+		dlog.Warn(ctx, err)
+	}
+}
+
 func tryLaunch(ctx context.Context, daemonID *daemon.Identifier, port int, args []string) (string, error) {
 	stdErr := bytes.Buffer{}
 	stdOut := bytes.Buffer{}

pkg/client/envconfig.go

@@ -18,9 +18,6 @@ type Env struct {
 	// This environment variable becomes the default for the images.agentImage and images.webhookAgentImage
 	AgentImage string `env:"TELEPRESENCE_AGENT_IMAGE,                   parser=possibly-empty-string,default="`
 
-	TelepresenceUID int `env:"TELEPRESENCE_UID, parser=strconv.ParseInt, default=0"`
-	TelepresenceGID int `env:"TELEPRESENCE_GID, parser=strconv.ParseInt, default=0"`
-
 	// The address that the user daemon is listening to (unless it is started by the client and uses a named pipe or unix socket).
 	UserDaemonAddress string `env:"TELEPRESENCE_USER_DAEMON_ADDRESS, parser=possibly-empty-string,default="`
 	ScoutDisable      bool   `env:"SCOUT_DISABLE, parser=strconv.ParseBool, default=0"`