search

neoave / mrack

Multicloud use-case based multihost async provisioner for CIs and testing during development
Apache License 2.0
12 stars 14 forks source link

fix: prefix job_id with J: for _get_recipe_info #289

Closed skycastlelily closed 7 months ago

skycastlelily commented 7 months ago

fix the tmt clean guest issue: https://github.com/teemtee/tmt/issues/2796

skycastlelily commented 7 months ago

The problem is:delete_host,checks whether host_id.startswith("J:"),while tmt get the job_id from _get_recipe_info ,and recipe['job_id'] doesn't has the J: prefix.

skycastlelily commented 7 months ago

Hi,pvoborni,would you please review this merge request,which is tiny and clear,and related to the merge request:https://github.com/neoave/mrack/pull/286:)

skycastlelily commented 7 months ago

Sorry for the late reply,I'm just back from state holiday vacation.

The problem lies in the line of tmt:https://github.com/teemtee/tmt/blob/main/tmt/steps/provision/mrack.py#L696,create_server dose return a job_id with J:,but the def create function in tmt will return the info from _get_repcipe_info,ie,no J:, so  it will pass a job_id without J" when it calls mrack provider's delete_host,and the error happens.My fault,I thought create_server will return a job_id without J:,will be more careful next time when I submit a merge request.That being said,IMO we could and also should fix the problem from mrack side,as we never know whether other users will do the same thing:use the job_id from _get_recipe_info.So I update the merge request, would please review it,when you get some time to spare?Thanks:)

skycastlelily commented 6 months ago

Sorry for the so late reply, I didn't realize I hadn't sent this out until just now.

it would be better to have the beaker provider more refactored so the logic around the id is not on 3 different places and ideally tmt could avoid using internal method for getting current state.

Yes, it will be great if you could find some time working on that:)

Merged #289 https://github.com/neoave/mrack/pull/289 into main.

Thanks:)

On Mon, Apr 8, 2024 at 6:21 PM Petr Vobornik @.***> wrote:

Merged #289 https://github.com/neoave/mrack/pull/289 into main.

— Reply to this email directly, view it on GitHub https://github.com/neoave/mrack/pull/289#event-12390446473, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKFR23CNO5CWSKQ4KRCJJQ3Y4JVQPAVCNFSM6AAAAABFK64HLSVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJSGM4TANBUGY2DOMY . You are receiving this because you authored the thread.Message ID: @.***>