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

Redesign "Network Configuration" UI #300

Closed
alinefm opened this issue Apr 5, 2016 · 27 comments
Closed

Redesign "Network Configuration" UI #300

alinefm opened this issue Apr 5, 2016 · 27 comments
Assignees
Labels
Milestone

Comments

@alinefm
Copy link
Member

alinefm commented Apr 5, 2016

There are a few issues in this "Network Configuration" section:

2073af205fb2483b

  1. There are scroll bars when they are not needed.
  2. Why allow multiple selection if no actions is allowed when selecting more than one interface ?
  3. To keep consistence, I'd suggest to have only one button 'Add'.
    When selecting this Add button, a dialog to choose the network type is displayed (similar to what we have when adding a new Template in Kimchi). According to selected type, display the appropriated inputs.

Said that, my suggestion is do change this section to something like below:

| + Add |  | Refresh |                                | Filter                |

| Name      | Type       | Address Space    | MAC Address          |              
| iface 1   | Ethernet   | 1.1.1.1          | 12:34:56:78:90       |  Actions  |
| iface 2   | Vlan       |                  | 12:34:56:78:90       |  Actions  |
| iface 3   | Ethernet   | 2.3.4.6          | 90:34:56:78:90       |  Actions  |

When selecting the Actions menu, the list of available actions will be listed: Down/Up, Restart, Settings, Delete.

All the widgets used for this mockup MUST follow Wok style. The idea of this proposal is to have something similar to what we have on Network tab in Kimchi.

@alinefm
Copy link
Member Author

alinefm commented Apr 8, 2016

More details about the "Add" button behavior can be found at #308

@alinefm
Copy link
Member Author

alinefm commented Apr 15, 2016

The 'Add' button is a feature provided by Ginger s390x. So I will move the issue #308 to the right project.

@potula-chandra
Copy link
Member

@alinefm :
Q. Why multiple selection is enabled if no action can be taken?
Definitely, multiple selection will be enabled and actions can be taken based on the selections. Unfortunately all features are not implemented as per the network design pages been approved by the community.

We had trouble with python-augeas and restricted to one interface earlier. But now with code optimization multiple selection will be possible for performing certain actions.

@samhenri
Copy link
Contributor

samhenri commented May 25, 2016

Hi @chandrureddy

Sorry for the delay. On my environment the multiple selection doesn't work. If I select more than one item, the Actions drop-down is disabled. I think we should at least disable the dropdown list item (adding the class .disabled and checking it with javascript) for the matched actions (If I select only interfaces that are down, then enable the "Up" action and "Restart", if I select multiple interfaces then only Restart is enabled). If this is not possible then I agree with @alinefm mockups, the interface actions button should be moved to each line and the Add and Refresh buttons to the top left of the table.

@potula-chandra
Copy link
Member

Hi Samuel,

These actions are possible with the current implementations. I guess as
part of the issue we can work out the solution and fix these.

Again we intentionally not allowed earlier to avoid core dump issues by
augeas. Now since back end code optimization done we can enable multiple
selection and possible user actions against them.

Regards
Chandra

On 5/25/16 11:52 PM, Samuel wrote:

Hi @chandrureddy https://github.com/chandrureddy

Sorry for the delay. On my environment the multiple selection doesn't
work. If I select more than one item, the Actions drop-down is
disabled. I think we should at least disable the dropdown

(adding the class .disabled and checking it with javascript) for the

matched actions (If I select only interfaces that are down, then
enable the "Up" action and "Restart", if I select multiple interfaces
then only Restart is enabled). If this is not possible then I agree
with @alinefm https://github.com/alinefm mockups, the interface
actions button should be moved to each line and the Add and Refresh
buttons to the top left of the table.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#300 (comment)

@danielhb
Copy link
Contributor

This is the proposed fix in the ML:

image

Note that the 'Add' and 'Refresh' buttons are on the top right side of the table. This is not compliant with the rework being made in the Global Network where, in its first version, put everything on the
top left side:

image

@alinefm @chandrureddy @samhenri we can't allow 2 features at the same table to have such differences in the layout. We need a consensus of where this 'Refresh' and 'Add' buttons are going to be for both Global Network and Network Settings layout.

@samhenri
Copy link
Contributor

@danielhb I believe the current patch sent to the ML was the final patch for Network Configuration using jQuery Bootgrid. Although the title says "redesign "Network Configuration" UI" there's still work to be done. I was waiting this patch in order to work with the DataTables port.

@danielhb
Copy link
Contributor

@samhenri are you saying that the relocation of the 'Add' and 'Refresh' button is going to be made by you in this port to DataTables?

If that's the case then I will relieve the current patch in the ML from doing it.

@samhenri
Copy link
Contributor

I thought that this what was aligned a few scrum meetings ago, right @alinefm @chandrureddy ?

@danielhb
Copy link
Contributor

@samhenri Ok, I'll inform that in the ML

@alinefm
Copy link
Member Author

alinefm commented Jul 11, 2016

