Merge ~steverydz/maas:fix-static-ip-and-subnet-assignment into maas:master

[Steve Rydz]

LP: #1806099 - Make sure static IP is assigned when static mode rather than auto assigned

LP: #1806112 - Send subnet ID when creating a bridge or a bond in machine interfaces

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix-static-ip-and-subnet-assignment lp:~steverydz/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 53d1296654513b3d3bc73e9356a18d4fee488365

review: Approve

[Mike Pontillo (mpontillo)]

I think the code in this branch is fine, but I think we should discuss the larger picture. Your previous merge proposal stated that back-end changes would be required to get this to work. But I think the reason that setting the IP mode and subnet doesn't work is because it was designed that way for a good reason, and we should discuss further before we change that.

That is, if I am creating a bridge, it should inherit the IP assignment from the interface I created it on. Similarly, if I create a bond, it should inherit the IP assignments from its parent. (The only difference: AUTO and DHCP assignments should be combined, and if a combination of AUTO, DHCP, and Static appear on bond parents, we should prefer static, auto, and DHCP in that order.)

Right now it's not doing that it all. In fact, instead MAAS tends to *remove* all the IP addresses rather than even trying to make a good guess.

***

With that, here are some important questions about the UX that we need to answer:

Should the user be allowed to configure the IP mode at bond or bridge creation time?

If the answer is "yes", what subset of the configuration of the currently-existing interfaces should be shown by default, so that the UI is consistent with the back-end? (We can use the algorithm I described above, but what about edge cases such as multiple static IP address assignments?)

If the answer is "no", we just need to remove IP assignment from the "add" screen and make sure that when bonds or bridges are added, MAAS does something that is predictable and unsurprising. Then we could just replace the IP assignment editing boxes that appear upon bond or bridge creation with some text that states what MAAS will do with existing IP assignments.

(Additionally, when a bond or bridge is deleted, we should probably take any IP assignments and place them on the first parent interface.)

***

I tested this with the API as well; given a machine with system_id 'ta3rxy' and two NICs (IDs 6 and 8), the minimum to create a bond is something like the following:

    maas admin interfaces create-bond ta3rxy parents=6 parents=10 name=bond0

(It fails without the `name=bond0` part.)

The documentation states that leaving out the VLAN results in "disconnected", but that turns out to not be true. In MAAS 2.5, leaving the VLAN out results in the bond being connected to the same VLAN as its parent.

Just as is the case with the UI, creating the bond results in no IP addresses being transferred to the new bond.

Similarly, this is the minimum command to create a bridge:

    maas admin interfaces create-bridge ta3rxy parents=6 name=br0

As with bonds, no IP address assignments are transferred to the new bridge.

review: Needs Information

[Martin Storey (cassiocassio)]

Questions, clarifiactions and a proposal:

https://images.zenhubusercontent.com/59391bc151aa2c305780dbd6/25aa7114-fbca-4b5e-92ef-f51003a44eb7

https://zpl.io/VOKkQEX

[Blake Rouse (blake-rouse)]

Working fix is here: https://code.launchpad.net/~blake-rouse/maas/+git/maas/+merge/364781

review: Disapprove

diff --git a/src/maasserver/static/js/angular/controllers/node_details_networking.js b/src/maasserver/static/js/angular/controllers/node_details_networking.js
index b4c2eac..06dd9fa 100644
--- a/src/maasserver/static/js/angular/controllers/node_details_networking.js
+++ b/src/maasserver/static/js/angular/controllers/node_details_networking.js
@@ -33,6 +33,7 @@ angular.module('MAAS').filter('filterByUnusedForInterface', function() {
 });
 
 
+
 // Filter that is specific to the NodeNetworkingController. Filters the
 // list of interfaces to not include the current parent interfaces being
 // bonded together.
