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

Add CI testing to x-pack/auditbeat #9362

Conversation

andrewkroh
Copy link
Member

Update auditbeat and x-pack/auditbeat to share logic for generating config and packages. This makes auditbeat and x-pack/auditbeat have independent package targets where auditbeat generates only OSS packages and x-pack/auditbeat generates Elastic licensed packages.

And x-pack/auditbeat will now be tested on Travis CI.

dev-tools/mage/pkgspecs.go Show resolved Hide resolved
dev-tools/mage/pkgspecs.go Show resolved Hide resolved
dev-tools/mage/kibana.go Show resolved Hide resolved
auditbeat/scripts/mage/package.go Outdated Show resolved Hide resolved
auditbeat/scripts/mage/package.go Outdated Show resolved Hide resolved
auditbeat/scripts/mage/docs.go Show resolved Hide resolved
auditbeat/scripts/mage/config.go Outdated Show resolved Hide resolved
auditbeat/scripts/mage/config.go Show resolved Hide resolved
@andrewkroh
Copy link
Member Author

andrewkroh commented Dec 3, 2018

@cwurm Some tests are failing in x-pack/auditbeat.

And there's an unrelated failure in metricbeat on jenkins.

@andrewkroh andrewkroh force-pushed the feature-auditbeat-host-build branch from 84ce662 to 2a12cb7 Compare December 3, 2018 17:51
@andrewkroh
Copy link
Member Author

andrewkroh commented Dec 3, 2018

I skipped one test that was failing.

After this merges I'll open a separate PR to add an OSX worker to the Travis CI matrix. This requires some additional infrastructure changes to be able to disable integration tests executed using Docker.

And once we merge the feature branch to master we can open a PR to add Jenkins testing of x-pack/auditbeat.

@andrewkroh andrewkroh requested a review from cwurm December 3, 2018 22:22
@andrewkroh
Copy link
Member Author

Because Jenkins failed on auditbeat/macos I triggered another build of only macos. It passed: https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-darwin/beat=auditbeat,label=macosx/2102/

I think it was some kind of error in downloading the pip modules. I saw a similar failure in journalbeat.

