Hi Lucio,
Thank you for your feedback :)
-Socorro
On 05/16/2016 12:48 PM, Lucio Correia wrote:
Hi Socorro,
Remember to fix this before sending final version:
warning: 4 lines add whitespace errors.
My observations testing the UI:
1) I'm unable to rename a macvtap, vepa or bridged network, because
"Destination" shows "Nothing selected" and there is no other option
there, probably because the only option is in use by this own macvtap
network.
In general, maybe if there are no entries being returned for the
Destination, I should disable the Save button.
2) Updating bridged networks, including VLAN_ID, is working fine. :)
I'm glad
to hear that bridged works as that was the one I wasn't able
to test.
Also, I've found a bug in the backend and am sending a fix to it.
Please use it in your tests.
I'll wait for your fix and address your comments
below in the meantime.
Other code suggestions inline below.
On 11-05-2016 21:55, Socorro Stoppler wrote:
> ---
> ui/js/src/kimchi.api.js | 23 ++++
> ui/js/src/kimchi.network.js | 14 +++
> ui/js/src/kimchi.network_edit_main.js | 216
> ++++++++++++++++++++++++++++++++++
> ui/pages/network-edit.html.tmpl | 75 ++++++++++++
> ui/pages/tabs/network.html.tmpl | 3 +-
> 5 files changed, 330 insertions(+), 1 deletion(-)
> 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 3a94631..25dc397 100644
> --- a/ui/js/src/kimchi.api.js
> +++ b/ui/js/src/kimchi.api.js
> @@ -634,6 +634,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),
> @@ -647,6 +658,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 d362010..9816646 100644
> --- a/ui/js/src/kimchi.network.js
> +++ b/ui/js/src/kimchi.network.js
> @@ -90,6 +90,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" : ""
> });
> @@ -106,6 +108,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) {
> @@ -127,6 +131,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");
> @@ -141,8 +147,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") {
> @@ -179,6 +187,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_edit_main.js
> b/ui/js/src/kimchi.network_edit_main.js
> new file mode 100644
> index 0000000..24ae0c5
> --- /dev/null
> +++ b/ui/js/src/kimchi.network_edit_main.js
> @@ -0,0 +1,216 @@
> +/*
> + * 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_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');
> + };
> +
> + kimchi.retrieveNetwork(kimchi.selectedNetwork, initNetwork);
> +
> + var generalSubmit = function(event) {
> + $('#networkFormOk').prop('disabled', true);
> + var data = $('#networkConfig').serializeObject();
> + kimchi.updateNetworkValues();
> + };
> +
> + $('#networkConfig').on('submit', generalSubmit);
> + $('#networkFormOk').on('click', generalSubmit);
> +
> +};
> +
> +kimchi.setupNetworkFormEventForEdit = function(network) {
> + //Not sure if this is needed since not much testing on bridged
> network has been done
> + //if (kimchi.capabilities && kimchi.capabilities.nm_running) {
> + //
> wok.message.warn(i18n['KCHNET6001W'],'#alert-modal-container');
> + //}
> +
> + // Netowrk name validation
> + $("#networkName").on("keyup", function(event) {
> + $("#networkName").toggleClass("invalid-field",
> !$("#networkName").val().match(/^[^\"\/]+$/));
> + kimchi.updateNetworkFormButtonForEdit();
> + });
> +
This block:
> + var selectedType = network['connection'];
> + if(selectedType === kimchi.NETWORK_TYPE_MACVTAP || selectedType
> === kimchi.NETWORK_TYPE_VEPA) {
> + if (selectedType === kimchi.NETWORK_TYPE_VEPA){
> + $('#networkDestinationID').attr('multiple', true);
> + if($('#networkDestinationID option').length > 10 ) {
> + $('#networkDestinationID').data('liveSearch',true);
> + }
> + }
> + else {
> + $('#networkDestinationID').attr('multiple',
> false).data('liveSearch',false);
> + }
> + $('#networkDestinationID').selectpicker('destroy');
> +
> + kimchi.loadInterfacesForEdit(new Array("nic",
"bonding"));
> + } else {
> + kimchi.loadInterfacesForEdit();
> + }
is duplicated from network_add js and perhaps could be made a function
for both network_add and network_edit if we make loadInterfaces
function also generic (see comment below on loadInterfacesForEdit).
Does it make sense?
> +
> + //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');
> +};
> +
> +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) {
> + //TODO: More checking may need to be done here
> + if (network.vlan_id !== "") {
> + network.vlan_id = parseInt($("#networkVlanID").val());
> + }
> + }
> + return network;
> +};
> +
> +kimchi.updateNetworkValues = function() {
> + var network = kimchi.getNetworkDialogValuesForEdit();
> + var settings = {
> + name : network.name,
> + subnet: network.subnetRange,
> + interfaces: [ network.interface ],
> + vlan_id: network.vlan_id
> + };
> +
> + if (network.type !== "nat" && network.type !==
"isolated") {
> + delete settings['subnet'];
> + } else { // either nat or isolated
> + delete settings['interfaces'];
> + delete settings['vlan_id'];
> + }
> +
> + if (network.type === kimchi.NETWORK_TYPE_BRIDGED) {
> + if (settings.vlan_id === "") {
> + delete settings['vlan_id'];
> + } else {
> + settings.vlan_id = parseInt($("#networkVlanID").val());
> + }
> + } else {
> + delete settings['vlan_id'];
> + }
> +
> + // Just like in Add Network, VEPA connection - network.interface
> - is already an array
> + if (network.type === kimchi.NETWORK_TYPE_VEPA) {
> + settings.interfaces = network.interface;
> + }
> +
> + kimchi.updateNetwork(kimchi.selectedNetwork, settings, function() {
> + $("#networkBody").empty();
> + kimchi.initNetworkListView();
> + wok.window.close();
> + }, function(settings) {
> +
> wok.message.error(settings.responseJSON.reason,'#alert-modal-container');
> + $('#networkFormOk').prop('disabled', false);
> + });
> +};
> +
> +kimchi.loadInterfacesForEdit = function(interfaceFilterArray) {
This function seems to be duplicated code from create_network. Is it
possible to use that one here or adapt it to work in both cases?
> + kimchi.getInterfaces(function(result) {
> + var options = [];
> + $selectDestination = $('#networkDestinationID');
> + $selectDestination.empty();
> + var nics = {};
> + $('#networkDestinationID').find('option').remove();
> + var selectDestinationOptionHTML = '';
> + for (var i = 0; i < result.length; i++) {
> + if (typeof interfaceFilterArray === 'undefined') {
> + options.push({label:result[i].name,value:result[i].name});
> + nics[result[i].name] = result[i];
> + selectDestinationOptionHTML += '<option
> data-type="'+ result[i].type +'" value="'+ result[i].name
+ '">' +
> result[i].name + '</option>';
> + } else {
> + for (var k = 0; k < interfaceFilterArray.length; k++) {
> + if (result[i].type == interfaceFilterArray[k]) {
> + options.push({label:result[i].name,value:result[i].name});
> + nics[result[i].name] = result[i];
> + selectDestinationOptionHTML += '<option
> data-type="'+ result[i].type +'" value="'+ result[i].name
+ '">' +
> result[i].name + '</option>';
> + }
> + }
> + }
> + }
> +
> + $selectDestination.append(selectDestinationOptionHTML);
> + $('#networkDestinationID').selectpicker('refresh');
> + });
> +
> +};
> diff --git a/ui/pages/network-edit.html.tmpl
> b/ui/pages/network-edit.html.tmpl
> new file mode 100644
> index 0000000..3f2f3fe
> --- /dev/null
> +++ b/ui/pages/network-edit.html.tmpl
> @@ -0,0 +1,75 @@
> +#*
> + * 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="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>
>