diff --git a/CHANGELOG.yml b/CHANGELOG.yml index ace302eb87..1c2de66223 100644 --- a/CHANGELOG.yml +++ b/CHANGELOG.yml @@ -35,21 +35,30 @@ items: notes: - type: bugfix title: The DNS search path on Windows is now restored when Telepresence quits - body: The DNS search path that Telepresence uses to simulate the DNS lookup functionality in the connected + body: >- + The DNS search path that Telepresence uses to simulate the DNS lookup functionality in the connected cluster namespace was not removed by a telepresence quit, resulting in connectivity problems from the workstation. Telepresence will now remove the entries that it has added to the search list when it quits. + - type: bugfix + title: The user-daemon would sometimes get killed when used by multiple simultaneous CLI clients. + body: >- + The user-daemon would die with a fatal "fatal error: concurrent map writes" error in the + connector.log, effectively killing the ongoing connection. - type: bugfix title: Multiple services ports using the same target port would not get intercepted correctly. - body: Intercepts didn't work when multiple service ports were using the same container port. Telepresence would + body: >- + Intercepts didn't work when multiple service ports were using the same container port. Telepresence would think that one of the ports wasn't intercepted and therefore disable the intercept of the container port. - type: bugfix title: Root daemon refuses to disconnect. - body: The root daemon would sometimes hang forever when attempting to disconnect due to a deadlock in + body: >- + The root daemon would sometimes hang forever when attempting to disconnect due to a deadlock in the VIF-device. - type: bugfix title: Fix panic in user daemon when traffic-manager was unreachable - body: The user daemon would panic if the traffic-manager was unreachable. It will now instead report + body: >- + The user daemon would panic if the traffic-manager was unreachable. It will now instead report a proper error to the client. - type: change title: Removal of backward support for versions predating 2.6.0 diff --git a/integration_test/helm_test.go b/integration_test/helm_test.go index de319d7799..fce68d1886 100644 --- a/integration_test/helm_test.go +++ b/integration_test/helm_test.go @@ -118,7 +118,7 @@ func (s *helmSuite) Test_HelmMultipleInstalls() { s.Contains(stdout, "Connected to context") s.Eventually(func() bool { return itest.Run(ctx, "curl", "--silent", "--connect-timeout", "1", fmt.Sprintf("%s.%s", svc, s.appSpace2)) == nil - }, 7*time.Second, 1*time.Second) + }, 15*time.Second, 3*time.Second) }) s.Run("Can intercept", func() { diff --git a/pkg/client/scout/os_metadata_darwin.go b/pkg/client/scout/os_metadata_darwin.go index e07cd5c0e5..1148ae9bda 100644 --- a/pkg/client/scout/os_metadata_darwin.go +++ b/pkg/client/scout/os_metadata_darwin.go @@ -10,12 +10,10 @@ import ( "github.com/datawire/dlib/dlog" ) -func getOsMetadata(ctx context.Context) map[string]any { - osMeta := map[string]any{ - "os_version": "unknown", - "os_build_version": "unknown", - "os_name": "unknown", - } +func setOsMetadata(ctx context.Context, osMeta map[string]any) { + osMeta["os_version"] = "unknown" + osMeta["os_build_version"] = "unknown" + osMeta["os_name"] = "unknown" cmd := dexec.CommandContext(ctx, "sw_vers") cmd.DisableLogging = true if r, err := cmd.Output(); err != nil { @@ -36,5 +34,4 @@ func getOsMetadata(ctx context.Context) map[string]any { } } } - return osMeta } diff --git a/pkg/client/scout/os_metadata_linux.go b/pkg/client/scout/os_metadata_linux.go index d2b31d3b43..404bfc40df 100644 --- a/pkg/client/scout/os_metadata_linux.go +++ b/pkg/client/scout/os_metadata_linux.go @@ -28,8 +28,7 @@ func isWSL(ctx context.Context) bool { return strings.Contains(v, "WSL") || strings.Contains(v, "Windows") } -func getOsMetadata(ctx context.Context) map[string]any { - osMeta := map[string]any{} +func setOsMetadata(ctx context.Context, osMeta map[string]any) { osMeta["os_docker"] = isDocker(ctx) osMeta["os_wsl"] = isWSL(ctx) f, err := os.Open("/etc/os-release") @@ -38,7 +37,7 @@ func getOsMetadata(ctx context.Context) map[string]any { } if err != nil { dlog.Warnf(ctx, "Unable to open /etc/os-release or /usr/lib/os-release: %v", err) - return osMeta + return } scanner := bufio.NewScanner(f) osRelease := map[string]string{} @@ -49,7 +48,7 @@ func getOsMetadata(ctx context.Context) map[string]any { } if err := scanner.Err(); err != nil { dlog.Warnf(ctx, "Unable to scan contents of /etc/os-release: %v", err) - return osMeta + return } // Different Linuxes will report things in different ways, so this will scan the // contents of osRelease and look for each of the different keys that a value might be under @@ -65,5 +64,4 @@ func getOsMetadata(ctx context.Context) map[string]any { osMeta["os_name"] = getFromOSRelease("ID", "NAME") osMeta["os_version"] = getFromOSRelease("VERSION", "VERSION_ID") osMeta["os_build_version"] = getFromOSRelease("BUILD_ID") - return osMeta } diff --git a/pkg/client/scout/os_metadata_test.go b/pkg/client/scout/os_metadata_test.go index 28c4468f7a..d5b82c7178 100644 --- a/pkg/client/scout/os_metadata_test.go +++ b/pkg/client/scout/os_metadata_test.go @@ -8,7 +8,8 @@ import ( func TestOsMetadata(t *testing.T) { ctx := dlog.NewTestContext(t, false) - osMeta := getOsMetadata(ctx) + osMeta := make(map[string]any) + setOsMetadata(ctx, osMeta) for _, k := range []string{"os_version", "os_name"} { v := osMeta[k] if v == "" || v == "unknown" { diff --git a/pkg/client/scout/os_metadata_windows.go b/pkg/client/scout/os_metadata_windows.go index 4c18ceb35d..17d5e6d1f1 100644 --- a/pkg/client/scout/os_metadata_windows.go +++ b/pkg/client/scout/os_metadata_windows.go @@ -10,14 +10,13 @@ import ( "github.com/telepresenceio/telepresence/v2/pkg/proc" ) -func getOsMetadata(ctx context.Context) map[string]any { +func setOsMetadata(ctx context.Context, osMeta map[string]any) { cmd := proc.CommandContext(ctx, "wmic", "os", "get", "Caption,Version,BuildNumber", "/value") cmd.DisableLogging = true r, err := cmd.Output() - osMeta := map[string]any{} if err != nil { dlog.Warnf(ctx, "Error running wmic: %v", err) - return osMeta + return } scanner := bufio.NewScanner(bytes.NewReader(r)) for scanner.Scan() { @@ -42,5 +41,4 @@ func getOsMetadata(ctx context.Context) map[string]any { if err := scanner.Err(); err != nil { dlog.Warnf(ctx, "Unable to scan wmic output: %v", err) } - return osMeta } diff --git a/pkg/client/scout/reporter.go b/pkg/client/scout/reporter.go index 20df815ded..8218976436 100644 --- a/pkg/client/scout/reporter.go +++ b/pkg/client/scout/reporter.go @@ -69,9 +69,11 @@ var idFiles = map[InstallType]string{ //nolint:gochecknoglobals // constant Docker: "docker_id", } -// getInstallIDFromFilesystem returns the telepresence install ID, and also sets the reporter base -// metadata to include any conflicting install IDs written by old versions of the product. -func getInstallIDFromFilesystem(ctx context.Context, reporter *metriton.Reporter, installType InstallType) (string, error) { +// setInstallIDFromFilesystem sets the telepresence install ID in the given map, including any conflicting +// install IDs written by old versions of the product. +// +//nolint:gochecknoglobals // can be overridden for test purposes +var setInstallIDFromFilesystem = func(ctx context.Context, installType InstallType, md map[string]any) (string, error) { type filecacheEntry struct { Body string Err error @@ -146,7 +148,7 @@ func getInstallIDFromFilesystem(ctx context.Context, reporter *metriton.Reporter } } - reporter.BaseMetadata["new_install"] = len(allIDs) == 0 + md["new_install"] = len(allIDs) == 0 // We don't want to add the extra ids until we've decided if it's a new install or not // this is because we'd like a new install of type A to be reported even if there's already @@ -167,7 +169,7 @@ func getInstallIDFromFilesystem(ctx context.Context, reporter *metriton.Reporter for product, id := range allIDs { if id != retID { - reporter.BaseMetadata["install_id_"+product] = id + md["install_id_"+product] = id } } return retID, nil @@ -178,23 +180,28 @@ func getInstallIDFromFilesystem(ctx context.Context, reporter *metriton.Reporter const bufferSize = 40 func NewReporterForInstallType(ctx context.Context, mode string, installType InstallType, reportAnnotators []ReportAnnotator, reportMutators []ReportMutator) Reporter { - r := &reporter{ - reporter: &metriton.Reporter{ - Application: "telepresence2", - Version: client.Version(), - GetInstallID: func(r *metriton.Reporter) (string, error) { - id, err := getInstallIDFromFilesystem(ctx, r, installType) - if err != nil { - id = "00000000-0000-0000-0000-000000000000" - r.BaseMetadata["new_install"] = true - r.BaseMetadata["install_id_error"] = err.Error() - } - return id, nil - }, - }, - reportAnnotators: reportAnnotators, - reportMutators: reportMutators, + md := make(map[string]any, 12) + setOsMetadata(ctx, md) + installID, err := setInstallIDFromFilesystem(ctx, installType, md) + if err != nil { + installID = "00000000-0000-0000-0000-000000000000" + md["new_install"] = true + md["install_id_error"] = err.Error() } + // Fixed (growing) metadata passed with every report + md["mode"] = mode + md["trace_id"] = uuid.NewString() // It's sent as JSON so might as well convert it to a string once here. + md["goos"] = runtime.GOOS + md["goarch"] = runtime.GOARCH + + // Discover how Telepresence was installed based on the binary's location + installMethod, err := client.GetInstallMechanism() + if err != nil { + dlog.Errorf(ctx, "scout error getting executable: %s", err) + } + md["install_method"] = installMethod + setDefaultEnvironmentMetadata(md) + if env := client.GetEnv(ctx); env != nil && !env.ScoutDisable { // Some tests disable scout reporting by setting the host IP to 127.0.0.1. This spams // the logs with lots of "connection refused" messages and makes them hard to read. @@ -208,8 +215,21 @@ func NewReporterForInstallType(ctx context.Context, mode string, installType Ins } } } - r.initialize(ctx, mode, runtime.GOOS, runtime.GOARCH) - return r + + return &reporter{ + reporter: &metriton.Reporter{ + Application: "telepresence2", + Version: client.Version(), + GetInstallID: func(r *metriton.Reporter) (string, error) { + return installID, nil + }, + BaseMetadata: md, + }, + reportAnnotators: reportAnnotators, + reportMutators: reportMutators, + buffer: make(chan bufEntry, bufferSize), + done: make(chan struct{}), + } } // DefaultReportAnnotators are the default annotator functions that the NewReporter function will pass to NewReporterForInstallType. @@ -267,30 +287,6 @@ func SetMetadatum(ctx context.Context, key string, value any) { } } -// initialization broken out or constructor for the benefit of testing. -func (r *reporter) initialize(ctx context.Context, mode, goos, goarch string) { - r.buffer = make(chan bufEntry, bufferSize) - r.done = make(chan struct{}) - - // Fixed (growing) metadata passed with every report - baseMeta := getOsMetadata(ctx) - baseMeta["mode"] = mode - baseMeta["trace_id"] = uuid.NewString() // It's sent as JSON so might as well convert it to a string once here. - baseMeta["goos"] = goos - baseMeta["goarch"] = goarch - - // Discover how Telepresence was installed based on the binary's location - installMethod, err := client.GetInstallMechanism() - if err != nil { - dlog.Errorf(ctx, "scout error getting executable: %s", err) - } - baseMeta["install_method"] = installMethod - for k, v := range getDefaultEnvironmentMetadata() { - baseMeta[k] = v - } - r.reporter.BaseMetadata = baseMeta -} - func (r *reporter) InstallID() string { return r.reporter.InstallID() } @@ -399,9 +395,8 @@ func (r *reporter) doReport(ctx context.Context, be *bufEntry) { } } -// Returns a metadata map containing all the additional environment variables to be reported. -func getDefaultEnvironmentMetadata() map[string]string { - metadata := map[string]string{} +// setDefaultEnvironmentMetadata sets all the additional environment variables to be reported. +func setDefaultEnvironmentMetadata(metadata map[string]any) { for _, e := range os.Environ() { pair := strings.SplitN(e, "=", 2) if strings.HasPrefix(pair[0], EnvironmentMetadataPrefix) { @@ -409,5 +404,4 @@ func getDefaultEnvironmentMetadata() map[string]string { metadata[key] = pair[1] } } - return metadata } diff --git a/pkg/client/scout/reporter_test.go b/pkg/client/scout/reporter_test.go index b0ba6baaac..99034137b2 100644 --- a/pkg/client/scout/reporter_test.go +++ b/pkg/client/scout/reporter_test.go @@ -334,13 +334,9 @@ func TestInstallID(t *testing.T) { func TestReport(t *testing.T) { const ( - mockVersion = "v2.4.5-test" - mockApplication = "telepresence2" - mockInstallID = "00000000-1111-2222-3333-444444444444" - mockMode = "test-mode" - mockOS = "linux" - mockARCH = "amd64" - mockAction = "test-action" + mockInstallID = "00000000-1111-2222-3333-444444444444" + mockMode = "test-mode" + mockAction = "test-action" ) type testcase struct { InputEnv map[string]string @@ -353,8 +349,8 @@ func TestReport(t *testing.T) { ExpectedMetadata: map[string]any{ "action": mockAction, "mode": mockMode, - "goos": mockOS, - "goarch": mockARCH, + "goos": runtime.GOOS, + "goarch": runtime.GOARCH, }, }, "with-additional-scout-meta": { @@ -371,8 +367,8 @@ func TestReport(t *testing.T) { ExpectedMetadata: map[string]any{ "action": mockAction, "mode": mockMode, - "goos": mockOS, - "goarch": mockARCH, + "goos": runtime.GOOS, + "goarch": runtime.GOARCH, "extra_field_1": "extra value 1", "extra_field_2": "extra value 2", }, @@ -385,8 +381,8 @@ func TestReport(t *testing.T) { ExpectedMetadata: map[string]any{ "action": mockAction, "mode": mockMode, - "goos": mockOS, - "goarch": mockARCH, + "goos": runtime.GOOS, + "goarch": runtime.GOARCH, "extra_field_1": "extra value 1", "extra_field_2": "extra value 2", }, @@ -405,8 +401,8 @@ func TestReport(t *testing.T) { ExpectedMetadata: map[string]any{ "action": mockAction, "mode": mockMode, - "goos": mockOS, - "goarch": mockARCH, + "goos": runtime.GOOS, + "goarch": runtime.GOARCH, "extra_field_1": "extra value 1", }, }, @@ -420,8 +416,8 @@ func TestReport(t *testing.T) { ExpectedMetadata: map[string]any{ "action": mockAction, "mode": "overridden mode", - "goos": mockOS, - "goarch": mockARCH, + "goos": runtime.GOOS, + "goarch": runtime.GOARCH, }, }, "with-report-annotators": { @@ -445,8 +441,8 @@ func TestReport(t *testing.T) { ExpectedMetadata: map[string]any{ "action": "overridden action", "mode": "overridden mode", - "goos": mockOS, - "goarch": mockARCH, + "goos": runtime.GOOS, + "goarch": runtime.GOARCH, "extra_field": "extra value", // Not overridden by annotation "annotation": "annotated value", }, @@ -488,19 +484,12 @@ func TestReport(t *testing.T) { for k, v := range tcData.InputEnv { os.Setenv(k, v) } - scout := &reporter{ - buffer: make(chan bufEntry, 40), - reporter: &metriton.Reporter{ - Application: mockApplication, - Version: mockVersion, - GetInstallID: func(r *metriton.Reporter) (string, error) { - return mockInstallID, nil - }, - Endpoint: testServer.URL, - }, - reportAnnotators: tcData.ReportAnnotators, + + setInstallIDFromFilesystem = func(ctx context.Context, installType InstallType, md map[string]any) (string, error) { + return mockInstallID, nil } - scout.initialize(ctx, mockMode, mockOS, mockARCH) + scout := NewReporterForInstallType(ctx, mockMode, CLI, tcData.ReportAnnotators, nil).(*reporter) + scout.reporter.Endpoint = testServer.URL // Start scout report processing... sc, cancel := context.WithCancel(dcontext.WithSoftness(ctx))