@@ -1525,7 +1526,8 @@ angular.module('MAAS').controller('NodeNetworkingController', [
                 vlan: vlan_id,
                 bond_mode: $scope.newBondInterface.bond_mode,
                 bond_lacp_rate: $scope.newBondInterface.lacpRate,
-                bond_xmit_hash_policy: $scope.newBondInterface.xmitHashPolicy
+                bond_xmit_hash_policy: $scope.newBondInterface.xmitHashPolicy,
+                subnet: $scope.newBondInterface.subnet.id
             };
             $scope.$parent.nodesManager.createBondInterface(
                 $scope.node, params).then(null, function(error) {
@@ -1668,7 +1670,8 @@ angular.module('MAAS').controller('NodeNetworkingController', [
                 vlan: vlan.id,
                 fabric: fabric.id,
                 bridge_stp: $scope.newBridgeInterface.bridge_stp,
-                bridge_fd: $scope.newBridgeInterface.bridge_fd
+                bridge_fd: $scope.newBridgeInterface.bridge_fd,
+                subnet: $scope.newBridgeInterface.subnet.id
             };
             $scope.$parent.nodesManager.createBridgeInterface(
                 $scope.node, params).then(null, function(error) {
@@ -1757,7 +1760,8 @@ angular.module('MAAS').controller('NodeNetworkingController', [
                         function(tag) { return tag.text; }),
                     mac_address: $scope.newInterface.mac_address,
                     vlan: $scope.newInterface.vlan.id,
-                    mode: $scope.newInterface.mode
+                    mode: $scope.newInterface.mode,
+                    ip_address: $scope.newInterface.ip_address
                 };
             }
             if(angular.isObject($scope.newInterface.subnet)) {
diff --git a/src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js b/src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js
index 85d1678..fd4be12 100644
--- a/src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js
+++ b/src/maasserver/static/js/angular/controllers/tests/test_node_details_networking.js
@@ -3754,7 +3754,8 @@ describe("NodeNetworkingController", function() {
                     vlan: vlan.id,
                     bond_mode: "active-backup",
                     bond_lacp_rate: "fast",
-                    bond_xmit_hash_policy: "layer2"
+                    bond_xmit_hash_policy: "layer2",
+                    subnet: undefined
                 });
             expect($scope.interfaces).toEqual([]);
             expect($scope.newBondInterface).toEqual({});
@@ -3802,7 +3803,8 @@ describe("NodeNetworkingController", function() {
                     vlan: null,
                     bond_mode: "active-backup",
                     bond_lacp_rate: "fast",
-                    bond_xmit_hash_policy: "layer2"
+                    bond_xmit_hash_policy: "layer2",
+                    subnet: undefined
                 });
             expect($scope.interfaces).toEqual([]);
             expect($scope.newBondInterface).toEqual({});
@@ -4033,6 +4035,7 @@ describe("NodeNetworkingController", function() {
                 vlan: vlan,
                 fabric: fabric
             };
+
             $scope.interfaces = [nic1];
             $scope.interfaceLinksMap = {};
             $scope.interfaceLinksMap[nic1.id] = {};
@@ -4045,6 +4048,7 @@ describe("NodeNetworkingController", function() {
             spyOn($scope, "cannotAddBridge").and.returnValue(false);
             $scope.newBridgeInterface.name = "br0";
             $scope.newBridgeInterface.mac_address = "00:11:22:33:44:55";
+            $scope.newBridgeInterface.subnet = {};
             $scope.addBridge();
 
             expect(MachinesManager.createBridgeInterface).toHaveBeenCalledWith(
@@ -4056,7 +4060,8 @@ describe("NodeNetworkingController", function() {
                     vlan: vlan.id,
                     fabric: fabric.id,
                     bridge_stp: false,
-                    bridge_fd: 15
+                    bridge_fd: 15,
+                    subnet: undefined
                 });
             expect($scope.interfaces).toEqual([]);
             expect($scope.newBridgeInterface).toEqual({});
@@ -4296,7 +4301,8 @@ describe("NodeNetworkingController", function() {
                         tags: [],
                         vlan: vlan.id,
                         subnet: subnet.id,
-                        mode: "auto"
+                        mode: "auto",
+                        ip_address: undefined
                     });
             expect($scope.newInterface).toEqual({});
             expect($scope.selectedMode).toBeNull();