Correct, @samhenri and @danielhb

danielhb pushed a commit that referenced this issue Jul 19, 2016
- Disable scroll bars when they are not needed
- Enable Actions button when selecting more than one interface

Required js,css,scss changes in Ginger for above mentioned issue.

v1 : Fix for code review comment(icon is too close to checkbox)
@alinefm alinefm added this to the Ginger 2.3 milestone Jul 22, 2016
@alinefm alinefm closed this as completed Jul 22, 2016
@alinefm alinefm reopened this Jul 22, 2016
@alinefm
Copy link
Member Author

alinefm commented Jul 22, 2016

Sorry, seems more changes are needed to close this issue.

@samhenri
Copy link
Contributor

samhenri commented Aug 5, 2016

I'm currently working on this redesign but I'll keep the "Actions" drop-down since the multi-selection is now working. I think the first button should be the Actions drop-down and then the Add drop-down, like this:

| Actions v |  | + Add v |  | Refresh |                              | Filter                |

| Name      | Type       | Address Space    | RDMA enabled   | MAC Address                   |
| iface 1   | Ethernet   | 1.1.1.1          | No             | 12:34:56:78:90                |
| iface 2   | Vlan       |                  | No             | 12:34:56:78:90                |
| iface 3   | Ethernet   | 2.3.4.6          | No             | 90:34:56:78:90                |

I'm having problems to add a new Bond or Interface, can someone show me a valid PUT request when the user hits the "Save" button? The "Parent Interface" and "Bond Members" selects are empty for me.

@samhenri
Copy link
Contributor

samhenri commented Aug 8, 2016

Hi, I'm unable to Add a Bond and Add a VLAN with the current code. The JSON from cfginterfaces looks like this:

[
  {
    "IPV4_INFO":{
      "PEERROUTES":"yes",
      "IPV4INIT":"yes",
      "DEFROUTE":"yes",
      "PEERDNS":"yes",
      "IPV4_FAILURE_FATAL":"no",
      "IPV4Addresses":[
        {
          "PREFIX":"255.255.255.0",
          "IPADDR":"10.10.71.45",
          "GATEWAY":"10.10.71.1"
        }
      ],
      "BOOTPROTO":"static"
    },
    "BASIC_INFO":{
      "UUID":"6b4d4616-5a39-48a1-b73e-58bef428a1b5",
      "MTU":"1500",
      "HWADDR":"1A:6B:53:EE:BB:29",
      "TYPE":"Ethernet",
      "ONBOOT":"yes",
      "NAME":"ens3"
    },
    "IPV6_INFO":{
      "IPV6INIT":"yes",
      "IPV6_PEERDNS":"yes",
      "IPV6_DEFROUTE":"yes",
      "IPV6_AUTOCONF":"yes",
      "IPV6_FAILURE_FATAL":"no",
      "IPV6_PEERROUTES":"yes"
    }
  }
]

I've checked the JS code and found this:

if ("BASIC_INFO" in interfaceInfo && "DEVICE" in (interfaceInfo.BASIC_INFO)) {
...
}

Is this correct? My CFG file doesn't seem to have the device key in BASIC_INFO.

@danielhb
Copy link
Contributor

danielhb commented Aug 8, 2016

"BASIC_INFO" is a grouping of the following parameters:

* BASIC_INFO: Basic information of network interface:
    * NAME: Name of the interface.
    * DEVICE: Device name of the interface.
    * ONBOOT: 'Yes' if interface is to be brought up during boot,
              'No' if interface should not be brought up during boot.
    * TYPE: Interface type ('nic', 'bonding', 'vlan')
    * MACADDR: Mac address of ethernet device.
    * VLAN: 'Yes' If its vlan device.
    * VLAN_ID: vlan id of the vlan device.
    * PHYSDEV: physical device associated with the vlan device.
    * BONDING_OPTS: Bonding parameters of the Bond device.
    * BONDING_MASTER: 'yes' if its bond device.
    * SLAVES: Returns slaves associated with the bond.
    * NETTYPE(system z): Returns network device driver type
    * SUBCHANNELS(system z):List of sub channels(network triplets) of
                            the interface

@samhenri
Copy link
Contributor

samhenri commented Aug 8, 2016

@danielhb I've updated my comment. The BASIC_INFO object is there but I don't have the DEVICE, so the select / comboboxes for Bonds and VLANS are always empty. Is this correct? Can't I look for the interface name instead of the device name if the DEVICE key is not available or is it a problem with my CFG file?

      if ("BASIC_INFO" in interfaceInfo && "DEVICE" in (interfaceInfo.BASIC_INFO)) {
        interfaceDetails["value"] = interfaceList[i].BASIC_INFO.DEVICE;
        interfaceDetails["text"] = interfaceList[i].BASIC_INFO.DEVICE;
        memberOptions.push(interfaceDetails);
      }

@danielhb
Copy link
Contributor

danielhb commented Aug 8, 2016

