teuthology: use test user name from the config

[phlogistonjohn]

A few places in the code try to use a configurable name but others effectively hard-code "ubuntu". Fix these cases to support more flexible user naming.

[zmc]

This is a good change; let's just get this tested with .e.g. a single scheduled job in sepia

[phlogistonjohn]

OK, is that something you would like me to do, or something you will take care of?
If me, is there a particular suite or something I should try? I usually just test orch or the smb tests in orch.

[phlogistonjohn]

https://pulpito.ceph.com/phlogistonjohn-2025-07-31_13:58:12-orch-wip-phlogistonjohn-testing-1-2025-07-22-0801-distro-default-gibba/
I performed this run having applied my patch from this PR on top of the teutholgoy main branch - no issues related to my change that I can see, just a known test flake in the smb orch suite.

[kshtsk]

Not sure I understand this, why 'None' does not work, in the context of the change it is the same.
Introducing the _default_user with value '.', instead of, for example, 'ubuntu', overcomplicates the code.

[phlogistonjohn]

I guess the comment doesn't explain the situation enough. The issue at hand is twofold - I do not want to use the username ubuntu on my test systems, I find it confusing as I mainly test on non-ubuntu systems. I have my teuthology configuration use something like cephtest. However, a few places in the code did not honor that existing configuration parameter.

The function canonicalize_hostname does not treat None as meaning "use the default user from the config" it is something more like "don't include a username component in the resulting hostname string".
I did not want to change this behavior because I didn't want to have to find all the existing callers and potentially change them.

I did consider making a sentinel object - like _deafult_user = object() but that would require a change to the type annotation and I thought that'd be ugly. I thought '.' seems like an obvious and safe sentinel value as it's not a real username and is akin to "here" in unix file systems and some other tools.

[kshtsk]

From what I understood, the cannonicalize_hostname takes user name defaults = 'ubuntu', when user is None, it should return hostname without any user "@" suffix.

I don't think we ever used user=None. However you're saying that it would
be still useful to keep this possibility to suppress any user.

I would suggest to drop the comment above, and instead add doctoring for the canonicalize_hostname adding description of what is expected to be done and parameters and return value.

And as soon as we're touching the function signature, could you please add return typing for it, like -> str:

[kshtsk]

And instead of confusing _default_user it would be better to user self descriptive naming. Since _canonicalize_hostname_default_user_sentinel is too long, maybe just use _default_user_sentinel or _user_sentinel.

[kshtsk]

From what I understand about sentinel pattern the default value is just an object(), but I am not strictly against longly . character, it just looks strange, and introduce hidden behavior, in case someone call the function user='.'.

[kshtsk]

The get_test_user already has default value 'ubuntu' for 'test_user' config parameter.

[phlogistonjohn]

@zmc ping

[kshtsk]

I guess the new behavior can be covered by a unit test, like adding a case to TestHostnames by https://github.com/ceph/teuthology/blob/main/teuthology/test/test_misc.py#L252-L313

[kshtsk]

% grep -r "'ubuntu'" teuthology | grep user
teuthology/provision/cloud/test/test_openstack.py:        assert userdata['runcmd'][-2:] == [['passwd', '-d', 'ubuntu'], ['touch', '/.teuth_provisioned']]
teuthology/provision/cloud/openstack.py:        user='ubuntu',
teuthology/provision/cloud/base.py:            conf=None, user='ubuntu',
teuthology/provision/downburst.py:    for user in [little_old_me, 'ubuntu', 'teuthology']:
teuthology/provision/downburst.py:    def __init__(self, name, os_type, os_version, status=None, user='ubuntu',
teuthology/openstack/__init__.py:        self.username = 'ubuntu'
teuthology/misc.py:def canonicalize_hostname(hostname, user: Optional[str] ='ubuntu'):
teuthology/misc.py:    return config.get('test_user', 'ubuntu')

Also more places can be updated to respect test_user, like downburst and libcloud open stack.

[phlogistonjohn]

@zmc as per our call yesterday: I have apparently been doing tests with this patch applied for weeks. I'm doing one more run today with it today after having rebased on teuthology main.

[phlogistonjohn]

Today's test run showed no regressions, FWIW.

[phlogistonjohn]

Also, I'd like to keep the scope of this change fairly small - I only really make use of it in my local teuthology setup. It's one where I don't generally test ubuntu based systems at all and feels weird to create a ubuntu user. I don't really expect to change the sepia lab values and start confusing everyone who is already used to this "quirk". Ultimately, I am just trying to carry fewer custom patches for that setup by up-streaming this. Thanks!

[zmc]

@phlogistonjohn can you link the run here please?

[phlogistonjohn]

https://pulpito.ceph.com/phlogistonjohn-2025-09-18_18:54:56-orch-wip-phlogistonjohn-smb-test-main-distro-default-gibba/

[kshtsk]

Here and after, why do we need a user name for this at all?
I suggest to have only ssh_config here and after:

ssh_config = '~/.ssh/config'
.
.
.
     run.Raw(ssh_config),

[phlogistonjohn]

I just noticed that I'm still carrying this custom patch in my working branch of teuthology. I'd really like to get this PR done and over with but I don't have the time to do much in this space. This fixed my issue and it works for me and it's reasonably minimal.

If I can't submit a minimal change and (let others) iterate and improve things afterward I'll probably have to close the PR (and corresponding issue) as I really have my time occupied by lots of other things. In other words, unless there's something wrong with the change it's a bit of a 'take it or leave it' situation for me. Thanks for understanding.

[phlogistonjohn]

I just rebased and it applied cleanly and the tests passed FWIW.