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

[Feature Request] Adding flag to run system tests without cleanup #582

Closed
P1llus opened this issue Nov 15, 2021 · 11 comments · Fixed by #1250
Closed

[Feature Request] Adding flag to run system tests without cleanup #582

P1llus opened this issue Nov 15, 2021 · 11 comments · Fixed by #1250
Assignees
Labels

Comments

@P1llus
Copy link
Member

P1llus commented Nov 15, 2021

In certain usecases it would be preferred that we run our system tests without doing the cleanup process at the end.

For example when creating dashboards for a package that we only have local testdata for, that we cannot integrate with properly, ingesting the test data is crucial for dashboard creation.

Adding this issue after a quick discussion with @mtojek on the subject, so we can look into it further.

@P1llus P1llus added the discuss label Nov 15, 2021
@mtojek mtojek added the good first issue Good for newcomers label Nov 19, 2021
@andrewkroh
Copy link
Member

I had been commenting out the code that deletes events in the system test running so I could build dashboards so I think this would be a good feature to add.

I also manually go in to Kibana to grab data for sample_event.json after running the system tests. If there were an option to dump the system test events to file I would use that to find file a suitable event for the sample_event.json.

@mtojek
Copy link
Contributor

mtojek commented May 16, 2022

Temporarily you can use the "timeout" flag to defer cleanup of documents.

@jsoriano
Copy link
Member

For example when creating dashboards for a package that we only have local testdata for, that we cannot integrate with properly, ingesting the test data is crucial for dashboard creation.

Maybe we should think specifically about the use case of building dashboards. For example, instead of having a flag to avoid cleanup of system tests, we could maybe have a command that starts the service and configures the agent to collect data from it. It could be a subcommand of elastic-package service.
This, or other command, could also record the data as fixtures that can be used in the future without needing to start the service.
Having a tool to record and reuse fixtures would also help to use real-world data instead of using the same data used on system tests.

In a more general sense, it may be also nice to add some mechanism to configure generators, that can be used for any purpouse that needs real-like data (sample data for dashboards, performance testing, mocking services for tests...).

I also manually go in to Kibana to grab data for sample_event.json after running the system tests. If there were an option to dump the system test events to file I would use that to find file a suitable event for the sample_event.json.

For this it'd be nice to have something as the conditions mechanism we have in Metricbeat, so events are still collected automatically, and the selection factor can be translated to config or code. Related: #773.

Said that, I am not oposed to have some mechanism to control tear downs 🙂 But for these specific use cases I would prefer to have more specific solutions.

@andrewkroh
Copy link
Member

For example, instead of having a flag to avoid cleanup of system tests, we could maybe have a command that starts the service and configures the agent to collect data from it.

I like the idea of a specialized command.

@mrodm
Copy link
Contributor

mrodm commented Feb 6, 2023

Thinking about options for this issue, I see some different approaches.

Adding a subcommand or parameter under elastic-package install:

  • Following the logic that is performed in tests, that command would install the package/integration and assign the new policy to the agent so that agent can start collecting data.
    • Another command would be needed to tear down the service or at least remove the configurations
  • Example: elastic-package install --setup
  • Workflow for developers:
    elastic-package install --setup
    elastic-package service up

Adding a subcommand or parameter under elastic-package service:

  • Following the logic that is performed in tests, that command would boot the service and it would also install the package/integration and assign the new policy to the agent so that agent can start collecting data.
    • Another command would be needed to tear down the service or at least remove the configurations
  • Example: elastic-package service up --setup
  • Workflow for developers:
    elastic-package service up --setup

Adding a subcommand or parameters under elastic-package test:

  • Separate from the test runners, the logic that install the package and setup everything to start collecting data.
  • It needs to be checked how the workflow for developers would be
    • If the integration is installed, an execution of elastic-package test -v probably it should not uninstall the package/tear down the service.

cc @elastic/ecosystem

@jsoriano
Copy link
Member

jsoriano commented Feb 6, 2023

@mrodm could you add the pros/cons, or if you have a prefered option?

I like this workflow, it is explicit about the different steps:

elastic-package install --setup
elastic-package service up

Is the --setup flag needed in install? I think it'd be ok if after these steps there is still no data ingested. Then the user can run tests or any other process to ingest data.

On the two first cases we would still need to add some flag to test so it doesn't tear down what was created by the previous commands, right?

elastic-package test --no-teardown

@mrodm
Copy link
Contributor

mrodm commented Feb 6, 2023

I would prefer the first option too, as you mention it is more explicit:

  1. Install package: elastic-package install
  2. Start the service: elastic-package service up

