Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[minor change] New module for out-of-band contract creation (DCNE-219) #700

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Ziaf007
Copy link

@Ziaf007 Ziaf007 commented Oct 24, 2024

added in module for oob_contract creation.
Updated test files for this as well.

@akinross akinross added the jira-sync Sync this issue to Jira label Oct 24, 2024
@github-actions github-actions bot changed the title [minor change] New module for out-of-band contract creation [minor change] New module for out-of-band contract creation (DCNE-219) Oct 24, 2024
that:
- present_check_mode is changed
- present_check_mode.previous == []
- present_check_mode.sent.vzBrCP.attributes.name == 'anstest'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertions are failing, this is the wrong class which should be vzOOBBrCP

msg: 'Please define the following variables: aci_hostname, aci_username and aci_password.'
when: aci_hostname is not defined or aci_username is not defined or aci_password is not defined

- name: ensure tenant exists for tests to kick off
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@akinross akinross self-requested a review December 16, 2024 13:52
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 21.25%. Comparing base (2fdee82) to head (9cddf5c).
Report is 7 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (2fdee82) and HEAD (9cddf5c). Click for more details.

HEAD has 13 uploads less than BASE
Flag BASE (2fdee82) HEAD (9cddf5c)
unit 8 1
integration 2 0
sanity 4 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #700       +/-   ##
===========================================
- Coverage   95.68%   21.25%   -74.44%     
===========================================
  Files         270        4      -266     
  Lines       12404     1087    -11317     
  Branches     1869      246     -1623     
===========================================
- Hits        11869      231    -11638     
- Misses        404      836      +432     
+ Partials      131       20      -111     
Flag Coverage Δ
integration ?
sanity ?
unit 21.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aliases: [ contract_name, name ]
description:
description:
- Description for the contract.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Description for the contract.
- The description of the OOB Contract.

@@ -0,0 +1,162 @@
# Test code for the ACI modules
# Copyright: (c) 2017, Jacob McGill (@jmcgill298)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update to your name

@@ -0,0 +1,162 @@
# Test code for the ACI modules
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from # CREATE/UPDATE/QUERY/DELETE Comments and separating CREATE and UPDATE, the format seems to be followed.
Do highlight if anything specific. I have updated the above.

state: absent
description: Ansible Test

- name: create contract - check mode works
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assert that defaults are set when attribute not provided

<<: *aci_oob_contract_present
register: present_idempotent

- name: update contract - update works
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update all attributes to a different value when possible


seealso:
- name: APIC Management Information Model reference
description: More information about the internal APIC class B(vz:BrCP).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
description: More information about the internal APIC class B(vz:BrCP).
description: More information about the internal APIC class B(vz:OOBBrCP).

options:
contract:
description:
- The name of the contract.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The name of the contract.
- The name of the OOB Contract.

aliases: [ descr ]
scope:
description:
- The scope of a service contract.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The scope of a service contract.
- The scope of the OOB Contract.

choices: [ application-profile, context, global, tenant ]
priority:
description:
- The desired QoS class to be used.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- The desired QoS class to be used.
- The desired Quality of Service (QoS) class to be used.

Comment on lines +238 to +255
tenant = "mgmt" # hard coding tenant to 'mgmt'
name_alias = module.params.get("name_alias")

aci = ACIModule(module)
aci.construct_url(
root_class=dict(
aci_class="fvTenant",
aci_rn="tn-{0}".format(tenant),
module_object=tenant,
target_filter={"name": tenant},
),
subclass_1=dict(
aci_class="vzOOBBrCP",
aci_rn="oobbrc-{0}".format(contract),
module_object=contract,
target_filter={"name": contract},
),
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define the subclass combine to be able to leverage class query for query all?

Suggested change
tenant = "mgmt" # hard coding tenant to 'mgmt'
name_alias = module.params.get("name_alias")
aci = ACIModule(module)
aci.construct_url(
root_class=dict(
aci_class="fvTenant",
aci_rn="tn-{0}".format(tenant),
module_object=tenant,
target_filter={"name": tenant},
),
subclass_1=dict(
aci_class="vzOOBBrCP",
aci_rn="oobbrc-{0}".format(contract),
module_object=contract,
target_filter={"name": contract},
),
)
name_alias = module.params.get("name_alias")
aci = ACIModule(module)
aci.construct_url(
root_class=dict(
aci_class="vzOOBBrCP",
aci_rn="tn-mgmt/oobbrc-{0}".format(contract),
module_object=contract,
target_filter={"name": contract},
),
)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have to test this once for querying operations. Will analyse over the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
jira-sync Sync this issue to Jira
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants