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

fix: Silent failure of DockerAgent's push_image(), pull_image() #2572

Merged

Conversation

jopemachine
Copy link
Member

@jopemachine jopemachine commented Jul 26, 2024

This PR modifies the behavior of the DockerAgent so that when an error occurs during the execution of push_image() or pull_image(), it does not fail silently but instead informs the user that an issue has occurred.

Test

Tested manually.
For example, when incorrect Docker credentials are used, it will raise an error like below.

❯ ./backend.ai session convert-to-image 7a6b642b-22f4-42fd-b6fb-bbdf2a818ba3 new-img
∙ Request to commit Session(name or id: 7a6b642b-22f4-42fd-b6fb-bbdf2a818ba3)
 33%|███████████████████████████████████████████████████████████                                                                                                                      | 1/3 [00:02<00:04,  2.01s/it]
✗ Error during the operation: <BackendError(-1): General Backend API Error. (RuntimeError("Failed to push image: unauthorized: HTTP Basic: Access denied. If a password was provided for Git authentication, the password was incorrect or you're required to use a token instead of a password. If a token was provided, it was either incorrect, expired, or improperly scoped. See https://gitlab.com/help/user/profile/account/two_factor_authentication.md#troubleshooting"))>

Checklist: (if applicable)

  • Milestone metadata specifying the target backport version
  • Mention to the original issue
  • Test case(s) to:
    • Demonstrate the difference of before/after
    • Demonstrate the flow of abstract/conceptual models with a concrete implementation

Copy link
Member Author

jopemachine commented Jul 26, 2024

@github-actions github-actions bot added comp:agent Related to Agent component comp:common Related to Common component size:S 10~30 LoC labels Jul 26, 2024
@jopemachine jopemachine changed the title fix: Silent failure of DockerAgent.push_image fix: Silent failure of DockerAgent.push_image() Jul 26, 2024
@jopemachine jopemachine force-pushed the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch from 38e91ed to 5de8fef Compare July 27, 2024 03:11
@jopemachine jopemachine force-pushed the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch from 5de8fef to d2492e5 Compare July 27, 2024 03:13
@jopemachine jopemachine changed the base branch from main to topic/07-27-fix_silent_shutdown_on_bgtask_failure July 27, 2024 03:13
@jopemachine jopemachine force-pushed the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch 4 times, most recently from f095d3a to d2e8827 Compare July 29, 2024 04:45
@jopemachine
Copy link
Member Author

In addition to modifying the push_image code, it appears that other aiodocker API calls, such as pull_image, also need to be revised.

@jopemachine jopemachine added the type:bug Reports about that are not working label Aug 3, 2024
Base automatically changed from topic/07-27-fix_silent_shutdown_on_bgtask_failure to main October 24, 2024 05:13
@jopemachine jopemachine force-pushed the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch 2 times, most recently from 2dafcb8 to e6ca06b Compare October 30, 2024 02:40
@jopemachine jopemachine changed the title fix: Silent failure of DockerAgent.push_image() fix: Silent failure of DockerAgent's push_image(), pull_image() Oct 30, 2024
@jopemachine jopemachine modified the milestones: 23.09, 24.03 Oct 30, 2024
@jopemachine
Copy link
Member Author

In this PR, I found several incorrect return types in aiodocker, which led me to create the PR below.
aio-libs/aiodocker#909.

Until the above PR is merged, I’ll manually cast these types to list and leave TODO comments as reminders.

@jopemachine jopemachine added action:on hold Hold it. Wait for the restart. area:docs Documentations and removed action:on hold Hold it. Wait for the restart. labels Oct 30, 2024
@jopemachine jopemachine marked this pull request as ready for review October 30, 2024 09:42
@jopemachine jopemachine requested a review from achimnol October 30, 2024 09:43
@jopemachine jopemachine force-pushed the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch from 0384696 to d0a5159 Compare October 30, 2024 09:43
@jopemachine jopemachine force-pushed the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch from d0a5159 to cfd9bed Compare November 12, 2024 05:48
Copy link
Collaborator

@HyeockJinKim HyeockJinKim left a comment

Choose a reason for hiding this comment

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

lgtm

@kyujin-cho kyujin-cho added this pull request to the merge queue Nov 28, 2024
Merged via the queue into main with commit e66bb24 Nov 28, 2024
21 checks passed
@kyujin-cho kyujin-cho deleted the topic/07-26-fix_silent_failure_of_dockeragent.push_image branch November 28, 2024 11:12
lablup-octodog pushed a commit that referenced this pull request Jan 7, 2025
…#2572)

Backported-from: main (24.12)
Backported-to: 24.09
Backport-of: 2572
lablup-octodog pushed a commit that referenced this pull request Jan 7, 2025
…#2572)

Backported-from: main (24.12)
Backported-to: 24.03
Backport-of: 2572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:agent Related to Agent component comp:common Related to Common component size:S 10~30 LoC type:bug Reports about that are not working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Silent failure of DockerAgent.push_image() due to absense of exception handling
3 participants