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

Retry for network changes on localhost #512

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Oct 26, 2023

Done

  • requests to alter networks on localhost will fail, add retry logic for those cases. The problem only occurs when talking directly to lxd (without dev server) on localhost via https://localhost:8443/ui/

QA

  1. Run the LXD-UI:
    • On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @lorumic or @edlerd for access.
    • With a local copy of this branch, run as described here.
  2. Perform the following QA steps:
    • check network editing/renaming/creation/deletion
    • the error can only be triggered when talking directly to LXD on localhost (without the dev server), maybe not worth to find a way to set this up, we can verify the fix from the nightly LXD build after it is merged.

@webteam-app
Copy link

Demo starting at https://lxd-ui-512.demos.haus

Copy link
Contributor

@lorumic lorumic left a comment

Choose a reason for hiding this comment

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

The fix makes sense, and I expect it to work even if I cannot test it, but it's a somewhat ugly workaround. Do we have an idea of what is the root cause of this problem? How was this found? Might be worth discussing it with the core team?

src/api/networks.tsx Show resolved Hide resolved
Copy link
Contributor

@lorumic lorumic left a comment

Choose a reason for hiding this comment

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

I will approve, but I still suggest to consider what I wrote in the previous comment:

The fix makes sense, and I expect it to work even if I cannot test it, but it's a somewhat ugly workaround. Do we have an idea of what is the root cause of this problem? How was this found? Might be worth discussing it with the core team?

@edlerd
Copy link
Collaborator Author

edlerd commented Oct 27, 2023

I will approve, but I still suggest to consider what I wrote in the previous comment:

The fix makes sense, and I expect it to work even if I cannot test it, but it's a somewhat ugly workaround. Do we have an idea of what is the root cause of this problem? How was this found? Might be worth discussing it with the core team?

Opened an issue canonical/lxd#12475

@lorumic
Copy link
Contributor

lorumic commented Oct 27, 2023

Opened an issue canonical/lxd#12475

Well done, thanks!

@edlerd edlerd merged commit 22e4ecc into canonical:main Nov 13, 2023
5 checks passed
@edlerd edlerd deleted the network-ops-retry branch November 13, 2023 11:22
@edlerd
Copy link
Collaborator Author

edlerd commented Nov 13, 2023

Merged as upstream has a hard time finding a fix and we want the UI to be without this bug.

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

Successfully merging this pull request may close these issues.

3 participants