-
Notifications
You must be signed in to change notification settings - Fork 42
fix: remove references to the default policy #281
Conversation
Internally, we were using the ID, so it should work as expected, but the specs were not in sync
💔 Tests FailedExpand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
Hi, thanks Manu - so the implication here is that each scenario runs in a separate, newly created (for that test) policy? That is certain to avoid failures / clean-up related problems we may have had, its a good addition. I see 2 areas to discuss / confirm:
|
Yes, the policy (master)/configuration (7.9.x) will be created before each scenario, so that each agent will be enrolled using this new policy.
I'd suggest working on this at the moment we need it, not now.
It's created at the beginning of the test execution, in the beforeSuite (before any scenario) hook. We have a Fleet setup, which calls a kibana URL for that. I guess it creates the default policy. I'd ask about what the implications of using the default policy are, to understand if we need specific scenarios for it. As of today, where a policy seems to be exactly the same as the default one (or at least I cannot distinguish any difference), I'd treat default as any other one, so I do not see a reason to not use the created ones... Unless we find a specific use case for the default policy |
@ph or @jen-huang maybe - Manu's question is exactly what I would like to confirm. The policies are supposed to be the same, and are currently - if there is any need to test the 'default' policy especially we need to know it now so we can create a new test case for it. |
@EricDavisX @mdelapenya What you are describing is exactly what is happening behind the scene when setup is run the default policy is created and necessary packages are installed for the system integration. Testing the default policy would be testing logs and metrics are collected on the machine and I think we should have a test for that, just for sanity. Note, I would keep the "assertion" as minimal as possible, because the system integration is in flux at the moment and we are iterating to reduce the scope of that integration. Reduce the metrics and the logs we are collecting and dealing with the difference between the operating system. |
I think we can merge this, and move the discussion about default policies to another issue, as we already merged the usage of a new policy for each scenario in #279 |
As discussed on Slack, I'm gonna merge this little one. Thanks! |
Internally, we were using the ID, so it should work as expected, but the specs were not in sync # Conflicts: # e2e/_suites/ingest-manager/features/agent_endpoint_integration.feature # e2e/_suites/ingest-manager/fleet.go
… | fix: remove references to the default policy (#281) backport for 7.9.x (#283) * feat: support creating (and removing) a policy for each scenario (#279) * feat: support creating a policy per scenario, not reusing default one * chore: extract tear down code to afterScenario methods * fix: move creation of the policy to the before scenario hook * fix: use proper URL after bad copy&paste * chore: add debug logs for removals # Conflicts: # e2e/_suites/ingest-manager/fleet.go # e2e/_suites/ingest-manager/ingest-manager_test.go * fix: remove references to the default policy (#281) Internally, we were using the ID, so it should work as expected, but the specs were not in sync # Conflicts: # e2e/_suites/ingest-manager/features/agent_endpoint_integration.feature # e2e/_suites/ingest-manager/fleet.go * chore: remove dependency added by mistake
agree - thanks. and it seems (still confirming) that we need not re-build any specific test for the Ingest Manager provided 'pre-built' policy, I believe it is programmatically created and is like any other policy so this change was quite harmless to test coverage (just needed to be confirmed). Also, indeed, we can continue tests of the System Integration at all speed. I found we didn't have a ticket for that, so I added one: https://github.com/elastic/e2e-testing/issues/284 |
What does this PR do?
This PR updates the specs/scenarios removing any reference to the default policy, because since #279 we are creating a policy for each scenario.
This change does not affect the tests, as we are internally using the policyID, so it should work as expected.
Why is it important?
Checklist
make notice
in the proper directory)How to test this PR locally
Run:
Related issues