Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing: update Prometheus tests to go 1.21.4 #1511

Merged
merged 10 commits into from
Nov 27, 2023

Conversation

martijnvans
Copy link
Contributor

@martijnvans martijnvans commented Nov 13, 2023

Description

Also provide better instructions for updating it using mirror_content.sh, which puts the tarball into a different bucket.

Also includes a few other mini fixes to installGolang, such as RunScriptRemotely -> RunRemotely and some comment tweaks.

This is a follow up to #1509.

Related issue

b/310621875

How has this been tested?

Checklist:

  • Unit tests
    • Unit tests do not apply.
    • Unit tests have been added/modified and passed for this PR.
  • Integration tests
    • Integration tests do not apply.
    • Integration tests have been added/modified and passed for this PR.
  • Documentation
    • This PR introduces no user visible changes.
    • This PR introduces user visible changes and the corresponding documentation change has been made.
  • Minor version bump
    • This PR introduces no new features.
    • This PR introduces new features, and there is a separate PR to bump the minor version since the last release already.
    • This PR bumps the version.

installCmd = fmt.Sprintf(`
cd (New-TemporaryFile | %% { Remove-Item $_; New-Item -ItemType Directory -Path $_ })
Invoke-WebRequest "https://go.dev/dl/go%s.windows-%s.msi" -OutFile golang.msi
Start-Process msiexec.exe -ArgumentList "/i","golang.msi","/quiet" -Wait `, goVersion, goArch)
} else {
installCmd = fmt.Sprintf(`
set -e
set -o pipefail
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I switched this to use RunRemotely because I personally prefer RunRemotely unless you need the extra features of RunScriptRemotely like command line arguments. RunRemotely is also simpler and produces better messages in the logs.

set -e is unnecessary because the command (excluding "set -o pipefail") is only 1 line long. (this applies equally to RunScriptRemotely and RunRemotely). We don't include set -e for other RunRemotely commands so I removed it here.

set -o pipefail is there so that a nonzero exit code from gsutil cp produces an error instead of being ignored. It's a best practice when piping commands together and you need them all to succeed.

@martijnvans martijnvans marked this pull request as ready for review November 14, 2023 16:45
@martijnvans martijnvans requested review from rafaelwestphal and removed request for rafaelwestphal November 14, 2023 16:51
Copy link
Contributor

@LujieDuan LujieDuan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@martijnvans martijnvans merged commit 64752bd into master Nov 27, 2023
69 checks passed
@martijnvans martijnvans deleted the martijnvans-update-go-1-21-with-fixes branch November 27, 2023 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants