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 new flags to just setup, teardown or run tests only #1250

Merged
merged 112 commits into from
Feb 15, 2024

Conversation

mrodm
Copy link
Contributor

@mrodm mrodm commented May 5, 2023

Closes #582

This PR is intended to add the required subcommands and/or parameters to allow just run the setup, no-provision or teardown of tests. Focused on system tests.

Added flags:

  • --setup
    • Creates all the resources , install the package, enrolls the agent
    • It ensures that some docs are stored in ES
    • Writes into ServiceSetupDir folder the original policy assigned to the agent as well as the policy created by elastic-package
  • --no-provision
    • Reads both policies from ServiceSetupDir
    • Install the package (in case there are changes locally)
    • Executes all the system tests
    • Does not remove the logs folder to avoid issues in service writing to those files (they should be truncated instead)
  • --tear-down
    • Reads both policies from ServiceSetupDir
    • Runs all the handlers to remove all resources created
    • Stop the service

Pending:

  • Add creation of a new folder under the current profile directory for test resources
  • Store needed resources between Setup, NoProvision and Teardown in a folder under the current profile (.e.g. `~/.elastic-package/profiles/default/testing/*). Examples of resources to be stored:
    • Original policy assigned before changing it in the setup method, so TearDown step can restore it
       $ ls -l ~/.elastic-package/profiles/default/stack/state/
      total 4
      -rw-r--r-- 1 user user 773 feb 14 17:15 service.json
  • Add methods and CLI parameters to Setup and TearDown for just one config and variant in system test
    • Those new parameters could be added just
      • if a new method in TestRunner interface like CanRunSetupTeardownIndependent() or CanRunSeparateTests returns true
      • if the runner implements a new interface (e.g. TestRunnerSetterUp)
        type TestRunnerSetterUp interface {
         	// Setup installs package and starts the service
         	Configure(TestOptions) error
        
        	// Setup installs package and starts the service
        	Setup(TestOptions) error
        }
        
  • Ensure this works also for input packages - tested with sql_input
  • CI step
  • Add new section in docs:

Checklist

  • Test with an integration package (e.g. nginx - integrations)
  • Test setting a non-default variant (e.g. mysql percona_8_0_13 - integrations)
  • Test with an input package (e.g. sql_input - integrations)
  • Test with a package using custom agents (e.g. auditd_manager - elastic-package)
  • Test with a package using kind as a service deployer (e.g. kubernetes - elastic-package)

How to test locally

cd /path/integrations/packages/mysql

# Start Elastic stack as usual
elastic-package stack down -v
elastic-package stack up -v -d

# Go to a package from integrations repository with variants, taken as example `mysql`
elastic-package test system -v --config-file data_stream/status/_dev/test/system/test-default-config.yml --variant percona_8_0_36 --setup
elastic-package test system -v --no-provision
elastic-package test system -v --tear-down

# Go to a package from integrations repository without variants, taken as example `hashicorp_vault`
elastic-package test system -v --config-file data_stream/audit/_dev/test/system/test-tcp-config.yml --setup

# this command can be executed several times
elastic-package test system -v --no-provision

elastic-package test system -v --tear-down


# Go to an input package, taken as example `sql_input`
elastic-package test system -v --config-file _dev/test/system/test-mysql-config.yml --setup

# this command can be executed several times
elastic-package test system -v --no-provision

elastic-package test system -v --tear-down

elastic-package stack down -v

@sharbuz
Copy link
Contributor

sharbuz commented Jul 10, 2023

Hi everyone,
Please update the branch because we merged an important change to the main branch: #1347

Comment on lines +99 to +101
if options.RunSetup || options.RunTearDown || options.RunTestsOnly {
return nil, errors.New("terraform service deployer not supported to run by steps")
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, this service deployer is not supported. I'd prefer to just leave this option (test flags) for stacks/services started locally for now.

This the error that is going to be shown to the user:

2024/02/15 09:44:36 DEBUG setting up service...
Error: error running package system tests: could not complete test run: could not create service runner: terraform service deployer not supported to run by steps

Copy link
Member

Choose a reason for hiding this comment

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

👍 we can add this later

@mrodm mrodm requested a review from jsoriano February 15, 2024 10:15
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

I think we can ship it ⛵
Thanks!

Comment on lines +99 to +101
if options.RunSetup || options.RunTearDown || options.RunTestsOnly {
return nil, errors.New("terraform service deployer not supported to run by steps")
}
Copy link
Member

Choose a reason for hiding this comment

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

👍 we can add this later

Comment on lines 996 to 997
dirPath := filepath.Dir(r.serviceStateFilePath)
err := os.RemoveAll(dirPath)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should remove the file only, in case we use the directory later for other states. Leaving the directory there should not be a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes totally sense.
Update this function to just delete the file, and update the tests accordingly.

Comment on lines 108 to 109
// variant flag should not be used with tear-down or no-provision flags
// cannot be defined here using MarkFlagsMutuallyExclusive as in --config-file
Copy link
Member

Choose a reason for hiding this comment

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

This comment seems a bit lonely around here, not clear to what it refers to. Or is another mutual exclusion missing here?

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 wanted to add a note that I could not set the mutually exclusive using testTypeCmd.MarkFlagsMutuallyExclusive. Variant flag is defined in the cmd variable and not in the testTypeCmd.

This is managed afterwards in the code:
https://github.com/elastic/elastic-package/pull/1250/files#diff-33701d098205d3537f071235876d903ddbfa851c492917e49852c9212bc1d4fdR316-R320

I'll update this comment with that reference

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @mrodm

@mrodm mrodm merged commit 81289fa into elastic:main Feb 15, 2024
3 checks passed
@mrodm mrodm deleted the add_flag_setup_testrunner branch February 15, 2024 12:09
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.

[Feature Request] Adding flag to run system tests without cleanup
4 participants