From 4532e8b73baa4cefa1c242b5747875bf82e88c8f Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 22 Jul 2025 16:45:01 -0700 Subject: [PATCH 1/4] Disable race test for integration tests of client tools managed updates and run them on windows platform --- .github/workflows/integration-tests-win.yaml | 66 +++++++++++++++++++ Makefile | 4 ++ integration/autoupdate/tools/main_test.go | 2 + integration/autoupdate/tools/updater_test.go | 2 + .../autoupdate/tools/updater_tsh_test.go | 2 +- 5 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/integration-tests-win.yaml diff --git a/.github/workflows/integration-tests-win.yaml b/.github/workflows/integration-tests-win.yaml new file mode 100644 index 0000000000000..9e9c5d5869b41 --- /dev/null +++ b/.github/workflows/integration-tests-win.yaml @@ -0,0 +1,66 @@ +name: Integration Tests (Win) +run-name: Integration Tests (Win) - ${{ github.run_id }} - @${{ github.actor }} + +on: + pull_request: + merge_group: + +jobs: + changes: + name: Check for relevant changes + runs-on: ubuntu-latest + permissions: + pull-requests: read + outputs: + changed: ${{ steps.changes.outputs.changed }} + steps: + - name: Checkout + if: ${{ github.event_name == 'merge_group' }} + uses: actions/checkout@v4 + - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2 + id: changes + with: + base: ${{ github.event.pull_request.base.ref || github.event.merge_group.base_ref }} + ref: ${{ github.event.pull_request.head.ref || github.event.merge_group.head_ref }} + filters: | + changed: + - '.github/workflows/integration-tests-win.yaml' + - 'lib/autoupdate/tools/**.go' + - 'lib/utils/packaging/**.go' + - 'go.mod' + - 'go.sum' + - 'build.assets/Makefile' + - 'build.assets/Dockerfile*' + - 'Makefile' + + test: + name: Integration Tests (Win) + needs: changes + if: ${{ !startsWith(github.head_ref, 'dependabot/') && needs.changes.outputs.changed == 'true' }} + runs-on: windows-2022-16core + + permissions: + contents: read + packages: read + + steps: + - name: Checkout Teleport + uses: actions/checkout@v4 + + - name: Get Go version + id: go-version + shell: bash + run: echo "go-version=$(make -s print-go-version | tr -d '\n')" >> $GITHUB_OUTPUT + + - name: Setup Go + uses: actions/setup-go@v5 + with: + cache: false + go-version: ${{ steps.go-version.outputs.go-version }} + + - name: Run tests + shell: bash + run: | + export OS="windows" + go test -timeout 30m -json ./integration/autoupdate/tools/... + diff --git a/Makefile b/Makefile index 52a3a04ccbce0..0c9b03e05349e 100644 --- a/Makefile +++ b/Makefile @@ -1131,11 +1131,15 @@ run-etcd: .PHONY: integration integration: FLAGS ?= -v -race integration: PACKAGES = $(shell go list ./... | grep 'integration\([^s]\|$$\)' | grep -v integrations/lib/testing/integration ) +integration: PACKAGES_NO_RACE = $(shell go list ./integration/autoupdate/tools/... | grep 'integration\([^s]\|$$\)' | grep -v integrations/lib/testing/integration ) integration: $(TEST_LOG_DIR) ensure-gotestsum @echo KUBECONFIG is: $(KUBECONFIG), TEST_KUBE: $(TEST_KUBE) $(CGOFLAG) go test -timeout 30m -json -tags "$(PAM_TAG) $(FIPS_TAG) $(BPF_TAG)" $(PACKAGES) $(FLAGS) \ | tee $(TEST_LOG_DIR)/integration.json \ | gotestsum --raw-command --format=testname -- cat + $(CGOFLAG) go test -timeout 30m -json -tags "$(PAM_TAG) $(FIPS_TAG) $(BPF_TAG)" $(PACKAGES_NO_RACE) $(filter-out -race,$(FLAGS)) \ + | tee $(TEST_LOG_DIR)/integration.json \ + | gotestsum --raw-command --format=testname -- cat # # Integration tests that run Kubernetes tests in order to complete successfully diff --git a/integration/autoupdate/tools/main_test.go b/integration/autoupdate/tools/main_test.go index ec69eeeb90082..43defe7facd59 100644 --- a/integration/autoupdate/tools/main_test.go +++ b/integration/autoupdate/tools/main_test.go @@ -1,3 +1,5 @@ +//go:build !race + /* * Teleport * Copyright (C) 2024 Gravitational, Inc. diff --git a/integration/autoupdate/tools/updater_test.go b/integration/autoupdate/tools/updater_test.go index 0ef2eabb4aa41..9f95174c16bb8 100644 --- a/integration/autoupdate/tools/updater_test.go +++ b/integration/autoupdate/tools/updater_test.go @@ -1,3 +1,5 @@ +//go:build !race + /* * Teleport * Copyright (C) 2024 Gravitational, Inc. diff --git a/integration/autoupdate/tools/updater_tsh_test.go b/integration/autoupdate/tools/updater_tsh_test.go index 6e56cf168dc7a..b3d19071e9510 100644 --- a/integration/autoupdate/tools/updater_tsh_test.go +++ b/integration/autoupdate/tools/updater_tsh_test.go @@ -1,4 +1,4 @@ -//go:build !windows +//go:build !windows && !race /* * Teleport From e99694ab5be395f2b94925c2d107776839367ec0 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 22 Jul 2025 18:18:06 -0700 Subject: [PATCH 2/4] Fix tests where tool name for Windows have extension --- .github/workflows/integration-tests-win.yaml | 2 +- integration/autoupdate/tools/updater_test.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/integration-tests-win.yaml b/.github/workflows/integration-tests-win.yaml index 9e9c5d5869b41..53aac29c00969 100644 --- a/.github/workflows/integration-tests-win.yaml +++ b/.github/workflows/integration-tests-win.yaml @@ -62,5 +62,5 @@ jobs: shell: bash run: | export OS="windows" - go test -timeout 30m -json ./integration/autoupdate/tools/... + go test -timeout 30m ./integration/autoupdate/tools/... -v diff --git a/integration/autoupdate/tools/updater_test.go b/integration/autoupdate/tools/updater_test.go index 9f95174c16bb8..26824cf41d3ca 100644 --- a/integration/autoupdate/tools/updater_test.go +++ b/integration/autoupdate/tools/updater_test.go @@ -61,9 +61,9 @@ func TestUpdate(t *testing.T) { err := updater.Update(ctx, testVersions[0]) require.NoError(t, err) - tshPath, err := updater.ToolPath("tsh", testVersions[0]) + tshPath, err := updater.ToolPath(tools.DefaultClientTools()[0], testVersions[0]) require.NoError(t, err) - tctlPath, err := updater.ToolPath("tctl", testVersions[0]) + tctlPath, err := updater.ToolPath(tools.DefaultClientTools()[1], testVersions[0]) require.NoError(t, err) // Verify that the installed version is equal to requested one. @@ -107,7 +107,7 @@ func TestParallelUpdate(t *testing.T) { err := updater.Update(ctx, testVersions[0]) require.NoError(t, err) - tshPath, err := updater.ToolPath("tsh", testVersions[0]) + tshPath, err := updater.ToolPath(tools.DefaultClientTools()[0], testVersions[0]) require.NoError(t, err) // By setting the limit request next test http serving file going blocked until unlock is sent. @@ -182,7 +182,7 @@ func TestUpdateInterruptSignal(t *testing.T) { ) err := updater.Update(ctx, testVersions[0]) require.NoError(t, err) - tshPath, err := updater.ToolPath("tsh", testVersions[0]) + tshPath, err := updater.ToolPath(tools.DefaultClientTools()[0], testVersions[0]) require.NoError(t, err) var output bytes.Buffer @@ -248,7 +248,7 @@ func TestUpdateForOSSBuild(t *testing.T) { ) err := updater.Update(ctx, testVersions[0]) require.NoError(t, err) - tshPath, err := updater.ToolPath("tsh", testVersions[0]) + tshPath, err := updater.ToolPath(tools.DefaultClientTools()[0], testVersions[0]) require.NoError(t, err) // Verify that requested update is ignored by OSS build and version wasn't updated. From 39971888d8e3b77e458ac15a6f797cdbb55053a7 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 22 Jul 2025 21:30:06 -0700 Subject: [PATCH 3/4] Fix test with break event of windows --- .github/workflows/integration-tests-win.yaml | 2 +- .../autoupdate/tools/helper_unix_test.go | 7 ++++++ .../autoupdate/tools/helper_windows_test.go | 24 ++++++++++++++----- integration/autoupdate/tools/updater_test.go | 23 ++++++++++++++---- 4 files changed, 45 insertions(+), 11 deletions(-) diff --git a/.github/workflows/integration-tests-win.yaml b/.github/workflows/integration-tests-win.yaml index 53aac29c00969..edac12919d2b1 100644 --- a/.github/workflows/integration-tests-win.yaml +++ b/.github/workflows/integration-tests-win.yaml @@ -62,5 +62,5 @@ jobs: shell: bash run: | export OS="windows" - go test -timeout 30m ./integration/autoupdate/tools/... -v + go test -timeout 30m ./integration/autoupdate/tools -v diff --git a/integration/autoupdate/tools/helper_unix_test.go b/integration/autoupdate/tools/helper_unix_test.go index 61ba0766b90d4..7828f39546a2c 100644 --- a/integration/autoupdate/tools/helper_unix_test.go +++ b/integration/autoupdate/tools/helper_unix_test.go @@ -22,11 +22,18 @@ package tools_test import ( "errors" + "os/exec" "syscall" "github.com/gravitational/trace" ) +// newCommand creates command depends on platform. +func newCommand(path string, args ...string) *exec.Cmd { + cmd := exec.Command(path, args...) + return cmd +} + // sendInterrupt sends a SIGINT to the process. func sendInterrupt(pid int) error { err := syscall.Kill(pid, syscall.SIGINT) diff --git a/integration/autoupdate/tools/helper_windows_test.go b/integration/autoupdate/tools/helper_windows_test.go index b2ede9ade8c19..fe8269a964924 100644 --- a/integration/autoupdate/tools/helper_windows_test.go +++ b/integration/autoupdate/tools/helper_windows_test.go @@ -21,20 +21,32 @@ package tools_test import ( + "os/exec" "syscall" "github.com/gravitational/trace" - "golang.org/x/sys/windows" ) -var ( - kernel = windows.NewLazyDLL("kernel32.dll") - ctrlEvent = kernel.NewProc("GenerateConsoleCtrlEvent") -) +// newCommand creates command depends on platform. +func newCommand(path string, args ...string) *exec.Cmd { + cmd := exec.Command(path, args...) + cmd.SysProcAttr = &syscall.SysProcAttr{ + CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP, + } + return cmd +} // sendInterrupt sends a Ctrl-Break event to the process. func sendInterrupt(pid int) error { - r, _, err := ctrlEvent.Call(uintptr(syscall.CTRL_BREAK_EVENT), uintptr(pid)) + d, err := syscall.LoadDLL("kernel32.dll") + if err != nil { + return trace.Wrap(err) + } + p, err := d.FindProc("GenerateConsoleCtrlEvent") + if err != nil { + return trace.Wrap(err) + } + r, _, err := p.Call(syscall.CTRL_BREAK_EVENT, uintptr(pid)) if r == 0 { return trace.Wrap(err) } diff --git a/integration/autoupdate/tools/updater_test.go b/integration/autoupdate/tools/updater_test.go index 26824cf41d3ca..0655978263d68 100644 --- a/integration/autoupdate/tools/updater_test.go +++ b/integration/autoupdate/tools/updater_test.go @@ -24,6 +24,7 @@ import ( "bytes" "context" "fmt" + "io" "os" "os/exec" "regexp" @@ -186,14 +187,18 @@ func TestUpdateInterruptSignal(t *testing.T) { require.NoError(t, err) var output bytes.Buffer - cmd := exec.Command(tshPath, "version") - cmd.Stdout = &output - cmd.Stderr = &output + multiOut := io.MultiWriter(&output, os.Stdout) + cmd := newCommand(tshPath, "version") + cmd.Stdout = multiOut + cmd.Stderr = multiOut cmd.Env = append( os.Environ(), fmt.Sprintf("%s=%s", teleportToolsVersion, testVersions[1]), ) err = cmd.Start() + if err != nil { + t.Log(output.String()) + } require.NoError(t, err, "failed to start updater") pid := cmd.Process.Pid @@ -216,7 +221,10 @@ func TestUpdateInterruptSignal(t *testing.T) { require.Fail(t, "failed to wait till the download is started") case <-lock: time.Sleep(100 * time.Millisecond) - require.NoError(t, sendInterrupt(pid)) + t.Logf("sending signal to updater, pid: %d, test pid: %d", pid, os.Getpid()) + err := sendInterrupt(pid) + require.NoError(t, err, "failed to send signal to updater") + time.Sleep(100 * time.Millisecond) lock <- struct{}{} } @@ -229,6 +237,10 @@ func TestUpdateInterruptSignal(t *testing.T) { require.NoError(t, err) } assert.Contains(t, output.String(), "Update progress:") + + matches := pattern.FindStringSubmatch(output.String()) + require.Len(t, matches, 2) + require.Equal(t, testVersions[0], matches[1]) } // TestUpdateForOSSBuild verifies the update logic for AGPL editions of Teleport requires @@ -258,6 +270,9 @@ func TestUpdateForOSSBuild(t *testing.T) { fmt.Sprintf("%s=%s", teleportToolsVersion, testVersions[1]), ) out, err := cmd.Output() + if err != nil { + t.Log(string(out)) + } require.NoError(t, err) matches := pattern.FindStringSubmatch(string(out)) From 26c42b9b3636a580605d0228f3ccd72e64c4d857 Mon Sep 17 00:00:00 2001 From: Vadym Popov Date: Tue, 22 Jul 2025 22:13:41 -0700 Subject: [PATCH 4/4] Revert the disabling of the -race flag for MU tests, as recompilation takes more time than running the tests --- Makefile | 4 ---- integration/autoupdate/tools/main_test.go | 2 -- integration/autoupdate/tools/updater_test.go | 2 -- integration/autoupdate/tools/updater_tsh_test.go | 2 +- 4 files changed, 1 insertion(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 0c9b03e05349e..52a3a04ccbce0 100644 --- a/Makefile +++ b/Makefile @@ -1131,15 +1131,11 @@ run-etcd: .PHONY: integration integration: FLAGS ?= -v -race integration: PACKAGES = $(shell go list ./... | grep 'integration\([^s]\|$$\)' | grep -v integrations/lib/testing/integration ) -integration: PACKAGES_NO_RACE = $(shell go list ./integration/autoupdate/tools/... | grep 'integration\([^s]\|$$\)' | grep -v integrations/lib/testing/integration ) integration: $(TEST_LOG_DIR) ensure-gotestsum @echo KUBECONFIG is: $(KUBECONFIG), TEST_KUBE: $(TEST_KUBE) $(CGOFLAG) go test -timeout 30m -json -tags "$(PAM_TAG) $(FIPS_TAG) $(BPF_TAG)" $(PACKAGES) $(FLAGS) \ | tee $(TEST_LOG_DIR)/integration.json \ | gotestsum --raw-command --format=testname -- cat - $(CGOFLAG) go test -timeout 30m -json -tags "$(PAM_TAG) $(FIPS_TAG) $(BPF_TAG)" $(PACKAGES_NO_RACE) $(filter-out -race,$(FLAGS)) \ - | tee $(TEST_LOG_DIR)/integration.json \ - | gotestsum --raw-command --format=testname -- cat # # Integration tests that run Kubernetes tests in order to complete successfully diff --git a/integration/autoupdate/tools/main_test.go b/integration/autoupdate/tools/main_test.go index 43defe7facd59..ec69eeeb90082 100644 --- a/integration/autoupdate/tools/main_test.go +++ b/integration/autoupdate/tools/main_test.go @@ -1,5 +1,3 @@ -//go:build !race - /* * Teleport * Copyright (C) 2024 Gravitational, Inc. diff --git a/integration/autoupdate/tools/updater_test.go b/integration/autoupdate/tools/updater_test.go index 0655978263d68..0a356469c683b 100644 --- a/integration/autoupdate/tools/updater_test.go +++ b/integration/autoupdate/tools/updater_test.go @@ -1,5 +1,3 @@ -//go:build !race - /* * Teleport * Copyright (C) 2024 Gravitational, Inc. diff --git a/integration/autoupdate/tools/updater_tsh_test.go b/integration/autoupdate/tools/updater_tsh_test.go index b3d19071e9510..6e56cf168dc7a 100644 --- a/integration/autoupdate/tools/updater_tsh_test.go +++ b/integration/autoupdate/tools/updater_tsh_test.go @@ -1,4 +1,4 @@ -//go:build !windows && !race +//go:build !windows /* * Teleport