add opt. service deps, opt-in services to test selection

[alexrashed]

Motivation

Now that we have the selective test execution for opt-in services introduced in #10301, we can onboard additional services to the test selection.
In this PR I added the services I own to the list (😻), but it might make sense to collect the first iteration of opt ins here and merge them together.
So please feel free to push to this PR if you want the tests of your service being selectively executed in PRs here.
In order to to so, just:

1. Make sure the mandatory and optional service dependencies in localstack/utils/bootstrap.py are correct.

• They should list your service (if it's not completely self-contained).
• API_DEPENDENCIES
  • This mapping states that a service requires one or more other services to provide it's basic functionality.
  • f.e. the following example means that firehose depends on kinesis to work:

    "firehose": ["kinesis"],

• API_DEPENDENCIES_OPTIONAL
  • This mapping states that a service needs one or more other services for certain non-basic parts, f.e. with optional integrations.
  • On the example of firehose that would be at least opensearch and elasticsearch, because these are potential targets for firehose streams.

2. Add your service provider code and the test code to the OPT_IN. The list is alphabetically sorted, I expect it to grow quite fast (and then become obsolete).

Changes

• Introduces the notion of optional service dependencies, in contrast to "normal" / mandatory service dependencies.
• Adds the following services to the Opt-In of the selective test execution:
  • ACM
  • ElasticSearch
  • OpenSearch
  • SNS

TODO

• Discuss if we should split the optional from the mandatory service dependency definition.
  • API_DEPENDENCIES is used for strict service loading, where it is used as a mandatory service definition.
    • f.e. lambda cannot work without s3 and sts
  • For the test selection, we are using this mapping for all dependencies (including optional dependencies).
    • f.e. firehose can work without opensearch for as long as you don't use it as a target. However, it cannot work without kinesis at all.
  • /cc @bentsku @steffyP @thrau

[github-actions]

LocalStack Community integration with Pro

    2 files  ± 0      2 suites  ±0   1h 30m 3s ⏱️ -32s
2 725 tests +15  2 470 ✅ +15  255 💤 ±0  0 ❌ ±0 
2 727 runs  +15  2 470 ✅ +15  257 💤 ±0  0 ❌ ±0 

Results for commit 0195dfa. ± Comparison against base commit 7494fd5.

♻️ This comment has been updated with latest results.

[bentsku]

LGTM! Nice, that looks really neat now, thank you so much for updating it. Can I still push SNS to the list of opt-in, even though it's ready for review?
Also, thanks for updating the PR description, it's really clear 👍

[alexrashed]

LGTM! Nice, that looks really neat now, thank you so much for updating it. Can I still push SNS to the list of opt-in, even though it's ready for review? Also, thanks for updating the PR description, it's really clear 👍

Of course, please feel free to add your opt-ins, I'll merge this PR early next week with all the added opt-ins. :)
This obviously also counts for all other service owner's opt ins, but we can also just add more in a follow-up PR or individually! 🚀

[bentsku]

This is not exactly related to this PR, but to the previous which added the test selection. I think we have a recursion problem when expending the list, I've tried to add SNS and related services, and we can't resolve the list anymore.

If we imagine we have the following (real) optional dependencies:

API_DEPENDENCIES_OPTIONAL = {
    "firehose": ["opensearch", "es", "s3"],
    "lambda": ["logs", "cloudwatch"],
    "ses": ["sns"],
    "sns": ["sqs", "lambda", "firehose", "ses", "logs"],
    "sqs": ["cloudwatch"],
    "logs": ["lambda", "kinesis", "firehose"]
}

We will forever be trying to resolve the logs service. Lambda will publish to logs, so if a change to logs is done, we would want to execute lambda tests (and sns with this list). But logs can also publish to a lambda destination, and now we have a recursion loop.
We have the same problem with ses and sns, both can interact with each other.

To solve this, I believe we might not need to actually do a recursive call, because we would trigger more tests that would not be needed.

Let's take the same list as above. Let's say I'm doing a change in ses. sns has some dependency/tests with ses, so we want to execute them. But I don't think that means we want to execute all dependencies of sns, as we did not touch this service. sns will take care itself of testing the changes with ses. If we're testing multiple integrations like like let's say sns into lambda into logs into kinesis into whatever next, I think a scenario test could do it, but I don't think we need to run the whole test suite.

Maybe I am missing something, but do you think this is necessary? Should we just reverse-resolve the API_DEPENDENCIES_OPTIONAL once? I believe the list will be clear once we add all services there, and would not need recursion (in _expand_api_dependencies). And with recursion and the full list of all dependencies, we would most probably execute almost all tests.

I have a commit ready I could push that fixes this issue, if you'd like. I could also open a different PR, as the scope is creeping a little 😅

\cc @dominikschubert @alexrashed

[dominikschubert]

I agree with @bentsku. We should for now avoid testing the full transitive relation here and just consider the first level of dependencies. Doing this fully recursively with all the potential service integrations is going to kinda defeat the whole purpose here.

I'd also suggest we separate the opt-in gathering and the introduction of the optional dependencies into a separate PR, but I'm also not opposed to just bundle it here if that's ok with @alexrashed.

[alexrashed]

Please just go ahead and push it directly in this PR, @bentsku :)

[bentsku]

Please just go ahead and push it directly in this PR, @bentsku :)

I've added SNS and some known dependencies linked to it with 555ce88

I've also updated the recursion and added a test for services depending on each other with cb6be45

[steffyP]

Thanks for jumping onto fixing issues with the dependencies and test selection @alexrashed and @bentsku 🙏 🚀

[dominikschubert]

Suggested change

	# - an optional service dependency is a service without which a service's basic functionality breaks,
	# - an optional service dependency is a service without which a service's basic functionality doesn't break,

[alexrashed]

Actually, I think the sentence is good, because it states without the service will break, right?

[dominikschubert]

but I thought the point is that the basic functionality of a service does not break if the service it optionally depends on is disabled?

[alexrashed]

You are totally right 😄

[dominikschubert]

It's not super urgent here, but should we maybe try add a reasoning for the specific dependencies as well? E.g. separating these on different lines for event source mappings vs. internal service dependencies.

[bentsku]

I agree, I also thought of it. It's kinda related to "upstream" vs "downstream". With event source mapping, it's a bit tricky... not many services are like this I believe?

Most of the current dependencies on these list are "targets" or external dependency like "global" services for telemetry / observability / events (events, logs, cloudwatch) I believe. Except for event source mapping 😅

[dominikschubert]

Generally I'd treat these dependencies just like we do for testing. If it's configured via the Lambda API, it's a dependency of Lambda, no matter if it's a "source" or a "sink"/"destination".