Disable password authentication when using keys#2695
Conversation
|
My hypothesis for this is here: #2687 (comment) |
|
/packit test |
psss
left a comment
There was a problem hiding this comment.
Thanks for digging into the issue! The change looks safe. Just a minor proposal to make things a bit more clear for the future.
02d117a to
0190c69
Compare
0190c69 to
c5aed09
Compare
|
So if I use 'password' option (which we have in the spec - https://tmt.readthedocs.io/en/stable/plugins/provision.html#id9) the problem will appear again? Isn't there a way for testcloud to have the info that cloud-init finished its job? (and will allow tmt to try to connect only after that?) |
Yeah, I am thinking here if tmt shouldn't internally always use a ssh key for the connections to the testcloud guests and take the user:password from the spec as something that users can use to connect to guests manually outside of tmt. From testcloud's perspective, if the image contains a working cloud-init (which are currently the only ones the testcloud supports, I have long rotting code for serial console fun for images without cloud-init, but that's another story), that should be all that's needed, and if some tests require to be ran under a different user, That said, I may be missing use-cases and/or reasons from the wider tmt perspective why user:password has to work too apart from just the ssh keys even for the master connection.
I've been thinking about this too while working on the issue. There may be ways to do this (but if we get to this route, I'd still prefer to have this PR merged as it's a painkiller for the default setup):
Probably apart from the first in the list, these would still leave a (albeit fractionally smaller, but still) surface of race, as both would be the last step of the cloud-init's runcmd, while the ssh restart happens in a non-atomic way after this phase. |
|
Hm, so it seems that the |
c5aed09 to
61c2cd2
Compare
|
So we ran ten instances of the test and all are green :-P |
|
/packit test -i provision |
6d0b334 to
1a82ef3
Compare
|
@frantisekz, so... it seems, that even if the fix is removed, everything is green 😁 |
😂 hehe, and that's how it is with races... I'd still say let's merge this change, it'll decrease the surface for the race for the default (key) auth method. And to kill it completely, I'll come with the proposed "Change the default ssh port from the guest perspective in the cloud-init's runcmd, and setup mapping for the different port, so unchanged ssh port is invisible for the tmt " mid-term (it'll require some api design work testcloud side if it were to be implemented properly). |
1a82ef3 to
c10685e
Compare
c10685e to
db39ff9
Compare
Yeah, makes sense. And we've also found why everything was green (wrong provision method variable). |
|
Here's a failure from a recent job. |
Fixes #2687
Pull Request Checklist