Moti Asayag has posted comments on this change. Change subject: core: Block removing network when it's related to iSCSI bond. ......................................................................
Patch Set 3: Code-Review-1 (10 comments) http://gerrit.ovirt.org/#/c/30913/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/NetworkValidator.java: Line 143: getNetworkNameReplacement()) Line 144: : ValidationResult.VALID; Line 145: } Line 146: Line 147: public ValidationResult notISCSIBondNetwork() { s/notISCSIBondNetwork/notIscsciBondNetwork Line 148: List<IscsiBond> iscsiBonds = getDbFacade().getIscsiBondDao().getIscsiBondsByNetworkId(network.getId()); Line 149: if (!iscsiBonds.isEmpty()) { Line 150: List<String> bondNames = new ArrayList<>(); Line 151: for (IscsiBond iscsiBond : iscsiBonds) { Line 146: Line 147: public ValidationResult notISCSIBondNetwork() { Line 148: List<IscsiBond> iscsiBonds = getDbFacade().getIscsiBondDao().getIscsiBondsByNetworkId(network.getId()); Line 149: if (!iscsiBonds.isEmpty()) { Line 150: List<String> bondNames = new ArrayList<>(); Please use ReplacementUtils.replaceWithNameable() instead of this loop. Line 151: for (IscsiBond iscsiBond : iscsiBonds) { Line 152: bondNames.add(iscsiBond.getName()); Line 153: } Line 154: return new ValidationResult(VdcBllMessages.NETWORK_CANNOT_REMOVE_ISCSI_BOND_NETWORK, Line 149: if (!iscsiBonds.isEmpty()) { Line 150: List<String> bondNames = new ArrayList<>(); Line 151: for (IscsiBond iscsiBond : iscsiBonds) { Line 152: bondNames.add(iscsiBond.getName()); Line 153: } please add a space line Line 154: return new ValidationResult(VdcBllMessages.NETWORK_CANNOT_REMOVE_ISCSI_BOND_NETWORK, Line 155: getNetworkNameReplacement(), Line 156: getIscsiBondsReplacement(bondNames)); Line 157: } Line 153: } Line 154: return new ValidationResult(VdcBllMessages.NETWORK_CANNOT_REMOVE_ISCSI_BOND_NETWORK, Line 155: getNetworkNameReplacement(), Line 156: getIscsiBondsReplacement(bondNames)); Line 157: } please add a space line Line 158: return ValidationResult.VALID; Line 159: } Line 160: Line 161: private String getNetworkNameReplacement() { Line 161: private String getNetworkNameReplacement() { Line 162: return String.format("$NetworkName %s", network.getName()); Line 163: } Line 164: Line 165: private String getIscsiBondsReplacement(List iscsiBonds) { please add type to List Line 166: return String.format("$IscsiBonds %s", iscsiBonds.toString()); Line 167: } Line 168: Line 169: protected ValidationResult networkNotUsed(List<? extends Nameable> entities, VdcBllMessages entitiesReplacement) { Line 162: return String.format("$NetworkName %s", network.getName()); Line 163: } Line 164: Line 165: private String getIscsiBondsReplacement(List iscsiBonds) { Line 166: return String.format("$IscsiBonds %s", iscsiBonds.toString()); please avoid relying on List.toString() for formatting the text and use: StringUtils.join(iscsiBonds, ", ") This will be provided by using ReplacementUtils.replaceWithNameable() Line 167: } Line 168: Line 169: protected ValidationResult networkNotUsed(List<? extends Nameable> entities, VdcBllMessages entitiesReplacement) { Line 170: if (entities.isEmpty()) { http://gerrit.ovirt.org/#/c/30913/3/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 610: TAGS_SPECIFIED_TAG_CANNOT_BE_THE_PARENT_OF_ITSELF=Operation canceled, recursive Tag hierarchy cannot be defined. Line 611: VM_CANNOT_MOVE_TO_CLUSTER_IN_OTHER_STORAGE_POOL=VM can be moved only to a Cluster in the same Data Center. Line 612: VM_CLUSTER_IS_NOT_VALID=Cannot ${action} ${type}. Cluster ID is not valid. Line 613: NETWORK_CANNOT_REMOVE_MANAGEMENT_NETWORK=The Management Network ('${NetworkName}') is mandatory and cannot be removed. Line 614: NETWORK_CANNOT_REMOVE_ISCSI_BOND_NETWORK=The Network ('${NetworkName}') could not be removed since it is part of the following iSCSI bonds:\n ${IscsiBonds}.\nPlease, remove the network first from those iSCSI bonds, and try again. Is the comma after "Please" needed ? Line 615: NETWORK_OLD_NETWORK_NOT_SPECIFIED=Previous network name is required. Line 616: ACTION_TYPE_FAILED_DETECTED_ACTIVE_VMS=Cannot ${action} ${type}. Active VMs were detected.\n\ Line 617: -Please ensure all VMs associated with this Storage Domain are stopped and in the Down state first. Line 618: ACTION_TYPE_FAILED_HOST_NOT_EXIST=The provided Host does not exist. http://gerrit.ovirt.org/#/c/30913/3/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/IscsiBondDaoTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/IscsiBondDaoTest.java: Line 118: Line 119: @Test Line 120: public void testGetEmptyIscsiBondIdByNetworkId() { Line 121: List<IscsiBond> fetchedIscsiBonds = dao.getIscsiBondsByNetworkId(networkId); Line 122: assertEquals(0, fetchedIscsiBonds.size()); assertTrue(fetchedIscsiBonds.isEmpty()) ? Line 123: } Line 124: Line 125: @Test Line 126: public void testGetEmptyIscsiBondIdByNotExistingNetworkId() { Line 124: Line 125: @Test Line 126: public void testGetEmptyIscsiBondIdByNotExistingNetworkId() { Line 127: List<IscsiBond> fetchedIscsiBonds = dao.getIscsiBondsByNetworkId(Guid.Empty); Line 128: assertEquals(0, fetchedIscsiBonds.size()); same Line 129: } Line 130: Line 131: @Test Line 132: public void testRemoveNetworkFromIscsiBond() { http://gerrit.ovirt.org/#/c/30913/3/frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties File frontend/webadmin/modules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties: Line 580: TAGS_SPECIFIED_TAG_CANNOT_BE_THE_PARENT_OF_ITSELF=Operation canceled, recursive Tag hierarchy cannot be defined. Line 581: VM_CANNOT_MOVE_TO_CLUSTER_IN_OTHER_STORAGE_POOL=VM can be moved only to a Cluster in the same Data Center. Line 582: VM_CLUSTER_IS_NOT_VALID=Cannot ${action} ${type}. Cluster ID is not valid. Line 583: NETWORK_CANNOT_REMOVE_MANAGEMENT_NETWORK=The Management Network ('${NetworkName}') is mandatory and cannot be removed. Line 584: NETWORK_CANNOT_REMOVE_ISCSI_BOND_NETWORK=The Network ('${NetworkName}') could not be removed since it is part of the following iSCSI bonds:\n ${IscsiBonds}.\nPlease, remove the network first from those iSCSI bonds, and try again. this change is not needed. network actions are supported only from the webadmin. Line 585: NETWORK_OLD_NETWORK_NOT_SPECIFIED=Previous network name is required. Line 586: ACTION_TYPE_FAILED_DETECTED_ACTIVE_VMS=Cannot ${action} ${type}. Active VMs were detected.\n\ Line 587: -Please ensure all VMs associated with this Storage Domain are stopped and in the Down state first. Line 588: ACTION_TYPE_FAILED_HOST_NOT_EXIST=The provided Host does not exist. -- To view, visit http://gerrit.ovirt.org/30913 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I68bd390175d42ee4e33773e12d6184a97a755eed Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
