Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lock.ops.unlock_one: Fail sooner on 403, with msg #1850

Merged
merged 1 commit into from
May 26, 2023
Merged

Conversation

zmc
Copy link
Member

@zmc zmc commented May 24, 2023

In the case of e.g. owners values not matching on an unlock attempt, we were exhausting all retries and failing to display the exact reason for the unlock failure. We can simply break on 403 errors and let the rest of the function do its thing.

In the case of e.g. owners values not matching on an unlock attempt, we
were exhausting all retries and failing to display the exact reason for
the unlock failure. We can simply break on 403 errors and let the rest
of the function do its thing.

Signed-off-by: Zack Cerza <zack@redhat.com>
@zmc zmc requested a review from batrick May 24, 2023 19:46
@zmc
Copy link
Member Author

zmc commented May 24, 2023

Here's a CLI session demonstrating what changed:

(virtualenv) zack@teuthology:~/teuthology$ time teuthology-lock -v --unlock smithi111
Traceback (most recent call last):
  File "/home/zack/teuthology/virtualenv/bin/teuthology-lock", line 8, in <module>
    sys.exit(main())
  File "/home/zack/teuthology/scripts/lock.py", line 18, in main
    sys.exit(teuthology.lock.cli.main(parse_args(sys.argv[1:])))
  File "/home/zack/teuthology/teuthology/lock/cli.py", line 203, in main
    if not ops.unlock_one(ctx, machine, user):
  File "/home/zack/teuthology/teuthology/lock/ops.py", line 209, in unlock_one
    while proceed():
  File "/home/zack/teuthology/teuthology/contextutil.py", line 135, in __call__
    raise MaxWhileTries(error_msg)
teuthology.exceptions.MaxWhileTries: 'unlock smithi111.front.sepia.ceph.com' reached maximum tries (10) after waiting for 32.5 seconds

real	0m29.106s
user	0m0.861s
sys	0m0.081s
(virtualenv) zack@teuthology:~/teuthology$ git checkout unmask-unlock-response
Switched to branch 'unmask-unlock-response'
Your branch is up to date with 'origin/unmask-unlock-response'.
(virtualenv) zack@teuthology:~/teuthology$ time teuthology-lock -v --unlock smithi111
2023-05-24 19:43:02,643.643 ERROR:teuthology.lock.ops:failed to unlock smithi111.front.sepia.ceph.com. reason: Cannot unlock - locked_by values must match

real	0m0.803s
user	0m0.703s
sys	0m0.054s
(virtualenv) zack@teuthology:~/teuthology$ teuthology-lock --list smithi111
[
    {
        "name": "smithi111.front.sepia.ceph.com",
        "description": "/home/teuthworker/archive/vshankar-2023-04-25_04:09:41-fs-wip-vshankar-testing-20230420.132447-testing-default-smithi/7250722",
        "up": true,
        "machine_type": "smithi",
        "is_vm": false,
        "vm_host": null,
        "os_type": "ubuntu",
        "os_version": "22.04",
        "arch": "x86_64",
        "locked": true,
        "locked_since": "2023-04-25 05:05:37.015600",
        "locked_by": "scheduled_vshankar@teuthology",
        "mac_address": null,
        "ssh_pub_key": "ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBCocR9CBppEnpWQB0VbC3HGh+Oxne8nlLd5DXnCCTD+aQgvw+qsMgkwA6Q47jJLbnLaj8Jb3mRl3Y14TXqlxnv0="
    }
]
(virtualenv) zack@teuthology:~/teuthology$ teuthology-lock -v --unlock smithi111 --owner scheduled_vshankar@teuthology
2023-05-24 19:43:57,950.950 INFO:teuthology.lock.ops:unlocked: smithi111.front.sepia.ceph.com

Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay \o/

@zmc zmc merged commit fa97ddd into main May 26, 2023
@zmc zmc deleted the unmask-unlock-response branch May 26, 2023 18:23
@dmick
Copy link
Member

dmick commented May 27, 2023

nice. this has bugged me for a while and I never dug in.

@zmc
Copy link
Member Author

zmc commented Aug 30, 2023

This was https://tracker.ceph.com/issues/58011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants