Document environment variable precedence

[happz]

Related to #4241

TODO

• untangle plan environment from test environment, and add plan environment to the final environment in the execute plugin
  • Do not inject plan environment into test environment #4455
• drop the duplication of "run" environment, run.environment and "plan CLI environment" seems to be the same thing: Refactor how tmt run collects its environment #4415
  • refactor run environment to use the same composition approach so we can better track the sources: Refactor how tmt run collects its environment #4415
• move TMT_SOURCE_DIR addition elsewhere - when it's needed, who needs to add it, do we need another context, etc.
• plan environment file - when it's pulled from guests, is it saved into the same runner path? Plan may prepare different content on different guests, sharing single local path might be incorrectly enforcing one version to all guests. Possibly a bug waiting to be noticed.
• Attach test environment to its invocation #4452
• Move test check invocation under the umbrella of test invocation #4453

Pull Request Checklist

• implement the feature
• write the documentation
• extend the test coverage
• update the specification
• adjust plugin docstring
• modify the json schema
• mention the version
• include a release note

[happz]

Ok, kicking of with a summary and a couple of notes and TODO items while my memory is reasonably fresh.

See https://tmt--4364.org.readthedocs.build/en/4364/environment.html for the preview.

Ignore the location of the document, I just needed to hook it somewhere.

[psss]

Synced priority with the issue and proposed for the next sprint as agreed on the hacking session.

[LecrisUT]

Do we document as this order first or invert the preference and document our intended order?

As for the order preference, I think it's better to be:

1. Plan env
2. Guest env (combined in plan env)
3. Test env
4. tmt controlled

[happz]

At this moment, what I have here is a collection of inputs as they are now, i.e. documenting the current state. It needs (senior) developers' input on what should be the desired order, which we would then document and test.

[LecrisUT]

I think we should just drop this and do the expansion at the caller or property to see the exact order being used.

This being done at the discover level? would indicate that the later step environments are not updated? I believe that's not the case since TMT_PLAN_ENVIRONMENT_FILE is being processed, but maybe that one is a special case altogether? The deciding parameter should be the guest env since that is only defined after the provision.

[happz]

In any case, it's much harder to control and follow. A plan environment being injected into test environment so early means it's there, forever, and from this moment there is no "plan environment" but only "test environment". I believe this needs to go, "plan environment" needs to be added in the correct order where the command environment is composed from components (execute/internal, prepare/*, finish/*).

[LecrisUT]

This one is a rather special case because it is being written on the guest. Let me verify the logic, after a PrepareShell step the plan data is synced back so any changes to TMT_PLAN_ENVIRONMENT_FILE are reflected back to the runner, and the runner always reads this source when needing to evaluate the plan env, e.g. when running ansible plugin.

But then what happens in the multi guest case? Isn't the plan data still shared between all guests?

[happz]

But then what happens in the multi guest case? Isn't the plan data still shared between all guests?

The puzzlement comes from the fact you speak about "plan data", I'm afraid we need to be more precise as there is so many of different inputs, and "plan environment" can mean a lot of them.

Yes, a test invoked on multiple guests shares some portion of environment: from run, from plan's keys, from test keys, from the importing plan. They also share the intrinsic bits - TMT_TREE, TMT_TEST_NAME, etc. They do not share environment from guest keys, they do not share environment from the plan environment file, as it may be have different content across guests. So, yeah, "plan data" is shared, but also isn't, depending of what you ask about.

[LecrisUT]

The puzzlement comes from the fact you speak about "plan data"

By plan data there, I mean the plan_workdir a.k.a. TMT_PLAN_DATA, under which the physical TMT_PLAN_ENVIRONMENT_FILE is located.

after a PrepareShell step the plan data is synced back so any changes to TMT_PLAN_ENVIRONMENT_FILE are reflected back to the runner,

they do not share environment from the plan environment file

So when doing guest.pull is there a filter to not pull $TMT_PLAN_DATA/variables.env? There are multiple aspects that seem to be in conflict with each other between:

• TMT_PLAN_DATA being shared across guests
• TMT_PLAN_ENVIRONMENT_FILE=$TMT_PLAN_DATA/variables.env
• Reading back changes to TMT_PLAN_ENVIRONMENT_FILE

[happz]

The puzzlement comes from the fact you speak about "plan data"

By plan data there, I mean the plan_workdir a.k.a. TMT_PLAN_DATA, under which the physical TMT_PLAN_ENVIRONMENT_FILE is located.

Hmmm, good point - I don't know the answer. In theory, a plan may put different content into this file on different guests, but do they share the same path on the runner when fetched from guests? This might be a bug.

after a PrepareShell step the plan data is synced back so any changes to TMT_PLAN_ENVIRONMENT_FILE are reflected back to the runner,

they do not share environment from the plan environment file

So when doing guest.pull is there a filter to not pull $TMT_PLAN_DATA/variables.env? There are multiple aspects that seem to be in conflict with each other between:

• TMT_PLAN_DATA being shared across guests
• TMT_PLAN_ENVIRONMENT_FILE=$TMT_PLAN_DATA/variables.env
• Reading back changes to TMT_PLAN_ENVIRONMENT_FILE

Indeed, I'm no longer sure they do not share plan environment file content. Maybe they do, and maybe they should not. Added a TODO item to the PR description.

[LecrisUT]

What about when used in ansible plugin? That one does not include these right? And similarly for all guestless step/plugins right?

[happz]

Correct, to reflect the current state it should be rather "Should apply to user commands executed on the given guest".