// PythonIntegTest executes the python system tests in the integration environment (Docker).
func PythonIntegTest(ctx context.Context) error {
if !mage.IsInIntegTestEnv() {
mg.Deps(Fields)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it also depend on Dashboards? Without it, I get the following error when running mage integtest in beats/auditbeat:

# mage integtest
[...]
>> python test: Integration Testing
E.......
======================================================================
ERROR: Test that the dashboards can be loaded with `setup --dashboards`
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/go/src/github.com/elastic/beats/auditbeat/tests/system/test_base.py", line 67, in test_dashboards
    shutil.copytree(kibana_dir, os.path.join(self.working_dir, "kibana"))
  File "/usr/lib/python2.7/shutil.py", line 171, in copytree
    names = os.listdir(src)
OSError: [Errno 2] No such file or directory: '/go/src/github.com/elastic/beats/auditbeat/build/kibana'

With Dashboards, I get further, but it still fails:

>> python test: Integration Testing
F.......
======================================================================
FAIL: Test that the dashboards can be loaded with `setup --dashboards`
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/go/src/github.com/elastic/beats/auditbeat/tests/system/test_base.py", line 80, in test_dashboards
    self.run_beat(extra_args=["setup", "--dashboards"], exit_code=0)
  File "/go/src/github.com/elastic/beats/auditbeat/tests/system/../../../metricbeat/tests/system/../../../libbeat/tests/system/beat/beat.py", line 164, in run_beat
    return proc.check_wait(exit_code)
  File "/go/src/github.com/elastic/beats/auditbeat/tests/system/../../../metricbeat/tests/system/../../../libbeat/tests/system/beat/beat.py", line 93, in check_wait
    exit_code, actual_exit_code)
AssertionError: Expected exit code to be 0, but it was 1

From build/system-tests/run/test_base.Test.test_dashboards/auditbeat.log:

2018-12-04T22:30:25.815Z        ERROR   instance/beat.go:795    Exiting: Failed to import index-pattern: Failed to load directory /go/src/github.com/elastic/beats/auditbeat/build/system-tests/run/test_base.Test.test_dashboards/kibana/6/index-pattern:
  error loading /go/src/github.com/elastic/beats/auditbeat/build/system-tests/run/test_base.Test.test_dashboards/kibana/6/index-pattern/auditbeat.json: blocked by: [FORBIDDEN/12/index read-only / allow delete (api)];. Response: {"objects":[{"id":"auditbeat-*","type":"index-pattern","error":{"message":"blocked by: [FORBIDDEN/12/index read-only / allow delete (api)];"}}]}
Exiting: Failed to import index-pattern: Failed to load directory /go/src/github.com/elastic/beats/auditbeat/build/system-tests/run/test_base.Test.test_dashboards/kibana/6/index-pattern:
  error loading /go/src/github.com/elastic/beats/auditbeat/build/system-tests/run/test_base.Test.test_dashboards/kibana/6/index-pattern/auditbeat.json: blocked by: [FORBIDDEN/12/index read-only / allow delete (api)];. Response: {"objects":[{"id":"auditbeat-*","type":"index-pattern","error":{"message":"blocked by: [FORBIDDEN/12/index read-only / allow delete (api)];"}}]}

I didn't look more into why this is happening. Does it happen for you, too?

Update auditbeat and x-pack/auditbeat to share logic for generating config and packages.
This makes auditbeat and x-pack/auditbeat have independent `package` targets where auditbeat
generates only OSS packages and x-pack/auditbeat generates Elastic licensed packages.

And x-pack/auditbeat will now be tested on Travis CI.
@andrewkroh andrewkroh force-pushed the feature-auditbeat-host-build branch from 174ff37 to ef15dbe Compare December 6, 2018 22:16
@andrewkroh
Copy link
Member Author

@cwurm I think I fixed the issues that you encountered. I tested both auditbeat and x-pack/auditbeat on Linux.

  • I added Dashboards as a dependency on the pythonIntegTest target.
  • And I updated the integration tests executed by mage to fix file ownership like the make version does.

mg.Deps(makeConfigTemplates)
// PythonUnitTest executes the python system tests.
func PythonUnitTest() error {
mg.Deps(mage.BuildSystemTestBinary)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think PythonUnitTest should depend on Fields. Otherwise, it fails after a mage clean.

Copy link
Member Author

Choose a reason for hiding this comment

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

I’ll fix that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed.

@cwurm
Copy link
Contributor

cwurm commented Dec 7, 2018

Great, both mage UnitTest and mage IntegTest seem to be working now.

mage check is throwing some errors related to formatting for me:

  1. On macOS, it fails for me in both beats/auditbeat:
>> check: Checking for modified files or incorrect permissions
Error: some files are not up-to-date. Run 'mage fmt update' then review and commit the changes. Modified: [auditbeat/module/file_integrity/metricset_test.go auditbeat/module/file_integrity/monitor/monitor_test.go]

and x-pack/auditbeat:

>> check: Checking for modified files or incorrect permissions
Error: some files are not up-to-date. Run 'mage fmt update' then review and commit the changes. Modified: [x-pack/auditbeat/include/list.go]
  1. On Linux, it's just in beats/auditbeat. Not sure why it's different.

Are you seeing the same thing?

@andrewkroh
Copy link
Member Author

git diff should show you what changed and give you clue about the failure. IIRC on MacOS I was seeing a few files modified when running both mage fmt and make fmt so I don’t think there’s anything specifically wrong with the mage implementation of the fmt target but possibly something wrong with my dev environment that it differs from the results we get when running it on CI or linux.

@cwurm
Copy link
Contributor

cwurm commented Dec 7, 2018

In x-pack/auditbeat, mage update causes the change, specifically module_include_list.go:

diff --git a/x-pack/auditbeat/include/list.go b/x-pack/auditbeat/include/list.go
index fa0a85e49..d903941ba 100644
--- a/x-pack/auditbeat/include/list.go
+++ b/x-pack/auditbeat/include/list.go
@@ -8,6 +8,7 @@ package include

 import (
        // Import packages that need to register themselves.
+       _ "github.com/elastic/beats/x-pack/auditbeat/module/system"
        _ "github.com/elastic/beats/x-pack/auditbeat/module/system/host"
        _ "github.com/elastic/beats/x-pack/auditbeat/module/system/packages"
        _ "github.com/elastic/beats/x-pack/auditbeat/module/system/process"

In beats/auditbeat it's indeed mage fmt:

diff --git a/auditbeat/module/file_integrity/metricset_test.go b/auditbeat/module/file_integrity/metricset_test.go
index c171deeee..9f8849044 100644
--- a/auditbeat/module/file_integrity/metricset_test.go
+++ b/auditbeat/module/file_integrity/metricset_test.go
@@ -206,7 +206,7 @@ func TestExcludedFiles(t *testing.T) {
        }

        wanted := map[string]bool{
-               dir:                              true,
+               dir: true,
                filepath.Join(dir, "FILE.TXT"):   true,
                filepath.Join(dir, ".gitignore"): true,
        }
@@ -271,7 +271,7 @@ func TestIncludedExcludedFiles(t *testing.T) {
        }

        wanted := map[string]bool{
-               dir:                                    true,
+               dir: true,
                filepath.Join(dir, ".ssh"):             true,
                filepath.Join(dir, ".ssh/known_hosts"): true,
        }
diff --git a/auditbeat/module/file_integrity/monitor/monitor_test.go b/auditbeat/module/file_integrity/monitor/monitor_test.go
index 0b496c8e6..729184c9f 100644
--- a/auditbeat/module/file_integrity/monitor/monitor_test.go
+++ b/auditbeat/module/file_integrity/monitor/monitor_test.go
@@ -248,7 +248,7 @@ func TestRecursiveSubdirPermissions(t *testing.T) {
        // File "b/b" is missing because a watch to b couldn't be installed

        expected := map[string]fsnotify.Op{
-               dest:                       fsnotify.Create,
+               dest: fsnotify.Create,
                filepath.Join(dest, "a"):   fsnotify.Create,
                filepath.Join(dest, "a/a"): fsnotify.Create,
                filepath.Join(dest, "b"):   fsnotify.Create,

Both changes don't seem harmful, so I'm happy to just go with them if it makes mage happy.

@andrewkroh
Copy link
Member Author

+ _ "github.com/elastic/beats/x-pack/auditbeat/module/system"

The addition of this package does not happen on this branch. That should only occur if the package has a _meta dir and contains .go files. Maybe you have some files that are not checked in to this branch? I have a future enhancement that also checks that something in the packages uses func init() b/c otherwise it doesn't make sense to do an import because its impossible for the package import to have a side-effect.

In beats/auditbeat it's indeed mage fmt

I see the same thing when running make fmt or directly goimports (which is odd because the format is wrong and it does not occur on linux). So I don't believe it's anything new in this PR.

@cwurm
Copy link
Contributor

cwurm commented Dec 7, 2018

That should only occur if the package has a _meta dir and contains .go files.

Ah, I found it. For some reason, I have a fields.go that's not checked in under system/. My mistake.

I see the same thing when running make fmt or directly goimports (which is odd because the format is wrong and it does not occur on linux). So I don't believe it's anything new in this PR.

Ok, we can deal with it later.

@andrewkroh andrewkroh merged commit e05a6f1 into elastic:feature-auditbeat-host Dec 7, 2018
cwurm pushed a commit to cwurm/beats that referenced this pull request Dec 10, 2018
* Refactor Auditbeat build logic

Update auditbeat and x-pack/auditbeat to share logic for generating config and packages.
This makes auditbeat and x-pack/auditbeat have independent `package` targets where auditbeat
generates only OSS packages and x-pack/auditbeat generates Elastic licensed packages.

And x-pack/auditbeat will now be tested on Travis CI.

* Skip failing Auditbeat system module test
* Skip failing system/process test
* Add temporary target alias for Windows CI
* Fix file permission issues caused by Docker usage
* Optimize chown by checking if UID/GID need changed
cwurm pushed a commit to cwurm/beats that referenced this pull request Dec 16, 2018
* Refactor Auditbeat build logic

Update auditbeat and x-pack/auditbeat to share logic for generating config and packages.
This makes auditbeat and x-pack/auditbeat have independent `package` targets where auditbeat
generates only OSS packages and x-pack/auditbeat generates Elastic licensed packages.

And x-pack/auditbeat will now be tested on Travis CI.

* Skip failing Auditbeat system module test
* Skip failing system/process test
* Add temporary target alias for Windows CI
* Fix file permission issues caused by Docker usage
* Optimize chown by checking if UID/GID need changed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants