feat: OC to IOS-XE QoS #246

jrouliez · 2023-07-03T03:15:29Z

feat: OC to IOS-XE QoS

…ven-devops/nso-oc-services into oc_to_xe_qos_class_map

m0lass3s · 2023-07-05T21:46:48Z

mdd/python/translation/openconfig_xe/xe_qos.py

+        raise ValueError(
+            f'Interface type {interface_type} not supported by this NSO_OC_Services implementation. Please file an issue at https://github.com/model-driven-devops/nso-oc-services')
+
+def modify_dscp(dscp):


Instead of a long winded if-else statement, just create a globally scoped object. We can then map by key-value pairs, where keys are dscp and values are those currently in new_dscp. You can still account for your if, but your else if and else can be converted to use the object.

Updates made.

m0lass3s · 2023-07-05T21:46:52Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    Configure forwarding-groups
+    """
+    if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

SIDE NOTE: I am not aware if nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group is guaranteed to always return something (i.e. it's defined). If so, then ignore this side note. This note refers to everything under nso_props.service.oc_qos__qos.

Changing block to:
for fg in nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group:
device_cdb.ios__policy_map.create(fg.name)
if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:
if device_cdb.ios__policy_map:
device_cdb.ios__policy_map.delete()

m0lass3s · 2023-07-05T21:46:54Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) > 0:
+        for fg in nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group:
+            device_cdb.ios__policy_map.create(fg.name)
+    elif len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:


Change to an if statement, since we're getting rid of the if statement above. It will only run if forwarding_group is empty.

See the reply from the previous conversation.

m0lass3s · 2023-07-05T21:52:14Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    Configure schedulers
+    """
+    if len(nso_props.service.oc_qos__qos.scheduler_policies.scheduler_policy) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T21:53:43Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    Configure classifiers
+    """
+    if len(nso_props.service.oc_qos__qos.classifiers.classifier) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T21:54:18Z

mdd/python/translation/openconfig_xe/xe_qos.py

+                        device_cdb.ios__class_map[c_map.name].prematch = 'match-all'
+                        device_cdb.ios__class_map[c_map.name].match.dscp.create(
+                            new_dscp)
+    elif len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:


Change to an if statement, since we're getting rid of the if statement above.

Updated 'elif' to 'if'.

m0lass3s · 2023-07-05T21:55:20Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    """
+    if len(nso_props.service.oc_qos__qos.scheduler_policies.scheduler_policy) > 0:
+        for sched_pol in nso_props.service.oc_qos__qos.scheduler_policies.scheduler_policy:
+            if len(sched_pol.schedulers.scheduler) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T22:00:26Z

mdd/python/translation/openconfig_xe/xe_qos.py

+                for sequence in sched_pol.schedulers.scheduler:
+                    # Configure policy-map
+                    p_map = device_cdb.ios__policy_map.create(sequence.output.config.output_fwd_group)
+                    if pmap_cmap:


Combine this to a single line, to prevent unnecessary tabbing

Suggested change

if pmap_cmap:

if pmap_cmap and p_map.name in pmap_cmap.keys():

'if' statement combined.

m0lass3s · 2023-07-05T22:02:29Z

mdd/python/translation/openconfig_xe/xe_qos.py

+                    """
+                    Configure interfaces
+                    """
+                    if len(nso_props.service.oc_qos__qos.interfaces.interface) > 0:


This if statement isn't required. The loop will run if there are more than 0 elements.

'if' statement removed.

m0lass3s · 2023-07-05T22:24:13Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if sequence.one_rate_two_color:
+        # Configure cir percentage
+        if sequence.one_rate_two_color.config.cir_pct:
+            device_cdb.ios__policy_map[p_map].ios__class[c_map].police_cir_percent.police.cir.percent.percentage = sequence.one_rate_two_color.config.cir_pct


The code can be cleaned up to make it less verbose. We can greatly remove a lot of repeated text such as device_cdb.ios__policy_map[p_map].ios__class[c_map].police_cir_percent.police.cir.percent by storing them in variables and referencing those variables instead of the long text. device_cdb.ios__policy_map[p_map].ios__class[c_map].police_cir_percent.police.cir.percent is just an example and there are a few others.

Updates made.

m0lass3s · 2023-07-12T21:31:14Z

mdd/python/translation/openconfig_xe/xe_qos.py

+def modify_dscp(dscp):
+    if (dscp % 2) != 0:
+        return dscp
+    if dscp in dscp_dict.keys():


This if statement isn't required. The get function will perform a check if that key exist, otherwise that second arg ('default') value is returned.

Suggested change

if dscp in dscp_dict.keys():

return dscp_dict.get(dscp, 'default')

If statement removed.

m0lass3s · 2023-07-12T21:40:40Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:
+        if device_cdb.ios__class_map:


Minor, but you can just combine these two if statements since you're checking both for truthy.

Suggested change

if len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:

if device_cdb.ios__class_map:

if (len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0

and device_cdb.ios__class_map):

m0lass3s · 2023-07-12T21:41:28Z

mdd/python/translation/openconfig_xe/xe_qos.py

+    if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:
+        if device_cdb.ios__policy_map:


Minor, but you can just combine these two if statements since you're checking both for truthy.

Suggested change

if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:

if device_cdb.ios__policy_map:

if (len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0

and device_cdb.ios__policy_map):

stmosher-dev

run tests

rerun

stmosher-dev

LGTM

rerun

jrouliez added 7 commits June 6, 2023 19:36

feat: OC to XE QoS Class-map

735f3dc

Merge branch 'main' into oc_to_xe_qos_class_map

181edb3

feat: OC to XE QoS Policy-map, One rate two color, Two rate three color.

fc3ee6d

Merge branch 'main' into oc_to_xe_qos_class_map

caeb439

Merge branch 'oc_to_xe_qos_class_map' of https://github.com/model-dri…

0d66255

…ven-devops/nso-oc-services into oc_to_xe_qos_class_map

feat: OC to XE QoS Two rate three color.

0efb147

feat: OC to XE QoS main updates

a8cbd76

m0lass3s requested changes Jul 6, 2023

View reviewed changes

feat: update to support mutiple class-maps in policy-maps

a34d8de

jrouliez force-pushed the oc_to_xe_qos_class_map branch from 0666e8b to a34d8de Compare July 12, 2023 17:20

m0lass3s reviewed Jul 12, 2023

View reviewed changes

jrouliez force-pushed the oc_to_xe_qos_class_map branch from fec7a74 to 40cda2d Compare July 13, 2023 04:04

stmosher-dev previously approved these changes Jul 13, 2023

View reviewed changes

jrouliez force-pushed the oc_to_xe_qos_class_map branch 2 times, most recently from c3140df to 84181b3 Compare July 14, 2023 04:41

stmosher-dev previously approved these changes Jul 14, 2023

View reviewed changes

fix: update classifier object to account for 'class-default'.

2e25df8

jrouliez force-pushed the oc_to_xe_qos_class_map branch from 84181b3 to 2e25df8 Compare July 14, 2023 19:03

stmosher-dev approved these changes Jul 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: OC to IOS-XE QoS #246

feat: OC to IOS-XE QoS #246

jrouliez commented Jul 3, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 12, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023 •

edited

Loading

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 7, 2023

m0lass3s Jul 5, 2023

jrouliez Jul 12, 2023

m0lass3s Jul 12, 2023 •

edited

Loading

jrouliez Jul 13, 2023

m0lass3s Jul 12, 2023 •

edited

Loading

jrouliez Jul 13, 2023

m0lass3s Jul 12, 2023 •

edited

Loading

jrouliez Jul 13, 2023

stmosher-dev left a comment

stmosher-dev left a comment

	if pmap_cmap:
	if pmap_cmap and p_map.name in pmap_cmap.keys():

	if dscp in dscp_dict.keys():
	return dscp_dict.get(dscp, 'default')

		if len(nso_props.service.oc_qos__qos.classifiers.classifier) == 0:
		if device_cdb.ios__class_map:

		if len(nso_props.service.oc_qos__qos.forwarding_groups.forwarding_group) == 0:
		if device_cdb.ios__policy_map:

feat: OC to IOS-XE QoS #246

Are you sure you want to change the base?

feat: OC to IOS-XE QoS #246

Conversation

jrouliez commented Jul 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrouliez Jul 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m0lass3s Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m0lass3s Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m0lass3s Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stmosher-dev left a comment

Choose a reason for hiding this comment

stmosher-dev left a comment

Choose a reason for hiding this comment

jrouliez Jul 7, 2023 •

edited

Loading

m0lass3s Jul 12, 2023 •

edited

Loading

m0lass3s Jul 12, 2023 •

edited

Loading

m0lass3s Jul 12, 2023 •

edited

Loading