Merge lp:~trapnine/maas/fix-1562107-ip-fail-feedback-1.9 into lp:maas/1.9

[Jeffrey C Jones]

Display error messages when saving interface links.

Backport of revision 5113 from trunk.

Display error messages when saving interface links.

[Mike Pontillo (mpontillo)]

Was this fix already on trunk? If so, which revision? If not, has fix that aligns better with the 2.0 code already landed?

review: Needs Information

[Andres Rodriguez (andreserl)]

This needs to be blocked on
https://code.launchpad.net/~trapnine/maas/fix-1562107-ip-fail-feedback-1.9/+
merge/297274

On Tue, Jun 14, 2016 at 10:14 AM, Mike Pontillo <<email address hidden>
> wrote:

> Review: Needs Information
>
> Was this fix already on trunk? If so, which revision? If not, has fix that
> aligns better with the 2.0 code already landed?
> --
>
> https://code.launchpad.net/~trapnine/maas/fix-1562107-ip-fail-feedback-1.9/+merge/297274
> Your team MAAS Committers is subscribed to branch lp:maas/1.9.
>

--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

[Jeffrey C Jones (trapnine)]

Approved - backport of https://code.launchpad.net/~trapnine/maas/fix-1562107-ip-fail-feedback-1.9/+
merge/297274.

review: Approve

[Jeffrey C Jones (trapnine)]

Rather https://code.launchpad.net/~trapnine/maas/fix-1562107-ip-fail-feedback/+merge/297268

[MAAS Lander (maas-lander)]

The attempt to merge lp:~trapnine/maas/fix-1562107-ip-fail-feedback-1.9 into lp:maas/1.9 failed. Below is the output from the failed tests.

Ign http://prodstack-zone-1.clouds.archive.ubuntu.com trusty InRelease
Get:1 http://security.ubuntu.com trusty-security InRelease [65.9 kB]
Get:2 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates InRelease [65.9 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports InRelease
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release.gpg
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty Release
Get:3 http://security.ubuntu.com trusty-security/main Sources [117 kB]
Get:4 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main Sources [277 kB]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [37.3 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [493 kB]
Get:7 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted Sources [5,352 B]
Get:8 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe Sources [156 kB]
Get:9 http://security.ubuntu.com trusty-security/universe amd64 Packages [130 kB]
Get:10 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse Sources [5,939 B]
Get:11 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [781 kB]
Get:12 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/restricted amd64 Packages [15.9 kB]
Get:13 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [361 kB]
Get:14 http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-updates/multiverse amd64 Packages [13.2 kB]
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty-backports/multiverse amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse Sources
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/restricted amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://prodstack-zone-1.clouds.archive.ubuntu.com trusty/multiverse amd64 Packages
Fetched 2,523 kB in 1s (1,364 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-...

[Mike Pontillo (mpontillo)]

Looks good, now that I know the fix already landed in trunk. Updated the commit message accordingly.

[Mike Pontillo (mpontillo)]

review: Approve

=== modified file 'src/maasserver/static/js/angular/controllers/node_details_networking.js'
--- src/maasserver/static/js/angular/controllers/node_details_networking.js	2015-11-09 17:37:16 +0000
+++ src/maasserver/static/js/angular/controllers/node_details_networking.js	2016-06-14 16:17:43 +0000
@@ -175,6 +175,7 @@
         $scope.subnets = SubnetsManager.getItems();
         $scope.interfaces = [];
         $scope.interfaceLinksMap = {};
+        $scope.interfaceErrorsByLinkId = {};
         $scope.originalInterfaces = {};
         $scope.showingMembers = [];
         $scope.focusInterface = null;
@@ -649,6 +650,13 @@
             return !angular.isObject(nic.subnet);
         };
 
+        $scope.getInterfaceError = function(nic) {
+            if(angular.isDefined(nic.link_id) && nic.link_id >= 0) {
+                return $scope.interfaceErrorsByLinkId[nic.link_id];
+            }
+            return null;
+        };
+
         // Called when the link mode for this interface and link has been
         // changed.
         $scope.saveInterfaceLink = function(nic) {
@@ -660,17 +668,17 @@
             }
             if(angular.isDefined(nic.link_id) && nic.link_id >= 0) {
                 params.link_id = nic.link_id;
+                delete $scope.interfaceErrorsByLinkId[nic.link_id];
             }
             if(nic.mode === LINK_MODE.STATIC && nic.ip_address.length > 0) {
                 params.ip_address = nic.ip_address;
             }
             return NodesManager.linkSubnet($scope.node, nic.id, params).then(
                 null, function(error) {
-                    // XXX blake_r: Just log the error in the console, but
-                    // we need to expose this as a better message to the
-                    // user.
                     console.log(error);
-
+                    if(angular.isDefined(nic.link_id) && nic.link_id >= 0) {
+                        $scope.interfaceErrorsByLinkId[nic.link_id] = error;
+                    }
                     // Update the interfaces so it is back to the way it
                     // was before the user changed it.
                     updateInterfaces();

=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js'
--- src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js	2015-11-09 17:37:16 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js	2016-06-14 16:17:43 +0000
@@ -1432,6 +1432,30 @@
                     "ip_address": nic.ip_address
                 });
         });
+
+        it("handles errors", function() {
+            var controller = makeController();
+            var nic = {
+                id: makeInteger(0, 100),
+                mode: "static",
+                subnet: { id: makeInteger(0, 100) },
+                link_id: makeInteger(0, 100),
+                ip_address: "192.168.122.1"
+            };
+            var defer = $q.defer();
+            spyOn(console, "log");
+            spyOn(NodesManager, "linkSubnet").and.returnValue(
+                defer.promise);
+            $scope.saveInterfaceLink(nic);
+            defer.reject("error");
+            $scope.$digest();
+            // Make sure error is set for UI.
+            expect($scope.interfaceErrorsByLinkId[nic.link_id]).toBe("error");
+            expect(console.log).toHaveBeenCalledWith("error");
+            // Make sure error is cleared on success.
+            $scope.saveInterfaceLink(nic);
+            expect($scope.interfaceErrorsByLinkId[nic.link_id]).toBeUndefined();
+        });
     });
 
     describe("subnetChanged", function() {

=== modified file 'src/maasserver/static/partials/node-details.html'
--- src/maasserver/static/partials/node-details.html	2015-12-03 17:09:21 +0000
+++ src/maasserver/static/partials/node-details.html	2016-06-14 16:17:43 +0000
@@ -462,7 +462,7 @@
                                                 data-ng-options="mode.mode as mode.text for mode in modes | filterLinkModes:interface">
                                             </select>
                                         </li>
-                                        <li class="margin-top--ten" data-ng-show="shouldShowIPAddress(interface)">
+                                        <li class="margin-top--ten" data-ng-if="shouldShowIPAddress(interface)">
                                             <input type="text" class="table__input no-margin-top"
                                                 data-ng-model="interface.ip_address"
                                                 data-ng-class="{ invalid: isIPAddressInvalid(interface) }"
@@ -470,6 +470,9 @@
                                                 data-ng-focus="setFocusInterface(interface)"
                                                 data-ng-blur="clearFocusInterface(interface)"
                                                 data-ng-disabled="interface.mode != 'static'">
+                                            <div class="table__data left" data-ng-if="getInterfaceError(interface)">
+                                                <span class="icon error"></span> {$ getInterfaceError(interface) $}
+                                            </div>
                                         </li>
                                     </ul>
                                     <span class="ng-hide" data-ng-show="isAllNetworkingDisabled() && !interface.discovered[0].ip_address">