diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ca9f3c685d..9634d11383 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -510,6 +510,18 @@ jobs: with: go-version: ${{ env.GO_VERSION }} + - name: Set version info + shell: pwsh + run: | + # ignore errors since they won't affect build + try { + ./scripts/Set-VersionInfo.ps1 + } catch { + Write-Output "::warning::Could not set hcsshim version info: $_" + } finally { + $LASTEXITCODE = 0 + } + - run: go build ./cmd/containerd-shim-runhcs-v1 - run: go build ./cmd/runhcs - run: go build ./cmd/tar2ext4 @@ -558,6 +570,18 @@ jobs: with: go-version: ${{ env.GO_VERSION }} + - name: Set version info + shell: pwsh + run: | + # ignore errors since they won't affect build + try { + ./scripts/Set-VersionInfo.ps1 + } catch { + Write-Output "::warning::Could not set hcsshim version info: $_" + } finally { + $LASTEXITCODE = 0 + } + - name: Test run: make test diff --git a/cmd/containerd-shim-runhcs-v1/main.go b/cmd/containerd-shim-runhcs-v1/main.go index 69304881bf..be5e950f16 100644 --- a/cmd/containerd-shim-runhcs-v1/main.go +++ b/cmd/containerd-shim-runhcs-v1/main.go @@ -21,6 +21,7 @@ import ( "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" "github.com/Microsoft/hcsshim/internal/shimdiag" + hcsversion "github.com/Microsoft/hcsshim/internal/version" // register common types spec with typeurl _ "github.com/containerd/containerd/runtime" @@ -32,13 +33,15 @@ const ttrpcAddressEnv = "TTRPC_ADDRESS" // Add a manifest to get proper Windows version detection. //go:generate go run github.com/josephspurrier/goversioninfo/cmd/goversioninfo -platform-specific -// version will be populated by the Makefile, read from -// VERSION file of the source code. -var version = "" - -// gitCommit will be the hash that the binary was built from -// and will be populated by the Makefile -var gitCommit = "" +// `-ldflags '-X ...'` only works if the variable is uninitialized or set to a constant value. +// keep empty and override with data from [internal/version] only if empty to allow +// workflows currently setting these values to work. +var ( + // version will be the repo version that the binary was built from + version = "" + // gitCommit will be the hash that the binary was built from + gitCommit = "" +) var ( namespaceFlag string @@ -81,11 +84,21 @@ func main() { } } + // fall back on embedded version info (if any), if variables above were not set + if version == "" { + version = hcsversion.Version + } + if gitCommit == "" { + gitCommit = hcsversion.Commit + } + _ = provider.WriteEvent( "ShimLaunched", nil, etw.WithFields( etw.StringArray("Args", os.Args), + etw.StringField("version", version), + etw.StringField("commit", gitCommit), ), ) diff --git a/cmd/gcs/main.go b/cmd/gcs/main.go index e0be1945ad..25751763dd 100644 --- a/cmd/gcs/main.go +++ b/cmd/gcs/main.go @@ -29,6 +29,7 @@ import ( "github.com/Microsoft/hcsshim/internal/guestpath" "github.com/Microsoft/hcsshim/internal/log" "github.com/Microsoft/hcsshim/internal/oc" + "github.com/Microsoft/hcsshim/internal/version" "github.com/Microsoft/hcsshim/pkg/securitypolicy" ) @@ -271,7 +272,11 @@ func main() { baseLogPath := guestpath.LCOWRootPrefixInUVM - logrus.Info("GCS started") + logrus.WithFields(logrus.Fields{ + "branch": version.Branch, + "commit": version.Commit, + "version": version.Version, + }).Info("GCS started") // Set the process core dump location. This will be global to all containers as it's a kernel configuration. // If no path is specified core dumps will just be placed in the working directory of wherever the process diff --git a/cmd/runhcs/main.go b/cmd/runhcs/main.go index 046119fb91..4ec95970bb 100644 --- a/cmd/runhcs/main.go +++ b/cmd/runhcs/main.go @@ -10,23 +10,27 @@ import ( "github.com/Microsoft/go-winio" "github.com/Microsoft/go-winio/pkg/etwlogrus" - "github.com/Microsoft/hcsshim/internal/regstate" - "github.com/Microsoft/hcsshim/internal/runhcs" "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" "github.com/urfave/cli" + + "github.com/Microsoft/hcsshim/internal/regstate" + "github.com/Microsoft/hcsshim/internal/runhcs" + hcsversion "github.com/Microsoft/hcsshim/internal/version" ) // Add a manifest to get proper Windows version detection. //go:generate go run github.com/josephspurrier/goversioninfo/cmd/goversioninfo -platform-specific -// version will be populated by the Makefile, read from -// VERSION file of the source code. -var version = "" - -// gitCommit will be the hash that the binary was built from -// and will be populated by the Makefile -var gitCommit = "" +// `-ldflags '-X ...'` only works if the variable is uninitialized or set to a constant value. +// keep empty and override with data from [internal/version] only if empty to allow +// workflows currently setting these values to work. +var ( + // version will be the repo version that the binary was built from + version = "" + // gitCommit will be the hash that the binary was built from + gitCommit = "" +) var stateKey *regstate.Key @@ -62,6 +66,14 @@ func main() { app.Name = "runhcs" app.Usage = usage + // fall back on embedded version info (if any), if variables above were not set + if version == "" { + version = hcsversion.Version + } + if gitCommit == "" { + gitCommit = hcsversion.Commit + } + var v []string if version != "" { v = append(v, version) diff --git a/internal/uvm/create_lcow.go b/internal/uvm/create_lcow.go index 9a80cafeeb..6b9d7cbbb9 100644 --- a/internal/uvm/create_lcow.go +++ b/internal/uvm/create_lcow.go @@ -508,7 +508,7 @@ Example JSON document produced once the hcsschema.ComputeSytem returned by makeL } */ -// Make the ComputeSystem document object that will be serialised to json to be presented to the HCS api. +// Make the ComputeSystem document object that will be serialized to json to be presented to the HCS api. func makeLCOWDoc(ctx context.Context, opts *OptionsLCOW, uvm *UtilityVM) (_ *hcsschema.ComputeSystem, err error) { logrus.Tracef("makeLCOWDoc %v\n", opts) diff --git a/internal/version/data/.gitignore b/internal/version/data/.gitignore new file mode 100644 index 0000000000..c79d14f2f2 --- /dev/null +++ b/internal/version/data/.gitignore @@ -0,0 +1,4 @@ +# DO NOT MOVE: this file prevents `//go:embed data/*` from failing at build time +BRANCH +COMMIT +VERSION diff --git a/internal/version/version.go b/internal/version/version.go new file mode 100644 index 0000000000..3883a51b84 --- /dev/null +++ b/internal/version/version.go @@ -0,0 +1,41 @@ +package version + +import ( + "embed" + "strings" +) + +// Using `//go:embed data/VERSION` (similarly for `data/COMMIT` and `data/BRANCH`) will +// fail at build time if the files don't exist, which will break existing build workflows. +// +// Alternatively, committing those files in git will cause problems as they will be constantly +// updated and overwitten if devs update them locally and commit the changes. +// +// Therefore, we embed a (non-empty) directory and look up the files at run-time so builds +// succeed regardless of whether the files are exist or not. +// `data/.gitignore` is our fallback file, which keeps `data/` non-empty and prevents [embed] +// from failing. + +// Using a dedicated `data` directory allows us to separate out what files to embed. +// (Writing `//go:embed *` would include everything in this directory, including this file.) + +// See scripts/Set-VersionInfo.ps1 for an example of setting these values via files in data/. + +//go:embed data/* +var data embed.FS + +var ( + // Branch is the git branch the binary was built from. + Branch = readDataFile("BRANCH") + + // Commit is the git commit the binary was built from. + Commit = readDataFile("COMMIT") + + // Version is the complete semver. + Version = readDataFile("VERSION") +) + +func readDataFile(f string) string { + b, _ := data.ReadFile("data/" + f) + return strings.TrimSpace(string(b)) +} diff --git a/scripts/Run-Tests.ps1 b/scripts/Run-Tests.ps1 index 1120afd139..2faafa172b 100644 --- a/scripts/Run-Tests.ps1 +++ b/scripts/Run-Tests.ps1 @@ -76,7 +76,4 @@ Invoke-TestCommand ` -TestCmd $testcmd ` -OutputFile $out ` -OutputCmd (& { if ( $Action -eq 'Bench' ) { $BenchstatPath } }) ` - -Preamble ` - -Date $Date ` - -Note $Note ` -Verbose:$Verbose diff --git a/scripts/Set-VersionInfo.ps1 b/scripts/Set-VersionInfo.ps1 new file mode 100644 index 0000000000..a61cf5d6ff --- /dev/null +++ b/scripts/Set-VersionInfo.ps1 @@ -0,0 +1,58 @@ +# Example of who to set the version/git information in ./internal/version so +# that the binaries will report the appropriate information. + +$ErrorActionPreference = 'Continue' +$VerbosePreference = 'Continue' + +$root = Split-Path -Path $PSScriptRoot -Parent +$dataDir = Join-Path $root 'internal/version/data' + +function run([string]$cmd, [string[]]$params) { + Write-Verbose "$cmd $params" + $s = & $cmd @params 2>&1 + if ( $LASTEXITCODE -ne 0 ) { + $err = "Command '$cmd $params' failed" + if ( -not [string]::IsNullOrWhiteSpace("$s") ) { + $err += ": $s" + } + Write-Warning $err + } else { + $s + } +} + +function writeto([string]$f, [string]$v) { + if ( [string]::IsNullOrWhiteSpace($v) ) { + return + } + $v = $v.Trim() + + Write-Output "${f}: $v" + $v | Out-File -Encoding ascii -FilePath (Join-Path $dataDir $f) -NoNewline +} + +$branch = run 'git' @('rev-parse', '--abbrev-ref', 'HEAD') +if ( $branch -ceq 'HEAD') { + Write-Warning 'Detected detached head' + $branch = run 'git' @('name-rev', '--name-only', 'HEAD') + if ( [string]::IsNullOrWhiteSpace($branch) -or $branch -eq 'undefined' ) { + Write-Warning "'git name-rev' failed: $branch" + $branch = run 'git' @('branch', '-r', '--contains=HEAD') | Select-Object -First 1 + } +} +$branch = $branch -replace '^(origin|remotes/origin|refs/heads)/', '' +writeto 'BRANCH' $branch + +$commit = run 'git' @('rev-parse', 'HEAD') +try { + run 'git' @('diff', '--no-ext-diff', '--quiet', '--exit-code', 'HEAD') + if ( $LASTEXITCODE -ne 0 ) { + Write-Warning 'Dirty repo' + $commit += '-dirty' + } +} catch { } +writeto 'COMMIT' $commit + +# don't use `--always`; we we're okay with an empty version since commit will likely be non-empty +$version = run 'git' @('describe', '--match=v[0-9]*', '--tags', '--dirty') +writeto 'VERSION' $version diff --git a/scripts/Test-LCOW-UVM.ps1 b/scripts/Test-LCOW-UVM.ps1 index 26dfba040d..30f4595adc 100644 --- a/scripts/Test-LCOW-UVM.ps1 +++ b/scripts/Test-LCOW-UVM.ps1 @@ -92,14 +92,7 @@ if ( $shell ) { $gcspath = "$GCSTestMount/gcs.test" } - $pre = "wait-paths -p $waitfiles -t 5 ; " + ` - 'echo nproc: `$(nproc) ; ' + ` - 'echo kernel: `$(uname -a) ; ' + ` - 'echo gcs.commit: `$(cat /info/gcs.commit 2>/dev/null) ; ' + ` - 'echo gcs.branch: `$(cat /info/gcs.branch 2>/dev/null) ; ' + ` - 'echo tar.date: `$(cat /info/tar.date 2>/dev/null) ; ' + ` - 'echo image.name: `$(cat /info/image.name 2>/dev/null) ; ' + ` - 'echo build.date: `$(cat /info/build.date 2>/dev/null) ; ' + $pre = "wait-paths -p $waitfiles -t 5 ; " $testcmd, $out = New-TestCommand ` -Action $Action ` @@ -144,10 +137,6 @@ $boot += " -exec `"$cmd`" " Invoke-TestCommand ` -TestCmd $boot ` - -TestCmdPreamble $testcmd ` -OutputFile (& { if ( $Action -ne 'Shell' ) { $out } }) ` -OutputCmd (& { if ( $Action -eq 'Bench' ) { 'benchstat' } }) ` - -Preamble ` - -Date $Date ` - -Note $Note ` -Verbose:$Verbose diff --git a/scripts/Testing.psm1 b/scripts/Testing.psm1 index f993817fec..8526f8f96c 100644 --- a/scripts/Testing.psm1 +++ b/scripts/Testing.psm1 @@ -106,23 +106,11 @@ function Invoke-TestCommand { [string] $TestCmd, - [string] - $TestCmdPreamble = $TestCmd, - [string] $OutputFile = '', [string] - $OutputCmd, - - [switch] - $Preamble, - - [DateTime] - $Date = (Get-Date), - - [string] - $Note + $OutputCmd ) Write-Verbose "Running command: $TestCmd" @@ -132,19 +120,6 @@ function Invoke-TestCommand { Write-Verbose "Saving output to: $OutputFile" } - - if ( $Preamble ) { - & { - Write-Output "test.date: $(Get-Date -Date $Date -UFormat '%FT%R%Z' -AsUTC)" - if ( $Note ) { - Write-Output "note: $Note" - } - Write-Output "test.command: $TestCmdPreamble" - if ( Get-Command -ErrorAction Ignore 'git' ) { - Write-Output "pkg.commit: $(git rev-parse HEAD 2>$null)" - } - } | Tee-Object -Encoding utf8 -FilePath $OutputFile - } Invoke-Expression $TestCmd | Tee-Object -Encoding utf8 -Append -FilePath $OutputFile diff --git a/test/functional/main_test.go b/test/functional/main_test.go index 30c69a4492..d89878fa98 100644 --- a/test/functional/main_test.go +++ b/test/functional/main_test.go @@ -142,6 +142,11 @@ func TestMain(m *testing.M) { l.TempPath = *flagLayerTempDir } + // print additional configuration options when running benchmarks, so we can better track performance. + if util.RunningBenchmarks() { + util.PrintAdditionalBenchmarkConfig() + } + e := m.Run() // close any uVMs that escaped diff --git a/test/gcs/main_test.go b/test/gcs/main_test.go index 495ef7e05d..80086923bf 100644 --- a/test/gcs/main_test.go +++ b/test/gcs/main_test.go @@ -8,11 +8,14 @@ import ( "fmt" "log" "os" + "path/filepath" + "strings" "testing" cgroups "github.com/containerd/cgroups/v3/cgroup1" "github.com/sirupsen/logrus" "go.opencensus.io/trace" + "golang.org/x/sys/unix" "github.com/Microsoft/hcsshim/internal/guest/runtime" "github.com/Microsoft/hcsshim/internal/guest/runtime/hcsv2" @@ -23,6 +26,7 @@ import ( "github.com/Microsoft/hcsshim/internal/protocol/guestresource" "github.com/Microsoft/hcsshim/pkg/securitypolicy" + "github.com/Microsoft/hcsshim/test/internal/util" testflag "github.com/Microsoft/hcsshim/test/pkg/flag" "github.com/Microsoft/hcsshim/test/pkg/require" ) @@ -73,6 +77,43 @@ func TestMain(m *testing.M) { logrus.WithError(err).Fatal("could not set up testing") } + // print additional configuration options when running benchmarks, so we can better track performance. + if util.RunningBenchmarks() { + util.PrintAdditionalBenchmarkConfig() + + // print uname info + uname := &unix.Utsname{} + if err := unix.Uname(uname); err == nil { + for n, bs := range map[string][]byte{ + "kernel-name": uname.Sysname[:], + "node-name": uname.Nodename[:], + "kernel-release": uname.Release[:], + "kernel-version": uname.Version[:], + "machine-name": uname.Machine[:], + "domain-name": uname.Domainname[:], + } { + if s := unix.ByteSliceToString(bs); s != "" && s != "(none)" { + fmt.Println(n + ": " + s) + } + } + } + + // print additional (rootfs) build info written by Makefile to /info/ + for _, f := range []string{ + "tar.date", + "image.name", + "build.date", + } { + if b, err := os.ReadFile(filepath.Join("/info", f)); err == nil { + if s := strings.TrimSpace(string(b)); s != "" { + // convention for benchmark config is kebab-case + fmt.Println("rootfs-" + strings.ReplaceAll(f, ".", "-") + ": " + s) + } + } + + } + } + os.Exit(m.Run()) } diff --git a/test/internal/layers/scratch.go b/test/internal/layers/scratch.go index 5e7489988f..eaaec8b731 100644 --- a/test/internal/layers/scratch.go +++ b/test/internal/layers/scratch.go @@ -21,10 +21,8 @@ const ( func CacheFile(_ context.Context, tb testing.TB, dir string) string { tb.Helper() if dir == "" { + // testing infra will remove the temp dir during cleanup dir = tb.TempDir() - tb.Cleanup(func() { - os.RemoveAll(dir) - }) } cache := filepath.Join(dir, CacheFileName) return cache diff --git a/test/internal/util/util.go b/test/internal/util/util.go index 5f3d15e386..1aaed3b6c9 100644 --- a/test/internal/util/util.go +++ b/test/internal/util/util.go @@ -1,8 +1,16 @@ package util import ( + "flag" + "fmt" + "os" + "runtime" + "strconv" "strings" + "time" "unicode" + + "github.com/Microsoft/hcsshim/internal/version" ) // CleanName returns a string appropriate for uVM, container, or file names. @@ -18,3 +26,76 @@ func CleanName(n string) string { } return strings.TrimSpace(strings.Map(mapper, n)) } + +// `go test` (and `go test -c`) runs in a binary with a suite of `-test.*` flags defined. +// use that as our cue that we are in a testing binary. + +// RunningBenchmarks returns whether benchmarks were requested to run from within a test suite. +// Returning true +// InTest returns whether the executable is currently being run from a test suite. +// +// Should not be called from init() or global variable initialization since there is no guarantee on whether +// testing flags have been defined, and ideally should be called after [flag.Parse]. +func RunningBenchmarks() (b bool) { + // TODO (go1.21): use [testing.Testing] (ref: https://github.com/golang/go/issues/52600) to shortcircut + + // go doesn't run benchmarks unless -test.bench flag is passed, so thats our cue. + // ref: https://pkg.go.dev/testing#hdr-Benchmarks + // + // (even if benchmarks are run via `go test -bench='.' `, go will still compile a testing binary + // and pass the flags as `-test.*`) + f := flag.Lookup("test.bench") + return f != nil && f.Value.String() != "" +} + +// PrintAdditionalBenchmarkConfig outputs additional configuration settings, such as: +// - start time +// - number of CPUs available +// - go version +// - version information from [github.com/Microsoft/hcsshim/internal/version] (if set) +// +// Benchmark configuration format: +// https://go.googlesource.com/proposal/+/master/design/14313-benchmark-format.md#configuration-lines +// +// For default configuration printed, see: [testing.(*B).Run()] in src/testing/benchmark.go +func PrintAdditionalBenchmarkConfig() { + // test binaries are not built with module information, so [debug.ReadBuildInfo] will only give + // the go version (which it ultimately gets via [runtime.Version]) and not provide the "vcs.revision" setting. + + for k, v := range map[string]string{ + // use dedicated os/arch fields (in addition to `OOS & GOARCH) to make this config header non-Go specific + // and standardize the architecture field. + "os": runtime.GOOS, + "arch": translateGOARCH(runtime.GOARCH), + + "goversion": runtime.Version(), + "num-cpu": strconv.Itoa(runtime.NumCPU()), + "start-time": time.Now().UTC().Format(time.RFC3339), // ISO 8601 + "command": strings.Join(os.Args, " "), + + "branch": version.Branch, + "commit": version.Commit, + "version": version.Version, + } { + if k != "" && v != "" { + fmt.Printf("%s: %s\n", k, v) + } + } +} + +// translate GOARCH to more "official" values. +// +// mostly for weirdness with how go reports x86 architectures. +// see: +// - https://en.wikipedia.org/wiki/X86 +// - https://en.wikipedia.org/wiki/X86-64 +func translateGOARCH(s string) string { + // from src/go/build/syslist.go in go repo + switch s { + case "386": + return "x86" + case "amd64": + return "x86_64" + } + return s +}