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

chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209

Closed
wants to merge 52 commits into from
Closed
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
b8d8f85
feat: add a mage command to list all system tests
mdelapenya Jun 7, 2021
d8fe22c
feat: add packageSystemTests commands for beat running system tests
mdelapenya Jun 7, 2021
ccead60
feat: support compressing integration test files
mdelapenya Jun 7, 2021
dd212e2
chore: evaluate if it's a symlink the soonest
mdelapenya Jun 8, 2021
81d44cd
chore(ci): run integration tests for metricbeat in a consistent manner
mdelapenya Jun 8, 2021
dddf17c
chore(ci): reorder xpack/metricbeat's pipeline to match oss'
mdelapenya Jun 8, 2021
5fe3f32
chore(ci): run filebeat's integration tests in separate stages
mdelapenya Jun 8, 2021
5494848
chore(ci): upload to GCP bucket system-tests tar files
mdelapenya Jun 8, 2021
aa87f5f
chore(ci): invoke new mage command before archiving files
mdelapenya Jun 8, 2021
d1dd718
chore(ci): remove unused script
mdelapenya Jun 8, 2021
57b6efb
fix(ci): remove non-needed field in CI descriptor
mdelapenya Jun 8, 2021
d5d9f9d
chore: fix typo
mdelapenya Jun 8, 2021
c67f530
chore: use ioutil for writing the tar file
mdelapenya Jun 9, 2021
229c69d
chore: defer closing the file
mdelapenya Jun 9, 2021
945983d
chore: move goal to its own package
mdelapenya Jun 9, 2021
26776fa
fix: remove unreachable code after UI merge
mdelapenya Jun 9, 2021
fcc32be
fix: add packageSystemTests to heartbeat build
mdelapenya Jun 9, 2021
58e084d
chore: separate unit from integration tests in heartbeat
mdelapenya Jun 9, 2021
b855019
fix: do not fail the process if there are no system tests
mdelapenya Jun 9, 2021
8b3a8c9
chore: log tar creation
mdelapenya Jun 9, 2021
5d6459a
chore: include new command in the elastic-agent
mdelapenya Jun 10, 2021
e5265a6
chore: move command to integtest package
mdelapenya Jun 10, 2021
30ccbed
Merge branch 'master' into archive-system-tests
mdelapenya Jun 14, 2021
ead2f67
Merge branch 'master' into archive-system-tests
mdelapenya Jun 21, 2021
b6dbec3
Merge branch 'master' into archive-system-tests
mdelapenya Jun 28, 2021
d4a87cb
Merge branch 'master' into archive-system-tests
mdelapenya Jun 29, 2021
218ee9a
Merge branch 'master' into archive-system-tests
mdelapenya Jun 30, 2021
70090f6
chore: apply same test layout in both heartbeat projects
mdelapenya Jun 30, 2021
d7b3d6f
chore: separate unit from goInteg tests in dockerlogbeat
mdelapenya Jun 30, 2021
244317c
chore: consistently call unit tests in auditbeat
mdelapenya Jun 30, 2021
0e3c931
chore: consistently run unit and goIntegTests in osquerybeat
mdelapenya Jun 30, 2021
2b5b183
fix: do not run "pythonIntegTest" for x-pack/heartbeat
mdelapenya Jun 30, 2021
86ad8ba
chore: consistently run unit and goInteg tests for functionbeat
mdelapenya Jun 30, 2021
b1e8aaa
chore: consistently run unit and goInteg tests for the elastic-agent
mdelapenya Jun 30, 2021
9d86f71
chore: consistently run unit tests in packetbeat
mdelapenya Jun 30, 2021
f914d57
chore: consistently run unit/integration tests for libbeat
mdelapenya Jun 30, 2021
729a432
chore: consistently run unit and goInteg tests for libbeat
mdelapenya Jun 30, 2021
8678dfc
chore: consistently run unit tests for x-pack/packetbeat
mdelapenya Jun 30, 2021
368417a
chore: update comment
mdelapenya Jun 30, 2021
0a17e48
chore: move new command to common package
mdelapenya Jun 30, 2021
d99d0b0
fix: create a plain directory layout for tar files
mdelapenya Jul 1, 2021
2edf800
Merge branch 'master' into archive-system-tests
mdelapenya Jul 1, 2021
f7b150a
chore: simplify creating path
mdelapenya Jul 1, 2021
2ed629b
chore: remove missing import comment
mdelapenya Jul 1, 2021
7aa8e53
chore: add debug trace for files to upload
mdelapenya Jul 1, 2021
78d203f
Merge branch 'master' into archive-system-tests
mdelapenya Jul 19, 2021
6c39510
Revert "chore: add debug trace for files to upload"
mdelapenya Jul 20, 2021
cf33568
fix: bring parameter back after resolving conflicts
mdelapenya Jul 20, 2021
05ab4ab
Merge branch 'master' into archive-system-tests
mdelapenya Jul 20, 2021
7bf7938
Revert "Revert "chore: add debug trace for files to upload""
mdelapenya Jul 21, 2021
83996c1
chore: use absolute paths
mdelapenya Jul 21, 2021
a230581
chore: remove withModule from beats without modules
mdelapenya Jul 27, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions .ci/scripts/search_system_tests.py