I thought that elastic-package install would be the one in charge of managing the policy and related steps, WDYT @jsoriano ?

Is the --setup flag needed in install?
At first, I thought to add this parameter to not change the behavior of elastic-package install command.

On the two first cases we would still need to add some flag to test so it doesn't tear down what was created by the previous commands, right?

elastic-package test --no-teardown

Yes, that's right. That parameter should be added too.
Maybe more changes are required in test subcommand if the package is already installed and collecting data ? Another parameter --no-install ?

And, should it also be added a new command/subcommand to tear-down the service under uninstall ?

An example of this scenario:

# Install package and prepare policy and agent
elastic-package install

# Boot up service, as the agent should have the config it should collect data
elastic-package service up

# Run tests with --no-teardown
elastic-package test --no-teardown [--no-install?]

# Remove package and related policy and configuration
elastic-package uninstall

@jillguyonnet jillguyonnet self-assigned this Feb 7, 2023
@jsoriano
Copy link
Member

jsoriano commented Feb 7, 2023

TLDR; Thinking about the scope of elastic-package install I have changed my opinion to prefer something like the second option in #582 (comment). To add --add-policy/--setup in elastic-package service up, that adds a policy apart of installing the service.

I thought that elastic-package install would be the one in charge of managing the policy and related steps, WDYT @jsoriano ?

elastic-package install already exists, it installs a package. As it is now it doesn't manage any policy and it'd be probably better to continue this way.

Is the --setup flag needed in install?

At first, I thought to add this parameter to not change the behavior of elastic-package install command.

+1 to don't change the behaviour or current install command.

The workflow you mention looks good, with the only point that I wouldn't mix policies with package installation (this is always a bit mixed, but let's try to avoid that here).

I think I would prefer to have the policy assigned in elastic-package service up, this command knows about the service and has information to configure the policy. It also seems to support specific variants and data streams. install doesn't have all this information.
It could also take care of installing the package if it hasn't been installed yet, or it could fail telling the user to run elastic-package install.

It could be done with a new flag:

elastic-package service up --add-policy

As the command works now, it is executed on the foreground and stops the service on interruption. We should remove the policy when stopping the service too.

Regarding the test commands, they would likely work with this workflow. As a different task, maybe we could have the following ones:

elastic-package test [asset|pipeline|system] setup  # It starts the service and installs the package and the policy according to the package configuration. aka elastic-package install + elastic-package service up --add-policy.
elastic-package test [asset|pipeline|system] --no-provision # It runs tests, skipping both setup and teardown phases.
elastic-package test [asset|pipeline|system] teardown # It executes the teardown of the test.

That would translate into refactoring the test runner interface to split the Run into Setup and Run.

@jillguyonnet jillguyonnet removed their assignment Feb 7, 2023
@mrodm
Copy link
Contributor

mrodm commented May 3, 2023

Looking at this offline with @jsoriano, it has been checked that in order to run a service and configure it properly so the elastic-agent can retrieve the documents, it is needed to have available the settings that are used currently in system tests. For instance zoom test config

Among others it is needed to have the policy template name, input, datastream among others.

For that, we have been checking that it could be worthy to focus on add this functionality into elastic-package test command, as proposed also here #582 (comment).

It could be started by adding just the subcommands setup and teardown:

  • elastic-package test [asset|pipeline|system] setup
  • elastic-package test [asset|pipeline|system] teardown

Taken into account that running elastic-package test [asset|pipeline|system] command should have the same behaviour as it does now (setup, run tests and teardown).

@agithomas
Copy link
Contributor

> elastic-package service up
> 
> # Run tests with --no-teardown
> elastic-package test --no-teardown [--no-install?]

Recently we have taken up projects such as Lens Migration, and TSDB enablement. Having the below-mentioned abilities really would have helped the project. Instead, some of us tweaked the elastic-package code to achieve the functionalities. But, it is good to make these features available to all so that they could take benefit of it.

  1. Create a service instance with elastic-package service up --add-policy.
  • When we have to test for compatibility checks or issue debugging, this certainly helps. If there exists a way to override the version of the image used, making use of the command-line argument, many more benefits we can obtain.
  1. Defer the tear down with elastic-package test --no-teardown. Not only for Lens migration, but also for TSDB checks for dashboard errors, this feature can be super beneficial.

Upvoting the feature request and also placing a request for prioritising, knowing the benefits of the above requirement.

@jsoriano jsoriano removed the good first issue Good for newcomers label Dec 7, 2023
@mrodm
Copy link
Contributor

mrodm commented Feb 15, 2024

Closing this issue after merging #1250

@mrodm mrodm closed this as completed Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants