teemtee / tmt

Test Management Tool
MIT License
83 stars 124 forks source link

$TMT_TOPOLOGY_BASH should be set for 'login' option #2193

Open cmackows opened 1 year ago

cmackows commented 1 year ago

$TMT_TOPOLOGY_BASH is not set when using 'login' option (for virtual guest).

[root@driver tree]# env | grep TMT
TMT_PLAN_DATA=/var/tmp/tmt/run-010/sts/plan/data
TMT_TREE=/var/tmp/tmt/run-010/sts/plan/tree

$TMT_TOPOLOGY_BASH is set for prepare steps

23:31:22 [driver (sts_driver)]                        cmd: set -eo pipefail; env
23:31:22 [driver (sts_driver)]                    environment: None
23:31:22 [driver (sts_driver)]                        err: Warning: Permanently added '192.168.122.47' (ED25519) to the list of known hosts.
23:31:22 [driver (sts_driver)]                        out: SHELL=/bin/bash
23:31:22 [driver (sts_driver)]                        out: PWD=/var/tmp/tmt/run-010/sts/plan/tree
23:31:22 [driver (sts_driver)]                        out: TMT_TOPOLOGY_YAML=/var/tmp/tmt/run-010/sts/plan/tree/tmt-test-topology-check_env-driver.yaml
23:31:22 [driver (sts_driver)]                        out: TMT_TOPOLOGY_BASH=/var/tmp/tmt/run-010/sts/plan/tree/tmt-test-topology-check_env-driver.sh
23:31:22 [driver (sts_driver)]                        out: USER=root
23:31:22 [driver (sts_driver)]                        out: TMT_TREE=/var/tmp/tmt/run-010/sts/plan/tree

log.txt

idorax commented 1 year ago

The root cause is dict environment is not updated before invoking Login._login(). If we look into ExecuteInternal.execute(), we can see the code in the following which updates environment,

Hence, we should add the similar code before invoking Login._login().

Hi @cmackows, would you please help to explain why the environment vars like TMT_TOPOLOGY_* are required if user runs tmt run login?

happz commented 1 year ago

The root cause is dict environment is not updated before invoking Login._login(). If we look into ExecuteInternal.execute(), we can see the code in the following which updates environment,

Hence, we should add the similar code before invoking Login._login().

Agreed. Feel free to propose a patch, I started working on this but got to solving unrelated issues hence no usable PR in my stash.

Hi @cmackows, would you please help to explain why the environment vars like TMT_TOPOLOGY_* are required if user runs tmt run login?

My 2 cents: why wouldn’t they be exposed? :) I think of this as “test-like environment” which should be consistent across prepare, execute, finish and other places where tmt runs user code. As another example, ansible prepare plugin should also expose them, via `—-extra-vars’ too.

cmackows commented 1 year ago
  • Hi @cmackows https://github.com/cmackows, would you please help to explain why the environment vars like TMTTOPOLOGY* are required if user runs tmt run login?

My 2 cents: why wouldn’t they be exposed? :) I think of this as “test-like environment” which should be consistent across prepare, execute, finish and other places where tmt runs user code. As another example, ansible prepare plugin should also expose them, via `—-extra-vars’ too.

Same as Miloš has pointed out. It leads to confusion with $TMT_TOPOLOGY_BASH not being set in login environment. -Chris

— Reply to this email directly, view it on GitHub https://github.com/teemtee/tmt/issues/2193#issuecomment-1634336751, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYTPEVHURY2HR45PGZGMSATXQAACDANCNFSM6AAAAAA2CKFSLA . You are receiving this because you were mentioned.Message ID: @.***>