Re: [Kimchi-devel] [PATCHv2] [Kimchi] Edit Virtual Network with passthrough support

Hi Socorro, On 19-05-2016 19:54, Socorro Stoppler wrote: > v2: > - Fixed bugs encountered while testing > - Added support for macvtap passthrough > - Addressed feedback from Lucio > - UI issues below are still unresolved > - Due to value from backend currently is not showing up when edit > comes up and instead > 'Nothing selected' is showing, this is being handled gracefully > in that if user leaves > the Destination field as is, the value (from backend) is preserved. I'm fine with this solution as definitive. If user changes it, you pass the parameter, otherwise not. Returning the nic in the backend may add confusion, since a bridged connection will be using a bridge interface based on that nic, not the nic itself. More comments below (only about TODOs in the code).

> > v1: > - First attempt of Edit Virtual Network UI > - Known UI Issues:s > - after the update happens, the list of networks are not shown > properly (i.e. rows are all > bunched up and actions buttons are diagonal, but fixes itself > after you move off the tab > and come back to it > - when edit panel first opens up, the value for the Destination > selector (if applicable) is > either showing 'Nothing selected' or it's showing the first item > on the list rather than > the value that came from backend > - Bridged network has not been tested so there's bound to be > issues there > > Proposal: > Per discussion w/Lucio, with regards to vlan_id, I'd like to propose > to have the backend show the vlan_id as a separate field. > This came about when I couldn't find that field and as it turns out, > it was because it was appended to the interface names > > Signed-off-by: Socorro Stoppler <socorro(a)linux.vnet.ibm.com&gt; > --- > ui/js/src/kimchi.api.js | 23 ++++ > ui/js/src/kimchi.network.js | 14 +++ > ui/js/src/kimchi.network_add_main.js | 20 ++-- > ui/js/src/kimchi.network_edit_main.js | 198 > ++++++++++++++++++++++++++++++++++ > ui/pages/network-edit.html.tmpl | 78 ++++++++++++++ > ui/pages/tabs/network.html.tmpl | 3 +- > 6 files changed, 327 insertions(+), 9 deletions(-) > create mode 100644 ui/js/src/kimchi.network_edit_main.js > create mode 100644 ui/pages/network-edit.html.tmpl > > diff --git a/ui/js/src/kimchi.api.js b/ui/js/src/kimchi.api.js > index 83d1cc7..8c01646 100644 > --- a/ui/js/src/kimchi.api.js > +++ b/ui/js/src/kimchi.api.js > @@ -648,6 +648,17 @@ var kimchi = { > }); > }, > > + retrieveNetwork : function(name, suc, err) { > + wok.requestJSON({ > + url : 'plugins/kimchi/networks/' + > encodeURIComponent(name), > + type : 'GET', > + contentType : 'application/json', > + dataType : 'json', > + success: suc, > + error: err > + }); > + }, > + > deleteNetwork : function(name, suc, err) { > wok.requestJSON({ > url : 'plugins/kimchi/networks/' + > encodeURIComponent(name), > @@ -661,6 +672,18 @@ var kimchi = { > }); > }, > > + updateNetwork : function(name, settings, suc, err) { > + wok.requestJSON({ > + url : "plugins/kimchi/networks/" + > encodeURIComponent(name), > + type : 'PUT', > + contentType : 'application/json', > + data : JSON.stringify(settings), > + dataType : 'json', > + success: suc, > + error: err > + }); > + }, > + > listHostPartitions : function(suc, err) { > wok.requestJSON({ > url : 'plugins/kimchi/host/partitions', > diff --git a/ui/js/src/kimchi.network.js b/ui/js/src/kimchi.network.js > index c0100cf..381449d 100644 > --- a/ui/js/src/kimchi.network.js > +++ b/ui/js/src/kimchi.network.js > @@ -91,6 +91,8 @@ kimchi.getNetworkItemHtml = function(network) { > startClass : network.state === "up" ? > "wok-hide-action-item" : "", > stopClass : network.state === "down" ? > "wok-hide-action-item" : disable_in_use, > stopDisabled : network.in_use ? "disabled" : "", > + editClass : network.state === "up" || network.in_use ? > "disabled" : "", > + editDisabled : network.state === "up" || network.in_use ? > "disabled" : "", > deleteClass : network.state === "up" || network.in_use ? > "disabled" : "", > deleteDisabled: network.state === "up" || network.in_use ? > "disabled" : "" > }); > @@ -107,6 +109,8 @@ kimchi.stopNetwork = function(network,menu) { > if (!network.in_use) { > $("[nwAct='delete']", menu).removeClass("disabled"); > $(":first-child", $("[nwAct='delete']", > menu)).removeAttr("disabled"); > + $("[nwAct='edit']", menu).removeClass("disabled"); > + $(":first-child", $("[nwAct='edit']", > menu)).removeAttr("disabled"); > } > $(".network-state", $("#" + > wok.escapeStr(network.name))).switchClass("loading", "down"); > }, function(err) { > @@ -128,6 +132,8 @@ kimchi.addNetworkActions = function(network) { > $("[nwAct='start']", menu).addClass("disabled"); > $("[nwAct='delete']", menu).addClass("disabled"); > $(":first-child", $("[nwAct='delete']", > menu)).attr("disabled", true); > + $("[nwAct='edit']", menu).addClass("disabled"); > + $(":first-child", $("[nwAct='edit']", > menu)).attr("disabled", true); > kimchi.toggleNetwork(network.name, true, function() { > $("[nwAct='start']", > menu).addClass("wok-hide-action-item"); > $("[nwAct='start']", menu).removeClass("disabled"); > @@ -142,8 +148,10 @@ kimchi.addNetworkActions = function(network) { > $("[nwAct='start']", menu).removeClass("disabled"); > if (!network.in_use) { > $("[nwAct='delete']", > menu).removeClass("disabled"); > + $("[nwAct='edit']", menu).removeClass("disabled"); > } > $(":first-child", $("[nwAct='delete']", > menu)).removeAttr("disabled"); > + $(":first-child", $("[nwAct='edit']", > menu)).removeAttr("disabled"); > wok.message.error(err.responseJSON.reason); > }); > } else if ($(evt.currentTarget).attr("nwAct") === "stop") { > @@ -180,6 +188,12 @@ kimchi.addNetworkActions = function(network) { > $('#networkGrid').dataGrid('deleteRow', > $(evt.currentTarget).parents(".wok-datagrid-row")); > }); > }, null); > + } else if ($(evt.currentTarget).attr("nwAct") === "edit") { > + if (network.state === "up" || network.in_use) { > + return false; > + } > + kimchi.selectedNetwork = network.name; > + wok.window.open('plugins/kimchi/network-edit.html'); > } > }); > > diff --git a/ui/js/src/kimchi.network_add_main.js > b/ui/js/src/kimchi.network_add_main.js > index 3090aeb..a500cf8 100644 > --- a/ui/js/src/kimchi.network_add_main.js > +++ b/ui/js/src/kimchi.network_add_main.js > @@ -56,7 +56,7 @@ kimchi.startNetworkCreation = function() { > }; > > kimchi.openNetworkDialog = function(okCallback) { > - kimchi.loadInterfaces(); > + kimchi.loadInterfaces(undefined, false); > > $("#networkFormOk").on("click", function() { > $("#networkFormOk").button("disable"); > @@ -73,10 +73,12 @@ kimchi.openNetworkDialog = function(okCallback) { > }); > }; > > -kimchi.setDefaultNetworkType = function(isInterfaceAvail) { > +kimchi.setDefaultNetworkType = function(isInterfaceAvail, bEdit) { > $("#networkType").selectpicker(); > if (!isInterfaceAvail) { > - kimchi.enableBridgeOptions(false); > + if (!bEdit) { > + kimchi.enableBridgeOptions(false); > + } > $("#networkBriDisabledLabel").removeClass('hidden'); > } else { > $("#networkBriDisabledLabel").remove(); > @@ -128,9 +130,9 @@ kimchi.setupNetworkFormEvent = function() { > } > } > $('#networkDestinationID').selectpicker('destroy'); > - kimchi.loadInterfaces(new Array("nic", "bonding")); > + kimchi.loadInterfaces(new Array("nic", "bonding"), false); > } else { > - kimchi.loadInterfaces(); > + kimchi.loadInterfaces(undefined, false); > } > }); > > @@ -175,7 +177,7 @@ kimchi.enableBridgeOptions = function(enable, > networkType, networkDestinationTyp > }; > }; > > -kimchi.loadInterfaces = function(interfaceFilterArray) { > +kimchi.loadInterfaces = function(interfaceFilterArray, bEdit) { > > var loadInterfacesHTML = function(result) { > var options = []; > @@ -200,8 +202,10 @@ kimchi.loadInterfaces = > function(interfaceFilterArray) { > } > $selectDestination.append(selectDestinationOptionHTML); > $('#networkDestinationID').selectpicker('refresh'); > - kimchi.setDefaultNetworkType(result.length!==0); > - kimchi.changeNetworkDestination(); > + kimchi.setDefaultNetworkType(result.length!==0, bEdit); > + if (!bEdit) { > + kimchi.changeNetworkDestination(); > + } > }; > > var networkType = $("#networkType").val(); > diff --git a/ui/js/src/kimchi.network_edit_main.js > b/ui/js/src/kimchi.network_edit_main.js > new file mode 100644 > index 0000000..761d2f4 > --- /dev/null > +++ b/ui/js/src/kimchi.network_edit_main.js > @@ -0,0 +1,198 @@ > +/* > + * Project Kimchi > + * > + * Copyright IBM Corp, 2016 > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +kimchi.network_edit_main = function() { > + var initNetwork = function(network) { > + var networkType = network['connection']; > + $('#bridgedContent').hide(); > + $('#networkType').val(networkType); > + $('#networkName').val(kimchi.selectedNetwork); > + > + var subnetValue = network['subnet']; > + if (subnetValue === "") { > + $('#networkSubnetRange').val("unavailable"); > + } else { > + $('#networkSubnetRange').val(subnetValue); > + } > + > + // Default to hide Subnet > + $('#subnetRange').hide(); > + > + if(networkType === "nat" || networkType === "isolated") { > + //Show subnet/dhcp range > + $('#subnetRange').show(); > + } > + > + if (networkType === kimchi.NETWORK_TYPE_MACVTAP || > + networkType === kimchi.NETWORK_TYPE_PASSTHROUGH || > + networkType === kimchi.NETWORK_TYPE_VEPA || > + networkType === kimchi.NETWORK_TYPE_BRIDGED) { > + $('#bridgedContent').show(); > + $('#networkDestination').show(); > + $('#vlan').hide(); > + if (networkType === kimchi.NETWORK_TYPE_BRIDGED) { > + //Now check if there's a vlan id and only show if > one exists > + var netInterface = network['interfaces']; > + var netInterfaceParts = netInterface[0].split('-', 2); > + var netvlanID = netInterfaceParts[1]; > + if (netvlanID !== undefined) { > + //Show vlan ID field; do not show the checkbox > + $('#vlan').show(); > + $('#vlan_chkbox').hide(); > + $('#vlan-enabled').show(); > + $('#networkVlanID').val(netvlanID); > + } > + } > + } > + > + kimchi.setupNetworkFormEventForEdit(network); > + > + //TODO: Trial and error trying to get select box to show > show the one that it's currently set to but it doesn't work > + var netInterface = network['interfaces']; > + $('#networkDestinationID').append('val', netInterface); > + $('#networkDestinationID').val(netInterface); > + $("#networkDestinationID > option[value='+netInterface+']").prop('selected', true); > + $('#networkDestinationID').selectpicker('refresh'); Is this code block to be removed? > + }; > + > + kimchi.retrieveNetwork(kimchi.selectedNetwork, initNetwork); > + > + var generalSubmit = function(event) { > + $('#networkFormOk').text(i18n['KCHAPI6010M']); > + $('#networkFormOk').prop('disabled', true); > + > + var data = $('#networkConfig').serializeObject(); > + kimchi.updateNetworkValues(); > + }; > + > + $('#networkConfig').on('submit', generalSubmit); > + $('#networkFormOk').on('click', generalSubmit); > +}; > + > +kimchi.setupNetworkFormEventForEdit = function(network) { > + var selectedType = network['connection']; > + if (selectedType === kimchi.NETWORK_TYPE_BRIDGED) { > + if (kimchi.capabilities && kimchi.capabilities.nm_running) { > + wok.message.warn(i18n['KCHNET6001W'],'#alert-modal-container'); > + } > + } > + > + // Network name validation > + $("#networkName").on("keyup", function(event) { > + $("#networkName").toggleClass("invalid-field", > !$("#networkName").val().match(/^[^\"\/]+$/)); > + kimchi.updateNetworkFormButtonForEdit(); > + }); > + > + var selectedType = network['connection']; > + if(selectedType === kimchi.NETWORK_TYPE_MACVTAP || > + selectedType === kimchi.NETWORK_TYPE_PASSTHROUGH || > + selectedType === kimchi.NETWORK_TYPE_VEPA) { > + if (selectedType === kimchi.NETWORK_TYPE_MACVTAP) { > + $('#networkDestinationID').attr('multiple', > false).data('liveSearch',false); > + } else { > + $('#networkDestinationID').attr('multiple', true); > + if($('#networkDestinationID option').length > 10 ) { > + $('#networkDestinationID').data('liveSearch',true); > + } > + } > + $('#networkDestinationID').selectpicker('destroy'); > + > + kimchi.loadInterfaces(new Array("nic", "bonding"), true); > + } else { > + kimchi.loadInterfaces(undefined, true); > + } > + //TODO: Need to confirm if this is still needed > + var netInterface = network['interfaces']; > + $('#networkDestinationID').append('val', netInterface); > + $('#networkDestinationID').val(netInterface); > + $("#networkDestinationID > option[value='+netInterface+']").prop('selected', true); > + $('#networkDestinationID').selectpicker('refresh'); Is this still needed? > +}; > + > +kimchi.updateNetworkFormButtonForEdit = function() { > + if ($("#networkName").hasClass("invalid-field")) { > + $('#networkFormOk').prop('disabled', true); > + } else{ > + $('#networkFormOk').prop('disabled', false); > + } > +}; > + > +kimchi.getNetworkDialogValuesForEdit = function() { > + var network = { > + name : $("#networkName").val(), > + type : $("#networkType").val(), > + subnetRange: $("#networkSubnetRange").val(), > + interface: $("#networkDestinationID").val(), > + vlan_id: $("#networkVlanID").val() > + }; > + if (network.type === kimchi.NETWORK_TYPE_BRIDGED) { > + if (network.vlan_id !== "") { > + network.vlan_id = parseInt($("#networkVlanID").val()); > + } > + } > + return network; > +}; > + > +kimchi.updateNetworkValues = function() { > + kimchi.retrieveNetwork(kimchi.selectedNetwork, function(settings) { > + var network = kimchi.getNetworkDialogValuesForEdit(); > + var data = { > + name : network.name, > + subnet: network.subnetRange, > + interfaces: [ network.interface ], > + vlan_id: network.vlan_id > + }; > + var originalDest = settings.interfaces; > + var updatedDest = $('#networkDestinationID').val(); > + if (originalDest === updatedDest || updatedDest === null) { > + delete data['interfaces']; > + } > + if (network.type !== "nat" && network.type !== "isolated") { > + delete data['subnet']; > + } else { // either nat or isolated > + delete data['interfaces']; > + delete data['vlan_id']; > + } > + if (network.type === kimchi.NETWORK_TYPE_BRIDGED) { > + if (data.vlan_id === "") { > + delete data['vlan_id']; > + } else { > + data.vlan_id = parseInt($("#networkVlanID").val()); > + } > + } else { > + delete data['vlan_id']; > + } > + > + // Just like in Add Network, VEPA connection - > network.interface - is already an array > + if (network.type === kimchi.NETWORK_TYPE_VEPA || > network.type === kimchi.NETWORK_TYPE_PASSTHROUGH) { > + if (network.interface !== null) { > + data.interfaces = network.interface; > + } > + } > + > + kimchi.updateNetwork(kimchi.selectedNetwork, data, function() { > + $("#networkBody").empty(); > + kimchi.initNetworkListView(); > + wok.window.close(); > + }, function(settings) { > + > wok.message.error(settings.responseJSON.reason,'#alert-modal-container'); > + $('#networkFormOk').text(i18n['KCHAPI6007M']); > + $('#networkFormOk').prop('disabled', false); > + }); > + }); > +}; > diff --git a/ui/pages/network-edit.html.tmpl > b/ui/pages/network-edit.html.tmpl > new file mode 100644 > index 0000000..4cc7232 > --- /dev/null > +++ b/ui/pages/network-edit.html.tmpl > @@ -0,0 +1,78 @@ > +#* > + * Project Kimchi > + * > + * Copyright IBM Corp, 2016 > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or > implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + *# > +#unicode UTF-8 > +#import gettext > +#from wok.cachebust import href > +#silent t = gettext.translation($lang.domain, $lang.localedir, > languages=$lang.lang, fallback=True) > +#silent _ = t.gettext > +#silent _t = t.gettext > +<html> > +<body> > +<div id="edit-network-window" class="window modal-content"> > + <div class="modal-header"> > + <h4 class="modal-title" id="networkModalLabel">$_("Edit > Network")</h4> > + </div> > + <div id="networkConfig" class="modal-body"> > + <span id="alert-modal-container"></span> > + <div class="form-group"> > + <label for="networkType">$_("Network Type")</label> > + <input type="text" class="form-control" id="networkType" > disabled="disabled" /> > + </div> > + <div class="form-group"> > + <label for="networkName">$_("Network Name")</label> > + <input type="text" class="form-control" name="netName" > id="networkName" /> > + <p class="help-block"> > + <i class="fa fa-info-circle"></i> $_("Name should > not contain '/' and '\"'.")</p> > + </div> > + <div id="subnetRange" class="form-group" "col-md-3"> > + <div class="form-group"> > + <label for="networkSubnetRange">$_("Address > Space")</label> > + <input type="text" class="form-control" > id="networkSubnetRange" /> > + </div> > + </div> > + <div id="networkBriDisabledLabel" class="form-group hidden"> > + <p>$_("(No interfaces found)")</p> > + </div> > + <div id="bridgedContent"> > + <div id="networkDestination" class="form-group"> > + <label > for="networkDestinationID">$_("Destination")</label> > + <select id="networkDestinationID" data-size="5" > class="selectpicker col-md-12 col-lg-12 form-control"> > + </select> > + </div> > + <div class="form-group" id="vlan"> > + <div id="vlan_chkbox"> > + <input id="enableVlan" class="wok-checkbox" > type="checkbox" value="" /> > + <label for="enableVlan" > id="labelEnableVlan">$_("Enable VLAN") </label> > + </div> > + <div id="vlan-enabled"> > + <label for="networkVlanID" > id="labelNetworkVlanID">$_("VLAN ID"): </label> > + <input type="text" id="networkVlanID" > class="form-control" /> > + </div> > + </div> > + </div> > + </div> > + <div class="modal-footer"> > + <button type="submit" id="networkFormOk" class="btn > btn-default">$_("Save")</button> > + <button type="button" id="networkFormCancel" > data-dismiss="modal" class="btn btn-default">$_("Cancel")</button> > + </div> > +</div> > +<script type="text/javascript"> > +kimchi.network_edit_main(); > +</script> > +</body> > +</html> > diff --git a/ui/pages/tabs/network.html.tmpl > b/ui/pages/tabs/network.html.tmpl > index d8660d3..6ddabaa 100644 > --- a/ui/pages/tabs/network.html.tmpl > +++ b/ui/pages/tabs/network.html.tmpl > @@ -99,6 +99,7 @@ > <ul class="dropdown-menu" > role="menu"> > <li role="presentation" > nwAct="start" class='{startClass}'><a><i class="fa > fa-undo"></i>$_("Start")</a></li> > <li role="presentation" > nwAct="stop" class='{stopClass}'><a {stopDisabled}><i class="fa > fa-ban"></i>$_("Stop")</a></li> > + <li role="presentation" > nwAct="edit" class='{editClass}'><a {editDisabled}><i class="fa > fa-pencil"></i>$_("Edit")</a></li> > <li role="presentation" > nwAct="delete" class='critical {deleteClass}'><a {deleteDisabled}><i > class="fa fa-minus-circle"></i>$_("Delete")</a></li> > </ul> > </div> > @@ -110,4 +111,4 @@ > kimchi.initNetwork(); > </script> > </body> > -</html> > \ No newline at end of file > +</html> >