feat[dcnm_policy]: adding the functions to handle use_desc_as_key #285
Conversation
…mplateName is reported
add test cases for use_desc_as_key
dsx1123
changed the title
dsx1123
changed the title
| @@ -943,6 +1098,35 @@ def dcnm_policy_create_policy(self, policy, command): | |||
|
|
|||
| return resp | |||
|
|
|||
| def dcnm_policy_update_policy(self, policy, command): | |||
There was a problem hiding this comment.
is this necessary. you could have used create_policy with 'PUT' command instead. Any specific reason for having a new routine for update?
There was a problem hiding this comment.
I have tried the PUT method using POLICY_BULK_CREATE URL, it didn't work, besides, it is not documented. So prefer to use the documented endpoint for the update.
| not pl["deleted"] | ||
| and ( | ||
| (wp["policy_id_given"] is False) | ||
| and (pl["templateName"] == wp["templateName"]) |
There was a problem hiding this comment.
Should the template names match even in the case of use_desc_as_key is True. May be in that case we should just compare description alone. Specifically can there be two policies with different template names but same description? If yes, then we should delete policies matching description if use_desc_as_key is True.
There was a problem hiding this comment.
the idea of use_desc_as_key is to only use the description as the key, the use case it can support is, that when the user is using a different template for the same description, the module will delete the old one and create a new one with the new template.
plugins/modules/dcnm_policy.py
Outdated
| # the user must make sure no policies with the same description are created on NDFC out of the playbook. | ||
| # If the description is not unique, the module will raise an error. | ||
|
|
||
| ## Below task will policies with description "policy_radius" on swtich1, switch2 and switch3, |
There was a problem hiding this comment.
typo, should read as
Below task will create policies ...
| @@ -637,12 +716,20 @@ def dcnm_policy_get_have(self): | |||
| pl | |||
| for pl in plist | |||
| for wp in self.want | |||
| if (pl["templateName"] == wp["templateName"]) | |||
| if pl.get("source", "") == "" | |||
There was a problem hiding this comment.
What is this check for? Not clear. Can you please add a comment for pl.get("source", "")?
There was a problem hiding this comment.
will add the comment, here is the reason:
exclude the policies that have the source
when the user modifies a policy but has not deployed the policy,
a sub-policy might be created with the same description name, but marked as deleted
The signature of this kind of policy is that it has an original policyId as the source
it should be excluded from the match list as the policy will be deleted once the user deploys the configuration
| @@ -1251,7 +1437,7 @@ def dcnm_translate_config(self, config): | |||
| updated_config.append(cfg) | |||
| config = updated_config | |||
| else: | |||
| config = override_config | |||
| config += override_config | |||
There was a problem hiding this comment.
Is this working? When I checked it said '+=' not supported for dicts. Suggested to use update instead.
There was a problem hiding this comment.
config and override_config are lists, not dicts, and yes, it works
create the PR to track the changes. Will add test cases and update the document