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

Improve networking #529

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Improve networking #529

merged 1 commit into from
Nov 17, 2023

Conversation

edlerd
Copy link
Collaborator

@edlerd edlerd commented Nov 13, 2023

Done

  • address various issues around network creation/editing discovered during in person review. Notes are in this doc
  • remove fan network type (simplifies bridge network handling)
  • streamline copy on network forms
  • fix ovn uplink network list to respect restricted.networks.uplinks setting for project, when that is unset, the non-ovn networks from the default project should be displayed in the list.

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:
    • created a bridge network
    • edit a bridge network
    • if ovn available, create / edit an ovn network
    • if ovn available, setup a project with restricted.networks.uplinks set. Enter some network names from the default project into project configuration > Allow custom restrictions on a project level > Restrictions > Networks > Network uplinks. Only networks explicitly listed should be available as uplink, when creating an ovn network in that project.

@webteam-app
Copy link

Demo starting at https://lxd-ui-529.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.

Please make the "Uplink" field a required one for OVN:

Peek 2023-11-14 13-54

@edlerd
Copy link
Collaborator Author

edlerd commented Nov 14, 2023

Please make the "Uplink" field a required one for OVN:

It can be empty, when no networks are available to the current user, the default network from the default profile will then be used. So changed it to be required, when at least 1 network to select from is available.

@lorumic
Copy link
Contributor

lorumic commented Nov 14, 2023

It can be empty, when no networks are available to the current user, the default network from the default profile will then be used. So changed it to be required, when at least 1 network to select from is available.

Can we make it disable the "Create" button, just like the "Name" field does when you input a name that is already assigned to another network?

Peek 2023-11-14 14-36

@edlerd edlerd force-pushed the improve-networking branch 2 times, most recently from df6f542 to 7bec0e5 Compare November 14, 2023 15:05
@edlerd
Copy link
Collaborator Author

edlerd commented Nov 14, 2023

Can we make it disable the "Create" button, just like the "Name" field does when you input a name that is already assigned to another network?

Disabling the button now. Also the selectable networks as uplinks are now properly filtered.

Added some docs for how to test OVN in the wiki.

@edlerd edlerd requested a review from lorumic November 14, 2023 16:13
@lorumic
Copy link
Contributor

lorumic commented Nov 15, 2023

I still have problems creating the OVN network. I get the "Option "network" value "UPLINK" is not one of the allowed uplink networks in project" error, even though I added it (and it is indeed correctly shown in the dropdown). See the screen capture below:

Peek 2023-11-15 09-53

@edlerd
Copy link
Collaborator Author

edlerd commented Nov 15, 2023

I still have problems creating the OVN network. I get the "Option "network" value "UPLINK" is not one of the allowed uplink networks in project" error, even though I added it (and it is indeed correctly shown in the dropdown). See the screen capture below:

I don't really know why this is happening, I suspect a wrong error message from the backend. With the OVN setup described in the wiki I can create OVN networks through the UI.

Edit: Regarding the error, can you either ask in the LXD-UI mattermost, or add it to the next sync agenda so we can figure this out? I won't see it as a blocker for this PR though?

@lorumic
Copy link
Contributor

lorumic commented Nov 15, 2023

Thanks, with microcloud it works. Probably I misconfigured something when I enabled OVN in the other cluster.

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.

Looks good overall, just a couple of small suggestions.

src/pages/networks/forms/NetworkFormBridge.tsx Outdated Show resolved Hide resolved
src/pages/networks/forms/NetworkSelector.tsx Outdated Show resolved Hide resolved
…fix ovn uplink list to respect restricted.networks.uplinks setting for project

Update src/pages/networks/forms/NetworkFormBridge.tsx

Co-authored-by: Michele Lo Russo <michele.lorusso@canonical.com>
@edlerd edlerd merged commit 34b2ffe into canonical:main Nov 17, 2023
5 checks passed
@edlerd edlerd deleted the improve-networking branch November 17, 2023 21:58
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