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

pants: Add system_user detection to pants-plugins/uses_services (+ mongo detection improvements) #6244

Merged
merged 4 commits into from
Sep 19, 2024

Conversation

cognifloyd
Copy link
Member

@cognifloyd cognifloyd commented Sep 17, 2024

Some of the tests require the system_user.user as an actual local user on the machine running pytest. That is now configurable with the ST2TESTS_SYSTEM_USER env var added in #6242. If someone forgot to set--or did not know they could set--the ST2TESTS_SYSTEM_USER env var, they will get really odd errors that are difficult to interpret and debug. So, this PR adds a pants-plugin feature to fail with an error message about setting that env var before pytest even runs tests that require the system_user.

This improves on the pants-plugins/uses_services plugin, which was added in these PRs (the PR descriptions can be helpful in understanding the uses_services plugin):

has_system_user.py script and the rule that runs it

has_system_user.py is the part that checks to see if the system_user is present on the system.

The has_system_user.py script gets opened and run in a pex with the similar rule logic to the mongo, redis, and rabbitmq detection rules.

Only system_user has to be passed to the has_system_user.py script to check for that user:

@dataclass(frozen=True)
class UsesSystemUserRequest:
"""One or more targets need the system_user (like stanley) using these settings.
The system_user attributes represent the system_user.user settings from st2.conf.
In st2 code, they come from:
oslo_config.cfg.CONF.system_user.user
"""
system_user: str = "stanley"

Here is the definition of the rule that runs has_system_user.py:

async def has_system_user(
request: UsesSystemUserRequest, platform: Platform
) -> HasSystemUser:

So, when another rule Gets HasSystemUser with a UsesSystemUserRequest, pants will also run the rule that generates Platform (described in #5864), and then it will run this has_system_user rule.

The has_system_user rule either returns HasSystemUser() if the user is present, or raises ServiceMissingError if it is not (the same error that the other uses_services rules raise). By raising an error, this actually breaks a convention for pants rules. Exceptions stop everything and get shown to the user right away, and for most goals, pants wants to see some dataclass returned that has something like a succeeded boolean instead. But, we want to error as early as possible, so this breaks that convention on purpose.

wiring up the test goal so it runs has_system_user when pytest runs on a target with the uses field.

The last piece that ties this all together is a rule that makes sure the has_system_user rule runs before pytest runs (if pants is running it on a target with the uses field). Here is the definition of the has_system_user_for_pytest rule:

async def has_system_user_for_pytest(
request: PytestUsesSystemUserRequest,
test_extra_env: TestExtraEnv,
) -> PytestPluginSetup:

This rule is fairly simple. It looks up the system_user based on the [test].extra_env_vars setting defined here:

st2/pants.toml

Lines 242 to 247 in 1413e11

[test]
extra_env_vars = [
# Use this so that the test system does not require the stanley user.
# For example: export ST2TESTS_SYSTEM_USER=${USER}
"ST2TESTS_SYSTEM_USER",
]

Then it just triggers running the has_system_user rule, passing in the system_user:

system_user = test_extra_env.env.get("ST2TESTS_SYSTEM_USER", "stanley")
# this will raise an error if system_user is not present
_ = await Get(HasSystemUser, UsesSystemUserRequest(system_user=system_user))
return PytestPluginSetup()

This rule needs the PytestUsesSystemUserRequest which selects targets that have the uses field.

class PytestUsesSystemUserRequest(PytestPluginSetupRequest):
@classmethod
def is_applicable(cls, target: Target) -> bool:
if not target.has_field(UsesServicesField):
return False
uses = target.get(UsesServicesField).value
return uses is not None and "system_user" in uses

This request will be made by the pants-internal pytest rules thanks to this UnionRule, which is a way to declare that our class "implements" the PytestPluginSetupRequest:

UnionRule(PytestPluginSetupRequest, PytestUsesSystemUserRequest),

If we need to add uses support to other targets, we will need to find similar UnionRules where we can inject our rule logic. For now, I've only wired this up so our uses rules will run for pytest.

update mongo bits to use pants idiom

This PR also updates the mongo detection rules to use pants to grab the ST2TESTS_PARALLEL_SLOT env var. I learned about how to do this recently, so I included that update here.

Pants is configured to use ST2TESTS_PARALLEL_SLOT here:

st2/pants.toml

Lines 229 to 234 in 1413e11

[pytest]
install_from_resolve = "pytest"
args = [
"--no-header", # don't print pytest version for every tested file
]
execution_slot_var = "ST2TESTS_PARALLEL_SLOT"

So, the mongo_is_running_for_pytest rule passes that var name here:

_ = await Get(
MongoIsRunning,
UsesMongoRequest(execution_slot_var=pytest.execution_slot_var or ""),
)

And this line tells pants to include the var in the env used when running is_mongo_running.py here:

execution_slot_variable=request.execution_slot_var,

Finally the db name calculation is moved into is_mongo_running.py here:

db_name = args.get(3, "st2-test{}")
connection_timeout_ms = args.get(4, 3000)
slot_var = args.get(5, "ST2TESTS_PARALLEL_SLOT")
db_name = db_name.format(os.environ.get(slot_var) or "")

@cognifloyd cognifloyd added this to the pants milestone Sep 17, 2024
@cognifloyd cognifloyd self-assigned this Sep 17, 2024
@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Sep 17, 2024
@cognifloyd cognifloyd force-pushed the pants-uses_services-system_user branch from 67aefa9 to af11f97 Compare September 17, 2024 02:35
@cognifloyd cognifloyd force-pushed the pants-uses_services-system_user branch from af11f97 to 8337d97 Compare September 17, 2024 14:42
@cognifloyd cognifloyd force-pushed the pants-uses_services-system_user branch from 8337d97 to dc5a86b Compare September 17, 2024 15:24
@cognifloyd cognifloyd requested a review from a team September 17, 2024 18:46
@cognifloyd cognifloyd merged commit 5a833f2 into master Sep 19, 2024
29 checks passed
@cognifloyd cognifloyd deleted the pants-uses_services-system_user branch September 19, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure: ci/cd pantsbuild size/L PR that changes 100-499 lines. Requires some effort to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants