From 4a55bdb4a3123b1bb42ec18b66943ff2550abed5 Mon Sep 17 00:00:00 2001 From: Caleb Ellis Date: Thu, 21 May 2020 14:48:47 +1000 Subject: [PATCH] Improve composePreProcess readability + fix tests. --- legacy/src/app/controllers/pod_details.js | 56 +++++++++----- .../app/controllers/tests/test_pod_details.js | 73 +++++++++---------- .../directives/tests/test_pod_parameters.js | 14 +++- legacy/src/app/partials/pod-details.html | 6 +- 4 files changed, 89 insertions(+), 60 deletions(-) diff --git a/legacy/src/app/controllers/pod_details.js b/legacy/src/app/controllers/pod_details.js index c2a0afe122b..94ac00901ae 100644 --- a/legacy/src/app/controllers/pod_details.js +++ b/legacy/src/app/controllers/pod_details.js @@ -74,12 +74,12 @@ function PodDetailsController( sentence: "compose" }, obj: { - cores: 0, - memory: 0, + cores: "", + memory: "", storage: [ { type: "local", - size: 0, + size: "", tags: [], pool: {}, boot: true @@ -282,7 +282,12 @@ function PodDetailsController( // Validate storage pool requests const requests = compose.obj.requests; for (let i = 0; i < requests.length; i++) { - if (!requests[i].size || requests[i].size > requests[i].available) { + if ( + requests[i].size && + (isNaN(requests[i].size) || + Number(requests[i].size) <= 0 || + Number(requests[i].size) > Number(requests[i].available)) + ) { return false; } } @@ -409,11 +414,21 @@ function PodDetailsController( }; // Called before the compose params is sent over the websocket. - $scope.composePreProcess = function(params) { - params = angular.copy(params); - params.id = $scope.pod.id; + $scope.composePreProcess = (formValues) => { + const { compose, pod } = $scope; + const params = { + architecture: formValues.architecture, + cores: formValues.cores || $scope.getDefaultComposeValue("cores"), + domain: formValues.domain, + hostname: formValues.hostname, + id: pod.id, + memory: formValues.memory || $scope.getDefaultComposeValue("memory"), + pool: formValues.pool, + zone: formValues.zone, + }; + // Sort boot disk first. - var sorted = $scope.compose.obj.storage.sort(function(a, b) { + const sortedDisks = compose.obj.storage.sort((a, b) => { if (a.boot === b.boot) { return 0; } else if (a.boot && !b.boot) { @@ -422,13 +437,15 @@ function PodDetailsController( return 1; } }); + // Create the storage constraint. - var storage = []; - angular.forEach(sorted, function(disk, idx) { - var constraint = idx + ":" + disk.size; - var tags = disk.tags.map(function(tag) { - return tag.text; - }); + // :([,...]) + // e.g. "0:8(tag1, tag2)" + const storage = []; + sortedDisks.forEach((disk, i) => { + const diskSize = disk.size || $scope.getDefaultComposeValue("storage"); + let constraint = i + ":" + diskSize; + const tags = disk.tags.map((tag) => tag.text); if ($scope.pod.type === "rsd") { tags.splice(0, 0, disk.type); } else { @@ -441,6 +458,7 @@ function PodDetailsController( // Create the interface constraint. // :=[,=];... + // e.g. "eth0:ip=192.168.0.0,subnet_cidr=192.168.0.0/24" const interfaceConstraints = $scope.compose.obj.interfaces .filter((iface) => iface.ipaddress || iface.subnet) .map((iface) => { @@ -664,13 +682,15 @@ function PodDetailsController( $scope.getDefaultComposeValue = (param) => { if ($scope.pod) { - const podPowerType = $scope.power_types.find((type) => type.name === $scope.pod.type); + const podPowerType = $scope.power_types.find( + (type) => type.name === $scope.pod.type + ); if (podPowerType) { - const { defaults } = podPowerType; - return defaults[param] || 0; + const { defaults } = podPowerType; + return defaults[param] || ""; } } - return 0; + return ""; }; // Start watching key fields. diff --git a/legacy/src/app/controllers/tests/test_pod_details.js b/legacy/src/app/controllers/tests/test_pod_details.js index 8f87b5cca37..5b1b8f0e992 100644 --- a/legacy/src/app/controllers/tests/test_pod_details.js +++ b/legacy/src/app/controllers/tests/test_pod_details.js @@ -58,6 +58,7 @@ describe("PodDetailsController", function() { function makePod() { var pod = { id: podId++, + type: "lxd", default_pool: 0, $selected: false, capabilities: [], @@ -77,6 +78,10 @@ describe("PodDetailsController", function() { return pod; } + function defaultPowerTypes() { + return [{ name: "lxd", defaults: { cores: 1, memory: 2048, storage: 8 } }]; + } + // Create the pod that will be used and set the stateParams. var pod, $stateParams; beforeEach(function() { @@ -175,7 +180,7 @@ describe("PodDetailsController", function() { storage: [ { type: "local", - size: 8, + size: "", tags: [], pool: {}, boot: true @@ -575,6 +580,7 @@ describe("PodDetailsController", function() { it("correctly validates hostname", () => { makeController(); $scope.pod = makePod(); + $scope.power_types = defaultPowerTypes(); $scope.compose.obj.hostname = "!@#"; expect($scope.validateMachineCompose()).toBe(false); $scope.compose.obj.hostname = ""; @@ -586,17 +592,19 @@ describe("PodDetailsController", function() { it("correctly validates storage requests", () => { makeController(); $scope.pod = makePod(); - $scope.compose.obj.requests = [{ size: 8, available: 4 }]; + $scope.power_types = defaultPowerTypes(); + $scope.compose.obj.requests = [{ size: "8", available: 4 }]; expect($scope.validateMachineCompose()).toBe(false); - $scope.compose.obj.requests = [{ size: 0, available: 8 }]; + $scope.compose.obj.requests = [{ size: "0", available: 8 }]; expect($scope.validateMachineCompose()).toBe(false); - $scope.compose.obj.requests = [{ size: 8, available: 16 }]; + $scope.compose.obj.requests = [{ size: "8", available: 16 }]; expect($scope.validateMachineCompose()).toBe(true); }); it("correctly validates cpu request without overcommit", () => { makeController(); $scope.pod = makePod(); + $scope.power_types = defaultPowerTypes(); $scope.pod.total.cores = 8; $scope.compose.obj.cores = 10; expect($scope.validateMachineCompose()).toBe(false); @@ -615,6 +623,7 @@ describe("PodDetailsController", function() { it("correctly validates cpu request with overcommit", () => { makeController(); $scope.pod = makePod(); + $scope.power_types = defaultPowerTypes(); $scope.pod.total.cores = 8; $scope.pod.cpu_over_commit_ratio = 2.0; $scope.compose.obj.cores = 20; @@ -628,6 +637,7 @@ describe("PodDetailsController", function() { it("correctly validates memory request without overcommit", () => { makeController(); $scope.pod = makePod(); + $scope.power_types = defaultPowerTypes(); $scope.pod.total.memory_gb = 8; $scope.compose.obj.memory = 9000; expect($scope.validateMachineCompose()).toBe(false); @@ -646,6 +656,7 @@ describe("PodDetailsController", function() { it("correctly validates memory request with overcommit", () => { makeController(); $scope.pod = makePod(); + $scope.power_types = defaultPowerTypes(); $scope.pod.total.memory_gb = 8; $scope.pod.memory_over_commit_ratio = 2.0; $scope.compose.obj.memory = 18000; @@ -702,11 +713,7 @@ describe("PodDetailsController", function() { it("sets id to pod id", function() { makeControllerResolveSetActiveItem(); $scope.pod.type = "rsd"; - expect($scope.composePreProcess({})).toEqual({ - id: $scope.pod.id, - storage: "0:8(local)", - interfaces: "" - }); + expect($scope.composePreProcess({}).id).toEqual($scope.pod.id); }); it("sets rsd storage based on compose.obj.storage", function() { @@ -750,12 +757,9 @@ describe("PodDetailsController", function() { boot: false } ]; - expect($scope.composePreProcess({})).toEqual({ - id: $scope.pod.id, - storage: - "0:50(local,happy,days)," + "1:20(iscsi,one,two),2:60(local,other)", - interfaces: "" - }); + expect($scope.composePreProcess({}).storage).toEqual( + "0:50(local,happy,days)," + "1:20(iscsi,one,two),2:60(local,other)" + ); }); it("correctly sets the interface constraints", function() { @@ -781,11 +785,9 @@ describe("PodDetailsController", function() { "eth1:ip=192.168.1.5,subnet_cidr=192.168.1.0/24", "eth2:subnet_cidr=192.168.2.0/24" ].join(";"); - expect($scope.composePreProcess({})).toEqual({ - id: $scope.pod.id, - storage: "0:8()", - interfaces: expectedInterfaces - }); + expect($scope.composePreProcess({}).interfaces).toEqual( + expectedInterfaces + ); }); it("sets virsh storage based on compose.obj.storage", function() { @@ -835,12 +837,9 @@ describe("PodDetailsController", function() { boot: false } ]; - expect($scope.composePreProcess({})).toEqual({ - id: $scope.pod.id, - storage: - "0:50(pool2,happy,days)," + "1:20(pool1,one,two),2:60(pool3,other)", - interfaces: "" - }); + expect($scope.composePreProcess({}).storage).toEqual( + "0:50(pool2,happy,days)," + "1:20(pool1,one,two),2:60(pool3,other)" + ); }); it("sets virsh storage based on compose.obj.storage", function() { @@ -890,12 +889,9 @@ describe("PodDetailsController", function() { boot: false } ]; - expect($scope.composePreProcess({})).toEqual({ - id: $scope.pod.id, - storage: - "0:50(pool2,happy,days)," + "1:20(pool1,one,two),2:60(pool3,other)", - interfaces: "" - }); + expect($scope.composePreProcess({}).storage).toEqual( + "0:50(pool2,happy,days)," + "1:20(pool1,one,two),2:60(pool3,other)" + ); }); }); @@ -913,7 +909,7 @@ describe("PodDetailsController", function() { storage: [ { type: "local", - size: 8, + size: "", tags: [], pool: {}, boot: true @@ -938,7 +934,7 @@ describe("PodDetailsController", function() { expect($scope.compose.obj.storage.length).toBe(2); expect($scope.compose.obj.storage[1]).toEqual({ type: "local", - size: 8, + size: "", tags: [], pool: {}, boot: false @@ -947,6 +943,7 @@ describe("PodDetailsController", function() { it("adds a new iscsi storage item", function() { makeControllerResolveSetActiveItem(); + $scope.power_types = defaultPowerTypes(); $scope.pod.capabilities.push("iscsi_storage"); expect($scope.compose.obj.storage.length).toBe(1); $scope.composeAddStorage(); @@ -1043,16 +1040,16 @@ describe("PodDetailsController", function() { }); }); - describe("getComposeDefaultValue", () => { + describe("getDefaultComposeValue", () => { it("correctly returns default compose values for pod type", () => { makeControllerResolveSetActiveItem(); $scope.pod = { type: "lxd" }; $scope.power_types = [ { name: "lxd", defaults: { cores: 1, memory: 2048, storage: 8 } }, ]; - expect($scope.getComposeDefaultValue("cores")).toBe(1); - expect($scope.getComposeDefaultValue("memory")).toBe(2048); - expect($scope.getComposeDefaultValue("storage")).toBe(8); + expect($scope.getDefaultComposeValue("cores")).toBe(1); + expect($scope.getDefaultComposeValue("memory")).toBe(2048); + expect($scope.getDefaultComposeValue("storage")).toBe(8); }); }); }); diff --git a/legacy/src/app/directives/tests/test_pod_parameters.js b/legacy/src/app/directives/tests/test_pod_parameters.js index cf617b2382a..1436f309b51 100644 --- a/legacy/src/app/directives/tests/test_pod_parameters.js +++ b/legacy/src/app/directives/tests/test_pod_parameters.js @@ -94,6 +94,18 @@ describe("maasPodParameters", function() { } ] }, + { + name: "lxd", + description: "LXD", + driver_type: "pod", + fields: [ + { + name: "power_address", + label: "Power address", + scope: "bmc" + }, + ] + }, { name: "ipmi", description: "IPMI", @@ -108,7 +120,7 @@ describe("maasPodParameters", function() { it("sets podTypes", function() { var directive = compileDirective("true"); var scope = directive.isolateScope(); - podTypes = [powerTypes[0], powerTypes[1]]; + podTypes = [powerTypes[0], powerTypes[1], powerTypes[2]]; expect(scope.podTypes).toEqual(podTypes); }); diff --git a/legacy/src/app/partials/pod-details.html b/legacy/src/app/partials/pod-details.html index f1cc47606c3..c889002b79c 100644 --- a/legacy/src/app/partials/pod-details.html +++ b/legacy/src/app/partials/pod-details.html @@ -338,15 +338,15 @@

Storage configuration

- + ng-class="{'in-warning': storage.size !== '' && storage.size < 8}"> + data-ng-if="storage.size !== '' && storage.size < 8"> Warning Ubuntu typically requires 8GB minimum. Lower allocations may fail.