This file was deleted.

24 changes: 16 additions & 8 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ def withBeatsEnv(Map args = [:], Closure body) {
error("Error '${err.toString()}'")
} finally {
if (archive) {
archiveTestOutput(testResults: testResults, artifacts: artifacts, id: args.id, upload: upload)
archiveTestOutput(directory: directory, testResults: testResults, artifacts: artifacts, id: args.id, upload: upload)
}
tearDown()
}
Expand Down Expand Up @@ -752,6 +752,8 @@ def getCommonModuleInTheChangeSet(String directory) {
* to bypass some issues when working with big repositories.
*/
def archiveTestOutput(Map args = [:]) {
def directory = args.get('directory', '')

catchError(buildResult: 'SUCCESS', stageResult: 'UNSTABLE') {
if (isUnix()) {
fixPermissions("${WORKSPACE}")
Expand All @@ -775,13 +777,19 @@ def archiveTestOutput(Map args = [:]) {
}
if (args.upload) {
catchError(buildResult: 'SUCCESS', message: 'Failed to archive the build test results', stageResult: 'SUCCESS') {
def folder = cmd(label: 'Find system-tests', returnStdout: true, script: 'python .ci/scripts/search_system_tests.py').trim()
log(level: 'INFO', text: "system-tests='${folder}'. If no empty then let's create a tarball")
if (folder.trim()) {
// TODO: nodeOS() should support ARM
def os_suffix = isArm() ? 'linux' : nodeOS()
def name = folder.replaceAll('/', '-').replaceAll('\\\\', '-').replaceAll('build', '').replaceAll('^-', '') + '-' + os_suffix
tarAndUploadArtifacts(file: "${name}.tgz", location: folder)
withMageEnv(version: "${env.GO_VERSION}"){
Copy link
Member

Choose a reason for hiding this comment

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

Can it be done this change in a different PR? Then, we can remove the scripts that configure the tools too?

dir(directory){
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if this is '' as it would be if the .get() returned its default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are setting the variable with setEnvVar('GO_VERSION', readFile(".go-version").trim()). Do you mean that the readFile can return empty value? That would be the case if the .go-version file disappears, which will represent a breaking change in how Go is installed.

cmd(label: "Archive system tests files", script: 'mage packageSystemTests')
Copy link
Member

Choose a reason for hiding this comment

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

If packageSystemTests is only called here, are all the other changes required? I am a bit concerned that we may be missing the execution of some tests in some cases, it may be good to separate these changes to a different PR if possible, so we can merge this feature with less risks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let me split the PR in two:

  1. changing test execution layout on CI
  2. adding the new mage command

}
}
def fileName = 'build/system-tests-*.tar.gz' // see dev-tools/mage/pkg.go#PackageSystemTests method
dir("${BASE_DIR}"){
googleStorageUploadExt(
bucket: "gs://${JOB_GCS_BUCKET}/${env.JOB_NAME}-${env.BUILD_ID}",
credentialsId: "${JOB_GCS_EXT_CREDENTIALS}",
pattern: "${fileName}",
sharedPublicly: true
)
}
}
}
Expand Down
65 changes: 65 additions & 0 deletions dev-tools/mage/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,71 @@ func unzip(sourceFile, destinationDir string) error {
return nil
}

// Tar compress a directory using tar + gzip algorithms
jsoriano marked this conversation as resolved.
Show resolved Hide resolved
func Tar(src string, targetFile string) error {
fmt.Printf(">> creating TAR file from directory: %s, target: %s\n", src, targetFile)
var buf bytes.Buffer

// tar > gzip > buf
zr := gzip.NewWriter(&buf)
tw := tar.NewWriter(zr)

// walk through every file in the folder
filepath.Walk(src, func(file string, fi os.FileInfo, errFn error) error {
// if a symlink, skip file
if fi.Mode().Type() == os.ModeSymlink {
fmt.Printf(">> skipping symlink: %s\n", file)
return nil
}

// generate tar header
header, err := tar.FileInfoHeader(fi, file)
if err != nil {
fmt.Printf(">> error getting file info header: %s\n", err)
return err
}

// must provide real name
// (see https://golang.org/src/archive/tar/common.go?#L626)
header.Name = filepath.ToSlash(file)

// write header
if err := tw.WriteHeader(header); err != nil {
fmt.Printf(">> error writing header: %s\n", err)
return err
}

// if not a dir, write file content
if !fi.IsDir() {
data, err := os.Open(file)
if err != nil {
fmt.Printf(">> error opening file: %s\n", err)
return err
}
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
defer data.Close()
if _, err := io.Copy(tw, data); err != nil {
fmt.Printf(">> error compressing file: %s\n", err)
return err
}
}
return nil
})

// produce tar
if err := tw.Close(); err != nil {
fmt.Printf(">> error closing tar file: %s\n", err)
return err
}
// produce gzip
if err := zr.Close(); err != nil {
fmt.Printf(">> error closing gzip file: %s\n", err)
return err
}

// write the .tar.gzip
return ioutil.WriteFile(targetFile, buf.Bytes(), os.FileMode(0600))
}

func untar(sourceFile, destinationDir string) error {
file, err := os.Open(sourceFile)
if err != nil {
Expand Down
55 changes: 55 additions & 0 deletions dev-tools/mage/target/systemtest/python.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Licensed to Elasticsearch B.V. under one or more contributor
// license agreements. See the NOTICE file distributed with
// this work for additional information regarding copyright
// ownership. Elasticsearch B.V. licenses this file to you under
// the Apache License, Version 2.0 (the "License"); you may
// not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package systemtest

import (
"fmt"
"os"
"path/filepath"
"strings"

devtools "github.com/elastic/beats/v7/dev-tools/mage"
)

// PackageSystemTests packages the python system tests results
func PackageSystemTests() error {
excludeds := []string{".ci", ".git", ".github", "vendor", "dev-tools"}

// include run as it's the directory we want to compress
systemTestsDir := fmt.Sprintf("build%[1]csystem-tests%[1]crun", os.PathSeparator)
files, err := devtools.FindFilesRecursive(func(path string, _ os.FileInfo) bool {
base := filepath.Base(path)
for _, excluded := range excludeds {
if strings.HasPrefix(base, excluded) {
return false
}
}

return strings.HasPrefix(path, systemTestsDir)
})
if err != nil {
return err
}

if len(files) == 0 {
fmt.Printf(">> there are no system test files under %s", systemTestsDir)
return nil
}

return devtools.Tar(systemTestsDir, devtools.MustExpand("{{ elastic_beats_dir }}/build/system-tests-"+devtools.MustExpand("{{ repo.SubDir }}")+".tar.gz"))
}
13 changes: 10 additions & 3 deletions filebeat/Jenkinsfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,16 @@ stages:
branches: true ## for all the branches
tags: true ## for all the tags
stage: extended
build:
mage: "mage build test"
withModule: true ## run the ITs only if the changeset affects a specific module.
unitTest:
mage: "mage build unitTest"
stage: mandatory
goIntegTest:
mage: "mage goIntegTest"
withModule: true
stage: mandatory
pythonIntegTest:
mage: "mage pythonIntegTest" ## run the ITs only if the changeset affects a specific module.
withModule: true
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
stage: mandatory
macos:
mage: "mage build unitTest"
Expand Down
2 changes: 2 additions & 0 deletions filebeat/magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
// mage:import generate
_ "github.com/elastic/beats/v7/filebeat/scripts/mage/generate"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/systemtest"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/unittest"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/test"
Expand Down
12 changes: 10 additions & 2 deletions heartbeat/Jenkinsfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,16 @@ stages:
branches: true ## for all the branches
tags: true ## for all the tags
stage: extended
build:
mage: "mage build test"
unitTest:
mage: "mage build unitTest"
stage: mandatory
goIntegTest:
mage: "mage goIntegTest"
withModule: true
stage: mandatory
pythonIntegTest:
mage: "mage pythonIntegTest"
withModule: true
Comment on lines +43 to +47
Copy link
Member

Choose a reason for hiding this comment

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

Same here, not sure if we can use withModule. Heartbeat doesn't have modules, it has monitors, but they have a different directory layout.

stage: mandatory
macos:
mage: "mage build unitTest"
Expand Down
2 changes: 2 additions & 0 deletions heartbeat/magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ import (
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/build"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/systemtest"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/unittest"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/integtest"
Expand Down
8 changes: 4 additions & 4 deletions metricbeat/Jenkinsfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ stages:
platforms: ## override default labels in this specific stage.
- "windows-10"
stage: extended
windows-8:
windows-2008:
mage: "mage build unitTest"
platforms: ## override default labels in this specific stage.
- "windows-8"
- "windows-2008-r2"
stage: extended
windows-2008:
windows-8:
mage: "mage build unitTest"
platforms: ## override default labels in this specific stage.
- "windows-2008-r2"
- "windows-8"
stage: extended
windows-7:
mage: "mage build unitTest"
Expand Down
2 changes: 2 additions & 0 deletions metricbeat/magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ import (
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/pkg"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/systemtest"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/test"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/unittest"
Expand Down
13 changes: 10 additions & 3 deletions x-pack/filebeat/Jenkinsfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,16 @@ stages:
branches: true ## for all the branches
tags: true ## for all the tags
stage: extended
build:
mage: "mage build test"
withModule: true ## run the ITs only if the changeset affects a specific module.
unitTest:
mage: "mage build unitTest"
stage: mandatory
goIntegTest:
mage: "mage goIntegTest"
withModule: true
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
stage: mandatory
pythonIntegTest:
mage: "mage pythonIntegTest" ## run the ITs only if the changeset affects a specific module.
withModule: true
mdelapenya marked this conversation as resolved.
Show resolved Hide resolved
stage: mandatory
macos:
mage: "mage build unitTest"
Expand Down
2 changes: 2 additions & 0 deletions x-pack/filebeat/magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/compose"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/systemtest"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/unittest"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/test"
Expand Down
8 changes: 8 additions & 0 deletions x-pack/metricbeat/Jenkinsfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ stages:
unitTest:
mage: "mage build unitTest"
stage: mandatory
goIntegTest:
mage: "mage goIntegTest"
withModule: true
stage: mandatory
pythonIntegTest:
mage: "mage pythonIntegTest"
withModule: true
stage: mandatory
cloud:
cloud: "mage build test"
withModule: true ## run the ITs only if the changeset affects a specific module.
Expand Down
2 changes: 2 additions & 0 deletions x-pack/metricbeat/magefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import (
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/compose"
// mage:import
_ "github.com/elastic/beats/v7/dev-tools/mage/target/systemtest"
// mage:import
"github.com/elastic/beats/v7/dev-tools/mage/target/test"
// mage:import
_ "github.com/elastic/beats/v7/metricbeat/scripts/mage/target/metricset"
Expand Down