cherry pick [ONOS-4986] [ONOS-4985] Json defect fix to master
Change-Id: Ia9ead1babf3de43e6f492f4f3b6f4d6b9377b042
Showing
8 changed files
with
64 additions
and
25 deletions
| ... | @@ -80,7 +80,8 @@ public class PceQueryPathCommand extends AbstractShellCommand { | ... | @@ -80,7 +80,8 @@ public class PceQueryPathCommand extends AbstractShellCommand { |
| 80 | "constraints: \n" + | 80 | "constraints: \n" + |
| 81 | " cost : %s \n" + | 81 | " cost : %s \n" + |
| 82 | " bandwidth : %s", | 82 | " bandwidth : %s", |
| 83 | - tunnel.tunnelId().id(), tunnel.src().toString(), tunnel.dst().toString(), | 83 | + tunnel.tunnelId().id(), tunnel.path().src().deviceId().toString(), |
| 84 | + tunnel.path().dst().deviceId().toString(), | ||
| 84 | tunnel.type().name(), tunnel.tunnelName(), tunnel.annotations().value(COST_TYPE), | 85 | tunnel.type().name(), tunnel.tunnelName(), tunnel.annotations().value(COST_TYPE), |
| 85 | tunnel.annotations().value(AnnotationKeys.BANDWIDTH)); | 86 | tunnel.annotations().value(AnnotationKeys.BANDWIDTH)); |
| 86 | } | 87 | } | ... | ... |
| ... | @@ -47,7 +47,7 @@ public class PceUpdatePathCommand extends AbstractShellCommand { | ... | @@ -47,7 +47,7 @@ public class PceUpdatePathCommand extends AbstractShellCommand { |
| 47 | 47 | ||
| 48 | @Option(name = "-c", aliases = "--cost", description = "The cost attribute IGP cost (1) or TE cost (2).", | 48 | @Option(name = "-c", aliases = "--cost", description = "The cost attribute IGP cost (1) or TE cost (2).", |
| 49 | required = false, multiValued = false) | 49 | required = false, multiValued = false) |
| 50 | - int cost = -1; | 50 | + Integer cost = null; |
| 51 | 51 | ||
| 52 | @Option(name = "-b", aliases = "--bandwidth", description = "The bandwidth attribute of path. " | 52 | @Option(name = "-b", aliases = "--bandwidth", description = "The bandwidth attribute of path. " |
| 53 | + "Data rate unit is in Bps.", required = false, multiValued = false) | 53 | + "Data rate unit is in Bps.", required = false, multiValued = false) |
| ... | @@ -66,7 +66,7 @@ public class PceUpdatePathCommand extends AbstractShellCommand { | ... | @@ -66,7 +66,7 @@ public class PceUpdatePathCommand extends AbstractShellCommand { |
| 66 | } | 66 | } |
| 67 | 67 | ||
| 68 | // Cost validation | 68 | // Cost validation |
| 69 | - if (cost != -1) { | 69 | + if (cost != null) { |
| 70 | if ((cost < 1) || (cost > 2)) { | 70 | if ((cost < 1) || (cost > 2)) { |
| 71 | error("The cost attribute value is either IGP cost(1) or TE cost(2)."); | 71 | error("The cost attribute value is either IGP cost(1) or TE cost(2)."); |
| 72 | return; | 72 | return; | ... | ... |
| ... | @@ -162,13 +162,14 @@ public final class DefaultPcePath implements PcePath { | ... | @@ -162,13 +162,14 @@ public final class DefaultPcePath implements PcePath { |
| 162 | @Override | 162 | @Override |
| 163 | public String toString() { | 163 | public String toString() { |
| 164 | return toStringHelper(this) | 164 | return toStringHelper(this) |
| 165 | + .omitNullValues() | ||
| 165 | .add("id", id()) | 166 | .add("id", id()) |
| 166 | .add("source", source) | 167 | .add("source", source) |
| 167 | .add("destination", destination) | 168 | .add("destination", destination) |
| 168 | .add("lsptype", lspType) | 169 | .add("lsptype", lspType) |
| 169 | .add("name", name) | 170 | .add("name", name) |
| 170 | - .add("costConstraint", costConstraint.toString()) | 171 | + .add("costConstraint", costConstraint) |
| 171 | - .add("bandwidthConstraint", bandwidthConstraint.toString()) | 172 | + .add("bandwidthConstraint", bandwidthConstraint) |
| 172 | .toString(); | 173 | .toString(); |
| 173 | } | 174 | } |
| 174 | 175 | ||
| ... | @@ -241,8 +242,8 @@ public final class DefaultPcePath implements PcePath { | ... | @@ -241,8 +242,8 @@ public final class DefaultPcePath implements PcePath { |
| 241 | @Override | 242 | @Override |
| 242 | public Builder of(Tunnel tunnel) { | 243 | public Builder of(Tunnel tunnel) { |
| 243 | this.id = TunnelId.valueOf(tunnel.tunnelId().id()); | 244 | this.id = TunnelId.valueOf(tunnel.tunnelId().id()); |
| 244 | - this.source = tunnel.src().toString(); | 245 | + this.source = tunnel.path().src().deviceId().toString(); |
| 245 | - this.destination = tunnel.dst().toString(); | 246 | + this.destination = tunnel.path().dst().deviceId().toString(); |
| 246 | this.name = tunnel.tunnelName().toString(); | 247 | this.name = tunnel.tunnelName().toString(); |
| 247 | // LSP type | 248 | // LSP type |
| 248 | String lspType = tunnel.annotations().value(PcepAnnotationKeys.LSP_SIG_TYPE); | 249 | String lspType = tunnel.annotations().value(PcepAnnotationKeys.LSP_SIG_TYPE); | ... | ... |
| ... | @@ -509,7 +509,7 @@ public class PceManager implements PceService { | ... | @@ -509,7 +509,7 @@ public class PceManager implements PceService { |
| 509 | } | 509 | } |
| 510 | 510 | ||
| 511 | if (existingBwValue != null) { | 511 | if (existingBwValue != null) { |
| 512 | - if (bwConstraintValue == 0 && bwConstraint != null) { | 512 | + if (bwConstraint == null) { |
| 513 | bwConstraintValue = existingBwValue.bps(); | 513 | bwConstraintValue = existingBwValue.bps(); |
| 514 | } | 514 | } |
| 515 | //If bandwidth constraints not specified , take existing bandwidth for shared bandwidth calculation | 515 | //If bandwidth constraints not specified , take existing bandwidth for shared bandwidth calculation |
| ... | @@ -526,6 +526,11 @@ public class PceManager implements PceService { | ... | @@ -526,6 +526,11 @@ public class PceManager implements PceService { |
| 526 | constraints.add(CapabilityConstraint.of(CapabilityType.valueOf(lspSigType))); | 526 | constraints.add(CapabilityConstraint.of(CapabilityType.valueOf(lspSigType))); |
| 527 | if (costConstraint != null) { | 527 | if (costConstraint != null) { |
| 528 | constraints.add(costConstraint); | 528 | constraints.add(costConstraint); |
| 529 | + } else { | ||
| 530 | + //Take cost constraint from old tunnel if it is not specified in update flow | ||
| 531 | + costType = tunnel.annotations().value(COST_TYPE); | ||
| 532 | + costConstraint = CostConstraint.of(CostConstraint.Type.valueOf(costType)); | ||
| 533 | + constraints.add(costConstraint); | ||
| 529 | } | 534 | } |
| 530 | 535 | ||
| 531 | computedPathSet = computePath(links.get(0).src().deviceId(), links.get(links.size() - 1).dst().deviceId(), | 536 | computedPathSet = computePath(links.get(0).src().deviceId(), links.get(links.size() - 1).dst().deviceId(), |
| ... | @@ -845,6 +850,10 @@ public class PceManager implements PceService { | ... | @@ -845,6 +850,10 @@ public class PceManager implements PceService { |
| 845 | return; | 850 | return; |
| 846 | } | 851 | } |
| 847 | 852 | ||
| 853 | + if (pceStore.getTunnelInfo(tunnel.tunnelId()) == null) { | ||
| 854 | + //If bandwidth for old tunnel is not allocated i,e 0 then no need to release | ||
| 855 | + return; | ||
| 856 | + } | ||
| 848 | resourceService.release(pceStore.getTunnelInfo(tunnel.tunnelId()).tunnelConsumerId()); | 857 | resourceService.release(pceStore.getTunnelInfo(tunnel.tunnelId()).tunnelConsumerId()); |
| 849 | return; | 858 | return; |
| 850 | 859 | ||
| ... | @@ -1211,7 +1220,7 @@ public class PceManager implements PceService { | ... | @@ -1211,7 +1220,7 @@ public class PceManager implements PceService { |
| 1211 | localLspIdFreeList.add(Short.valueOf(tunnel.annotations().value(LOCAL_LSP_ID))); | 1220 | localLspIdFreeList.add(Short.valueOf(tunnel.annotations().value(LOCAL_LSP_ID))); |
| 1212 | } | 1221 | } |
| 1213 | // If not zero bandwidth, and delegated (initiated LSPs will also be delegated). | 1222 | // If not zero bandwidth, and delegated (initiated LSPs will also be delegated). |
| 1214 | - if (Float.parseFloat(tunnel.annotations().value(BANDWIDTH)) != 0 | 1223 | + if (bwConstraintValue != 0 |
| 1215 | && mastershipService.getLocalRole(tunnel.path().src().deviceId()) == MastershipRole.MASTER) { | 1224 | && mastershipService.getLocalRole(tunnel.path().src().deviceId()) == MastershipRole.MASTER) { |
| 1216 | releaseBandwidth(tunnel); | 1225 | releaseBandwidth(tunnel); |
| 1217 | } | 1226 | } | ... | ... |
| ... | @@ -98,6 +98,17 @@ public class DistributedPceStore implements PceStore { | ... | @@ -98,6 +98,17 @@ public class DistributedPceStore implements PceStore { |
| 98 | // List of PCC LSR ids whose BGP device information was not available to perform | 98 | // List of PCC LSR ids whose BGP device information was not available to perform |
| 99 | // label db sync. | 99 | // label db sync. |
| 100 | private HashSet<DeviceId> pendinglabelDbSyncPccMap = new HashSet(); | 100 | private HashSet<DeviceId> pendinglabelDbSyncPccMap = new HashSet(); |
| 101 | + private static final Serializer SERIALIZER = Serializer | ||
| 102 | + .using(new KryoNamespace.Builder().register(KryoNamespaces.API) | ||
| 103 | + .register(PcePathInfo.class) | ||
| 104 | + .register(CostConstraint.class) | ||
| 105 | + .register(CostConstraint.Type.class) | ||
| 106 | + .register(BandwidthConstraint.class) | ||
| 107 | + .register(SharedBandwidthConstraint.class) | ||
| 108 | + .register(CapabilityConstraint.class) | ||
| 109 | + .register(CapabilityConstraint.CapabilityType.class) | ||
| 110 | + .register(LspType.class) | ||
| 111 | + .build()); | ||
| 101 | 112 | ||
| 102 | @Activate | 113 | @Activate |
| 103 | protected void activate() { | 114 | protected void activate() { |
| ... | @@ -136,19 +147,7 @@ public class DistributedPceStore implements PceStore { | ... | @@ -136,19 +147,7 @@ public class DistributedPceStore implements PceStore { |
| 136 | 147 | ||
| 137 | failedPathSet = storageService.<PcePathInfo>setBuilder() | 148 | failedPathSet = storageService.<PcePathInfo>setBuilder() |
| 138 | .withName("failed-path-info") | 149 | .withName("failed-path-info") |
| 139 | - .withSerializer(Serializer.using( | 150 | + .withSerializer(SERIALIZER) |
| 140 | - new KryoNamespace.Builder() | ||
| 141 | - .register(KryoNamespaces.API) | ||
| 142 | - .register(PcePathInfo.class, | ||
| 143 | - CostConstraint.class, | ||
| 144 | - CostConstraint.Type.class, | ||
| 145 | - BandwidthConstraint.class, | ||
| 146 | - SharedBandwidthConstraint.class, | ||
| 147 | - CapabilityConstraint.class, | ||
| 148 | - CapabilityConstraint.CapabilityType.class, | ||
| 149 | - LspType.class) | ||
| 150 | - .build())) | ||
| 151 | - | ||
| 152 | .build() | 151 | .build() |
| 153 | .asDistributedSet(); | 152 | .asDistributedSet(); |
| 154 | 153 | ... | ... |
| ... | @@ -618,7 +618,10 @@ public class PceManagerTest { | ... | @@ -618,7 +618,10 @@ public class PceManagerTest { |
| 618 | build4RouterTopo(false, true, true, true, 100); | 618 | build4RouterTopo(false, true, true, true, 100); |
| 619 | 619 | ||
| 620 | // Setup tunnel. | 620 | // Setup tunnel. |
| 621 | - boolean result = pceManager.setupPath(D1.deviceId(), D2.deviceId(), "T123", null, WITH_SIGNALLING); | 621 | + List<Constraint> constraints = new LinkedList<Constraint>(); |
| 622 | + CostConstraint costConstraint = new CostConstraint(TE_COST); | ||
| 623 | + constraints.add(costConstraint); | ||
| 624 | + boolean result = pceManager.setupPath(D1.deviceId(), D2.deviceId(), "T123", constraints, WITH_SIGNALLING); | ||
| 622 | assertThat(result, is(true)); | 625 | assertThat(result, is(true)); |
| 623 | 626 | ||
| 624 | Collection<Tunnel> tunnels = (Collection<Tunnel>) pceManager.queryAllPath(); | 627 | Collection<Tunnel> tunnels = (Collection<Tunnel>) pceManager.queryAllPath(); | ... | ... |
| ... | @@ -21,6 +21,8 @@ import org.onosproject.codec.CodecContext; | ... | @@ -21,6 +21,8 @@ import org.onosproject.codec.CodecContext; |
| 21 | import org.onosproject.codec.JsonCodec; | 21 | import org.onosproject.codec.JsonCodec; |
| 22 | import org.onosproject.pce.pceservice.PcePath; | 22 | import org.onosproject.pce.pceservice.PcePath; |
| 23 | import org.onosproject.pce.pceservice.DefaultPcePath; | 23 | import org.onosproject.pce.pceservice.DefaultPcePath; |
| 24 | +import org.onosproject.net.intent.constraint.BandwidthConstraint; | ||
| 25 | +import org.onosproject.pce.pceservice.constraint.CostConstraint; | ||
| 24 | import org.slf4j.Logger; | 26 | import org.slf4j.Logger; |
| 25 | import org.slf4j.LoggerFactory; | 27 | import org.slf4j.LoggerFactory; |
| 26 | 28 | ||
| ... | @@ -70,6 +72,11 @@ public final class PcePathCodec extends JsonCodec<PcePath> { | ... | @@ -70,6 +72,11 @@ public final class PcePathCodec extends JsonCodec<PcePath> { |
| 70 | jNode = json.get(LSP_TYPE); | 72 | jNode = json.get(LSP_TYPE); |
| 71 | if (jNode != null) { | 73 | if (jNode != null) { |
| 72 | String lspType = jNode.asText(); | 74 | String lspType = jNode.asText(); |
| 75 | + //Validating LSP type | ||
| 76 | + int type = Integer.parseInt(lspType); | ||
| 77 | + if ((type < 0) || (type > 2)) { | ||
| 78 | + return null; | ||
| 79 | + } | ||
| 73 | resultBuilder.lspType(lspType); | 80 | resultBuilder.lspType(lspType); |
| 74 | } | 81 | } |
| 75 | 82 | ||
| ... | @@ -87,6 +94,11 @@ public final class PcePathCodec extends JsonCodec<PcePath> { | ... | @@ -87,6 +94,11 @@ public final class PcePathCodec extends JsonCodec<PcePath> { |
| 87 | jNode = constraintJNode.get(COST); | 94 | jNode = constraintJNode.get(COST); |
| 88 | if (jNode != null) { | 95 | if (jNode != null) { |
| 89 | String cost = jNode.asText(); | 96 | String cost = jNode.asText(); |
| 97 | + //Validating Cost type | ||
| 98 | + int costType = Integer.parseInt(cost); | ||
| 99 | + if ((costType < 1) || (costType > 2)) { | ||
| 100 | + return null; | ||
| 101 | + } | ||
| 90 | resultBuilder.costConstraint(cost); | 102 | resultBuilder.costConstraint(cost); |
| 91 | } | 103 | } |
| 92 | 104 | ||
| ... | @@ -94,6 +106,10 @@ public final class PcePathCodec extends JsonCodec<PcePath> { | ... | @@ -94,6 +106,10 @@ public final class PcePathCodec extends JsonCodec<PcePath> { |
| 94 | jNode = constraintJNode.get(BANDWIDTH); | 106 | jNode = constraintJNode.get(BANDWIDTH); |
| 95 | if (jNode != null) { | 107 | if (jNode != null) { |
| 96 | String bandwidth = jNode.asText(); | 108 | String bandwidth = jNode.asText(); |
| 109 | + double bw = Double.parseDouble(bandwidth); | ||
| 110 | + if (bw < 0) { | ||
| 111 | + return null; | ||
| 112 | + } | ||
| 97 | resultBuilder.bandwidthConstraint(bandwidth); | 113 | resultBuilder.bandwidthConstraint(bandwidth); |
| 98 | } | 114 | } |
| 99 | } | 115 | } |
| ... | @@ -114,8 +130,8 @@ public final class PcePathCodec extends JsonCodec<PcePath> { | ... | @@ -114,8 +130,8 @@ public final class PcePathCodec extends JsonCodec<PcePath> { |
| 114 | 130 | ||
| 115 | ObjectNode constraintNode = context.mapper() | 131 | ObjectNode constraintNode = context.mapper() |
| 116 | .createObjectNode() | 132 | .createObjectNode() |
| 117 | - .put(COST, path.costConstraint().toString()) | 133 | + .put(COST, ((CostConstraint) path.costConstraint()).type().type()) |
| 118 | - .put(BANDWIDTH, path.bandwidthConstraint().toString()); | 134 | + .put(BANDWIDTH, ((BandwidthConstraint) path.bandwidthConstraint()).bandwidth().bps()); |
| 119 | 135 | ||
| 120 | result.set(CONSTRAINT, constraintNode); | 136 | result.set(CONSTRAINT, constraintNode); |
| 121 | return result; | 137 | return result; | ... | ... |
| ... | @@ -101,6 +101,9 @@ public class PcePathWebResource extends AbstractWebResource { | ... | @@ -101,6 +101,9 @@ public class PcePathWebResource extends AbstractWebResource { |
| 101 | Tunnel tunnel = nullIsNotFound(get(PceService.class).queryPath(TunnelId.valueOf(id)), | 101 | Tunnel tunnel = nullIsNotFound(get(PceService.class).queryPath(TunnelId.valueOf(id)), |
| 102 | PCE_PATH_NOT_FOUND); | 102 | PCE_PATH_NOT_FOUND); |
| 103 | PcePath path = DefaultPcePath.builder().of(tunnel).build(); | 103 | PcePath path = DefaultPcePath.builder().of(tunnel).build(); |
| 104 | + if (path == null) { | ||
| 105 | + return Response.status(OK).entity(PCE_SETUP_PATH_FAILED).build(); | ||
| 106 | + } | ||
| 104 | ObjectNode result = mapper().createObjectNode(); | 107 | ObjectNode result = mapper().createObjectNode(); |
| 105 | result.set("path", codec(PcePath.class).encode(path, this)); | 108 | result.set("path", codec(PcePath.class).encode(path, this)); |
| 106 | return ok(result.toString()).build(); | 109 | return ok(result.toString()).build(); |
| ... | @@ -122,6 +125,10 @@ public class PcePathWebResource extends AbstractWebResource { | ... | @@ -122,6 +125,10 @@ public class PcePathWebResource extends AbstractWebResource { |
| 122 | JsonNode port = jsonTree.get("path"); | 125 | JsonNode port = jsonTree.get("path"); |
| 123 | TunnelService tunnelService = get(TunnelService.class); | 126 | TunnelService tunnelService = get(TunnelService.class); |
| 124 | PcePath path = codec(PcePath.class).decode((ObjectNode) port, this); | 127 | PcePath path = codec(PcePath.class).decode((ObjectNode) port, this); |
| 128 | + if (path == null) { | ||
| 129 | + return Response.status(OK).entity(PCE_SETUP_PATH_FAILED).build(); | ||
| 130 | + } | ||
| 131 | + | ||
| 125 | //Validating tunnel name, duplicated tunnel names not allowed | 132 | //Validating tunnel name, duplicated tunnel names not allowed |
| 126 | Collection<Tunnel> existingTunnels = tunnelService.queryTunnel(Tunnel.Type.MPLS); | 133 | Collection<Tunnel> existingTunnels = tunnelService.queryTunnel(Tunnel.Type.MPLS); |
| 127 | if (existingTunnels != null) { | 134 | if (existingTunnels != null) { |
| ... | @@ -171,6 +178,9 @@ public class PcePathWebResource extends AbstractWebResource { | ... | @@ -171,6 +178,9 @@ public class PcePathWebResource extends AbstractWebResource { |
| 171 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); | 178 | ObjectNode jsonTree = (ObjectNode) mapper().readTree(stream); |
| 172 | JsonNode pathNode = jsonTree.get("path"); | 179 | JsonNode pathNode = jsonTree.get("path"); |
| 173 | PcePath path = codec(PcePath.class).decode((ObjectNode) pathNode, this); | 180 | PcePath path = codec(PcePath.class).decode((ObjectNode) pathNode, this); |
| 181 | + if (path == null) { | ||
| 182 | + return Response.status(OK).entity(PCE_SETUP_PATH_FAILED).build(); | ||
| 183 | + } | ||
| 174 | List<Constraint> constrntList = new LinkedList<Constraint>(); | 184 | List<Constraint> constrntList = new LinkedList<Constraint>(); |
| 175 | // Assign bandwidth | 185 | // Assign bandwidth |
| 176 | if (path.bandwidthConstraint() != null) { | 186 | if (path.bandwidthConstraint() != null) { | ... | ... |
-
Please register or login to post a comment