If you're running in a Fedora/RHEL you should have the 'DEVICE' attribute in the cfgfile of a network card. It refers to the device as listed by ifconfig.

You can put a workaround in the UI to find for NAME if DEVICE isn't found and see if it works though. In some distros DEVICE and NAME are interchangeable.

@samhenri
Copy link
Contributor

samhenri commented Aug 8, 2016

Yes, i'm running Fedora 23 this time and found this. It seems is happening with some Ubuntu machines but I still need to confirm. I'll make a workaround to use the interface name if device is not available then.

@jay-katta
Copy link
Member

jay-katta commented Aug 9, 2016

Hi @samhenri , as @danielhb said if you are running in Fedora/RHEL you should have DEVICE attribute in ifcfg file of network interface.
As of now cfginterfaces does not support Ubuntu.
To understand more, can you share your operating system and ifcfg content details.

@samhenri
Copy link
Contributor

samhenri commented Aug 16, 2016

Hi @chandrureddy @alinefm @danielhb,, I'm almost finishing the redesigned panel but I have a question regarding the buttons.
Since I've added the _lodash library, I can filter arrays by values so I can check if all the selected items have the same network type, module, etc. For SR-IOV I know I can't display the option with multiple selected items because it opens a modal window and the user has to input the parameters to each network interface.
But for the Delete button, I think I could enable the button if the user selected networks that aren't "nic" type. In this case I would display a wok.confirm() dialog to remove the selected interfaces, including the ones that were selected on the other pages. What do you think?

@danielhb
Copy link
Contributor

On 08/16/2016 06:03 PM, Samuel wrote:

Hi @chandrureddy https://github.com/chandrureddy @alinefm
https://github.com/alinefm @danielhb https://github.com/danielhb,,
I'm almost finishing the redesigned panel but I have a question
regarding the buttons.
Since I've added the _lodash library, I can filter arrays by values so
I can check if all the selected items have the same network type,
module, etc. For SR-IOV I know I can't display the option with
multiple selected items because it opens a modal window and the user
has to input the parameters to each network interface.
But for the Delete button, I think I could enable the button if the
user selected networks that aren't "nic" type. In this case I would
display a wok.confirm() dialog to remove the selected interfaces,
including the ones that were selected on the other pages. What do you
think?

Go for it

Another question, when the current script checks if the selected row
has the the mlx5-core module installed, we have the following line:

|if ((selectedIf[0]["module"] != 'mlx5_core' && selectedIf[0]["module"]
!= 'mlx5-core') || (selectedIf[0]["nic_type"] === 'virtual' )) { ... } |

Is this correct? Why do we have to check for mlx5_core twice? As for
this feature, I can't test it because I'm running on a virtual
environment but I can mock an object and show the option to check if
the modal window is at least working.

Because the module sometimes appears as 'mlx5_core' and in others
'mlx5-core'. This happened
a lot with the former mlx4_core driver.

I'll be honest and say that I am not seeing 'mlx5-core' anymore but it's
better to be on the safe side.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#300 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA053GNwv1p7mlMc9G0WtBq0oDHgtG8aks5qgiW9gaJpZM4IAeaa.

@potula-chandra
Copy link
Member

@samhenri @danielhb in case of platform s390x interfaces of type 'nic' can also be deleted. So we have enable 'Delete' option in case of s390x for the interfaces of type 'nic' too.

@samhenri
Copy link
Contributor

@chandrureddy does the current implementation already checks s390x interfaces?

@potula-chandra
Copy link
Member

potula-chandra commented Aug 19, 2016

@samhenri I guess so. Removal of interfaces of type 'nic' should currently works on s390x.

@samhenri
Copy link
Contributor

I meant, how to check if the user is running a s390x or default ginger with JavaScript?

@potula-chandra
Copy link
Member

I guess recent patch by commit 84f43d5 disables button 'Delete'. But I suppose we have to enable 'Delete' in case of s390x.

May be @atreyeemukhopadhyay can help us here better as she recently done such changes and code handling in case of s390x.

@potula-chandra
Copy link
Member

potula-chandra commented Aug 19, 2016

We can rely on the API call of gingerbase : /plugins/gingerbase/host to know about the architecture. For more details : https://github.com/kimchi-project/gingerbase/blob/master/docs/API.md

In case of s390x you get the following details:

Basic Information
s390x
Architecture
KVM for IBM z Systems
OS Distro
1.1.3
OS Version
Z
OS Code Name
IBM/2827/743 H43
Processor
Online: 2 Offline: 2
Book(s): 4 Socket(s): 6 Core(s) per Socket: 6 Thread(s) per Core: 1
CPU(s)
Shared: 2 Dedicated: 0
Core(s)
Online: 4GiB Offline: 0B
Memory
Name: CSTLIN1 ID: 55
LPAR Details
Name: PR/SM Vendor: IBM
Hypervisor Details

danielhb pushed a commit that referenced this issue Aug 29, 2016
@alinefm alinefm closed this as completed Sep 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants