Merge branch 'release/v2.20' into release/v2

CHANGELOG.yml

@@ -55,6 +55,16 @@ items:
           client originate from the specified container. Additionally, if the
           `--replace` option is used, it ensures that this container is replaced.
         docs: https://telepresence.io/docs/reference/intercepts/container
+  - version: 2.20.2
+    date: 2024-10-21
+    notes:
+      - type: bugfix
+        title: Crash in traffic-manager configured with agentInjector.enabled=false
+        body: >-
+          A traffic-manager that was installed with the Helm value `agentInjector.enabled=false` crashed when a
+          client used the commands `telepresence version` or `telepresence status`. Those commands would call
+          a method on the traffic-manager that panicked if no traffic-agent was present. This method will now
+          instead return the standard `Unavailable` error code, which is expected by the caller.
   - version: 2.20.1
     date: 2024-10-10
     notes:

cmd/traffic/cmd/manager/service.go

@@ -138,9 +138,12 @@ func (*service) Version(context.Context, *empty.Empty) (*rpc.VersionInfo2, error
 }
 
 func (s *service) GetAgentImageFQN(ctx context.Context, _ *empty.Empty) (*rpc.AgentImageFQN, error) {
-	return &rpc.AgentImageFQN{
-		FQN: managerutil.GetAgentImage(ctx),
-	}, nil
+	if managerutil.AgentInjectorEnabled(ctx) {
+		return &rpc.AgentImageFQN{
+			FQN: managerutil.GetAgentImage(ctx),
+		}, nil
+	}
+	return nil, status.Error(codes.Unavailable, "")
 }
 
 func (s *service) GetLicense(context.Context, *empty.Empty) (*rpc.License, error) {

docs/release-notes.md

@@ -1,18 +1,11 @@
 
 [comment]: # (Code generated by relnotesgen. DO NOT EDIT.)
 # <img src="images/logo.png" height="64px"/> Telepresence Release Notes
-## Version 2.21.0
-## <div style="display:flex;"><img src="images/feature.png" alt="feature" style="width:30px;height:fit-content;"/><div style="display:flex;margin-left:7px;">Make usage data collection configurable using an extension point, and default to no-ops</div></div>
+## Version 2.20.2 <span style="font-size: 16px;">(October 21)</span>
+## <div style="display:flex;"><img src="images/bugfix.png" alt="bugfix" style="width:30px;height:fit-content;"/><div style="display:flex;margin-left:7px;">Crash in traffic-manager configured with agentInjector.enabled=false</div></div>
 <div style="margin-left: 15px">
 
-The OSS code-base will no longer report usage data to the proprietary collector at Ambassador Labs. The actual calls to the collector remain, but will be no-ops unless a proper collector client is installed using an extension point.
-</div>
-
-## <div style="display:flex;"><img src="images/feature.png" alt="feature" style="width:30px;height:fit-content;"/><div style="display:flex;margin-left:7px;">[Intercepts targeting a specific container](https://telepresence.io/docs/reference/intercepts/container)</div></div>
-<div style="margin-left: 15px">
-
--> In certain scenarios, the container owning the intercepted port differs from the container the intercept targets. This port owner's sole purpose is to route traffic from the service to the intended container, often using a direct localhost connection.
-This commit introduces a `--container <name>` option to the intercept command. While this option doesn't influence the port selection, it guarantees that the environment variables and mounts propagated to the client originate from the specified container. Additionally, if the `--replace` option is used, it ensures that this container is replaced.
+A traffic-manager that was installed with the Helm value `agentInjector.enabled=false` crashed when a client used the commands `telepresence version` or `telepresence status`. Those commands would call a method on the traffic-manager that panicked if no traffic-agent was present. This method will now instead return the standard `Unavailable` error code, which is expected by the caller.
 </div>
 
 ## Version 2.20.1 <span style="font-size: 16px;">(October 10)</span>

docs/release-notes.mdx

@@ -7,15 +7,10 @@ import { Note, Title, Body } from '@site/src/components/ReleaseNotes'
 [comment]: # (Code generated by relnotesgen. DO NOT EDIT.)
 
 # Telepresence Release Notes
-## Version 2.21.0
+## Version 2.20.2 <span style={{fontSize:'16px'}}>(October 21)</span>
 <Note>
-	<Title type="feature">Make usage data collection configurable using an extension point, and default to no-ops</Title>
-	<Body>The OSS code-base will no longer report usage data to the proprietary collector at Ambassador Labs. The actual calls to the collector remain, but will be no-ops unless a proper collector client is installed using an extension point.</Body>
-</Note>
-<Note>
-	<Title type="feature" docs="https://telepresence.io/docs/reference/intercepts/container">Intercepts targeting a specific container</Title>
-	<Body>-> In certain scenarios, the container owning the intercepted port differs from the container the intercept targets. This port owner's sole purpose is to route traffic from the service to the intended container, often using a direct localhost connection.
-This commit introduces a `--container <name>` option to the intercept command. While this option doesn't influence the port selection, it guarantees that the environment variables and mounts propagated to the client originate from the specified container. Additionally, if the `--replace` option is used, it ensures that this container is replaced.</Body>
+	<Title type="bugfix">Crash in traffic-manager configured with agentInjector.enabled=false</Title>
+	<Body>A traffic-manager that was installed with the Helm value `agentInjector.enabled=false` crashed when a client used the commands `telepresence version` or `telepresence status`. Those commands would call a method on the traffic-manager that panicked if no traffic-agent was present. This method will now instead return the standard `Unavailable` error code, which is expected by the caller.</Body>
 </Note>
 ## Version 2.20.1 <span style={{fontSize:'16px'}}>(October 10)</span>
 <Note>

go.mod

@@ -35,7 +35,7 @@ require (
 	github.com/spf13/cobra v1.8.1
 	github.com/spf13/pflag v1.0.5
 	github.com/stretchr/testify v1.9.0
-	github.com/telepresenceio/telepresence/rpc/v2 v2.20.1
+	github.com/telepresenceio/telepresence/rpc/v2 v2.20.2
 	github.com/vishvananda/netlink v1.3.0
 	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.55.0
 	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.55.0

integration_test/agent_injector_disabled_test.go

@@ -52,6 +52,34 @@ func (s *agentInjectorDisabledSuite) Test_AgentInjectorDisabled() {
 	s.Contains(logs, "Cluster domain derived from /etc/resolv.conf")
 }
 
+func (s *agentInjectorDisabledSuite) Test_VersionWithAgentInjectorDisabled() {
+	ctx := s.Context()
+	rq := s.Require()
+	restartCount := func() int {
+		pods := itest.RunningPods(ctx, "traffic-manager", s.ManagerNamespace())
+		if len(pods) == 1 {
+			for _, cs := range pods[0].Status.ContainerStatuses {
+				if cs.Name == "traffic-manager" {
+					return int(cs.RestartCount)
+				}
+			}
+		}
+		return -1
+	}
+	oldRestartCount := restartCount()
+	rq.GreaterOrEqual(oldRestartCount, 0)
+	s.TelepresenceConnect(ctx)
+	sr, err := itest.TelepresenceStatus(ctx)
+	itest.TelepresenceQuitOk(ctx)
+	rq.NoError(err)
+	tm := sr.TrafficManager
+	rq.NotNil(tm)
+	rq.Empty(tm.TrafficAgent)
+
+	// Verify that traffic-manager didn't crash
+	rq.Equal(oldRestartCount, restartCount())
+}
+
 func (s *agentInjectorDisabledSuite) Test_ManualAgent() {
 	s.TelepresenceConnect(s.Context())
 	defer itest.TelepresenceQuitOk(s.Context())

integration_test/injector_test.go

@@ -75,7 +75,7 @@ func (s *singleServiceSuite) Test_InterceptOperationRestoredAfterFailingInject()
 	}()
 
 	oneContainer := func() bool {
-		pods := itest.RunningPods(ctx, s.ServiceName(), s.AppNamespace())
+		pods := itest.RunningPodNames(ctx, s.ServiceName(), s.AppNamespace())
 		if len(pods) != 1 {
 			dlog.Infof(ctx, "got %d pods", len(pods))
 			return false

integration_test/itest/cluster.go

@@ -12,6 +12,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 	"sync"
@@ -469,7 +470,7 @@ func (s *cluster) RootdPProf() uint16 {
 func (s *cluster) CapturePodLogs(ctx context.Context, app, container, ns string) string {
 	var pods []string
 	for i := 0; ; i++ {
-		runningPods := RunningPods(ctx, app, ns)
+		runningPods := RunningPodNames(ctx, app, ns)
 		if len(runningPods) > 0 {
 			if container == "" {
 				pods = runningPods
@@ -808,7 +809,7 @@ func (s *cluster) UninstallTrafficManager(ctx context.Context, managerNamespace
 	TelepresenceOk(ctx, append([]string{"helm", "uninstall", "--manager-namespace", managerNamespace}, args...)...)
 
 	// Helm uninstall does deletions asynchronously, so let's wait until the deployment is gone
-	assert.Eventually(t, func() bool { return len(RunningPods(ctx, "traffic-manager", managerNamespace)) == 0 },
+	assert.Eventually(t, func() bool { return len(RunningPodNames(ctx, "traffic-manager", managerNamespace)) == 0 },
 		60*time.Second, 4*time.Second, "traffic-manager deployment was not removed")
 	TelepresenceQuitOk(ctx)
 }
@@ -1244,13 +1245,8 @@ func WithKubeConfig(ctx context.Context, cfg *api.Config) context.Context {
 	return WithEnv(ctx, map[string]string{"KUBECONFIG": kubeconfigFileName})
 }
 
-// RunningPods return the names of running pods with app=<service name>. Running here means
-// that at least one container is still running. I.e. the pod might well be terminating
-// but still considered running.
-func RunningPods(ctx context.Context, svc, ns string) []string {
-	out, err := KubectlOut(ctx, ns, "get", "pods", "-o", "json",
-		"--field-selector", "status.phase==Running",
-		"-l", "app="+svc)
+func RunningPods(ctx context.Context, svc, ns string) []core.Pod {
+	out, err := KubectlOut(ctx, ns, "get", "pods", "-o", "json", "--field-selector", "status.phase==Running", "-l", "app="+svc)
 	if err != nil {
 		getT(ctx).Log(err.Error())
 		return nil
@@ -1260,19 +1256,28 @@ func RunningPods(ctx context.Context, svc, ns string) []string {
 		getT(ctx).Log(err.Error())
 		return nil
 	}
-	pods := make([]string, 0, len(pm.Items))
-nextPod:
-	for _, pod := range pm.Items {
+	return slices.DeleteFunc(pm.Items, func(pod core.Pod) bool {
 		for _, cn := range pod.Status.ContainerStatuses {
 			if r := cn.State.Running; r != nil && !r.StartedAt.IsZero() {
 				// At least one container is still running.
-				pods = append(pods, pod.Name)
-				continue nextPod
+				return false
 			}
 		}
-	}
-	dlog.Infof(ctx, "Running pods %v", pods)
-	return pods
+		return true
+	})
+}
+
+// RunningPodNames return the names of running pods with app=<service name>. Running here means
+// that at least one container is still running. I.e. the pod might well be terminating
+// but still considered running.
+func RunningPodNames(ctx context.Context, svc, ns string) []string {
+	pods := RunningPods(ctx, svc, ns)
+	podNames := make([]string, len(pods))
+	for i := range pods {
+		podNames[i] = pods[i].Name
+	}
+	dlog.Infof(ctx, "Running pods %v", podNames)
+	return podNames
 }
 
 // RunningPodsWithAgents returns the names of running pods with a matching appPrefix that

integration_test/limitrange_test.go

@@ -23,7 +23,7 @@ func (is *installSuite) limitedRangeTest() {
 	itest.ApplyEchoService(ctx, svc, is.AppNamespace(), 8083)
 	defer func() {
 		is.NoError(itest.Kubectl(ctx, is.AppNamespace(), "delete", "svc,deploy", svc))
-		is.Eventually(func() bool { return len(itest.RunningPods(ctx, svc, is.AppNamespace())) == 0 }, 2*time.Minute, 6*time.Second)
+		is.Eventually(func() bool { return len(itest.RunningPodNames(ctx, svc, is.AppNamespace())) == 0 }, 2*time.Minute, 6*time.Second)
 	}()
 
 	_, _, err := itest.Telepresence(ctx, "intercept", "--mount", "false", svc)

integration_test/podscaling_test.go

@@ -161,5 +161,5 @@ func (s *interceptMountSuite) Test_StopInterceptedPodOfMany() {
 // that at least one container is still running. I.e. the pod might well be terminating
 // but still considered running.
 func (s *interceptMountSuite) runningPods(ctx context.Context) []string {
-	return itest.RunningPods(ctx, s.ServiceName(), s.AppNamespace())
+	return itest.RunningPodNames(ctx, s.ServiceName(), s.AppNamespace())
 }

pkg/vif/testdata/router/go.mod

@@ -51,7 +51,7 @@ require (
 	github.com/rogpeppe/go-internal v1.12.0 // indirect
 	github.com/spf13/cobra v1.8.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
-	github.com/telepresenceio/telepresence/rpc/v2 v2.20.1 // indirect
+	github.com/telepresenceio/telepresence/rpc/v2 v2.20.2 // indirect
 	github.com/vishvananda/netlink v1.3.0 // indirect
 	github.com/vishvananda/netns v0.0.4 // indirect
 	github.com/x448/float16 v0.8.4 // indirect