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.

Sign up for GitHub

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

Fix: Resolved errors for integration test in github action #50

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

Fix: Resolved errors for integration test in github action #50

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
497d196
initial commit
AnilGadiyarHJ Dec 6, 2024
9ce03fb
Fixed github action
AnilGadiyarHJ Dec 7, 2024
f239a20
address block changes
AnilGadiyarHJ Dec 10, 2024
aaff22d
addressed PR review
AnilGadiyarHJ Dec 11, 2024
d07f169
addressed PR review
AnilGadiyarHJ Dec 11, 2024
3521d48
Merge branch 'v2' into fix_misc_errors
AnilGadiyarHJ Dec 11, 2024
ae99357
addressed PR review
AnilGadiyarHJ Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 24 additions & 123 deletions plugins/modules/ipam_address_block.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,6 @@
- "The shared DHCP configuration that controls how leases are issued for the address block."
type: dict
suboptions:
abandoned_reclaim_time:
description:
- "The abandoned reclaim time in seconds for IPV4 clients."
type: int
abandoned_reclaim_time_v6:
description:
- "The abandoned reclaim time in seconds for IPV6 clients."
type: int
allow_unknown:
description:
- "Disable to allow leases only for known IPv4 clients, those for which a fixed address is configured."
Expand Down Expand Up @@ -474,32 +466,6 @@
- "The inheritance configuration for I(dhcp_config) field."
type: dict
suboptions:
abandoned_reclaim_time:
description:
- "The inheritance configuration for I(abandoned_reclaim_time) field from I(DHCPConfig) object."
type: dict
suboptions:
action:
description:
- "The inheritance setting for a field."
- "Valid values are:"
- "* I(inherit): Use the inherited value."
- "* I(override): Use the value set in the object."
- "Defaults to I(inherit)."
type: str
abandoned_reclaim_time_v6:
description:
- "The inheritance configuration for I(abandoned_reclaim_time_v6) field from I(DHCPConfig) object."
type: dict
suboptions:
action:
description:
- "The inheritance setting for a field."
- "Valid values are:"
- "* I(inherit): Use the inherited value."
- "* I(override): Use the value set in the object."
- "Defaults to I(inherit)."
type: str
allow_unknown:
description:
- "The inheritance configuration for I(allow_unknown) field from I(DHCPConfig) object."
Expand Down Expand Up @@ -781,10 +747,6 @@
dhcp_config:
lease_time:
action: override
abandoned_reclaim_time:
action: inherit
abandoned_reclaim_time_v6:
action: inherit
allow_unknown:
action: inherit
allow_unknown_v6:
Expand Down Expand Up @@ -974,16 +936,6 @@
type: dict
returned: Always
contains:
abandoned_reclaim_time:
description:
- "The abandoned reclaim time in seconds for IPV4 clients."
type: int
returned: Always
abandoned_reclaim_time_v6:
description:
- "The abandoned reclaim time in seconds for IPV6 clients."
type: int
returned: Always
allow_unknown:
description:
- "Disable to allow leases only for known IPv4 clients, those for which a fixed address is configured."
Expand Down Expand Up @@ -1672,66 +1624,6 @@
type: dict
returned: Always
contains:
abandoned_reclaim_time:
description:
- "The inheritance configuration for I(abandoned_reclaim_time) field from I(DHCPConfig) object."
type: dict
returned: Always
contains:
action:
description:
- "The inheritance setting for a field."
- "Valid values are:"
- "* I(inherit): Use the inherited value."
- "* I(override): Use the value set in the object."
- "Defaults to I(inherit)."
type: str
returned: Always
display_name:
description:
- "The human-readable display name for the object referred to by I(source)."
type: str
returned: Always
source:
description:
- "The resource identifier."
type: str
returned: Always
value:
description:
- "The inherited value."
type: int
returned: Always
abandoned_reclaim_time_v6:
description:
- "The inheritance configuration for I(abandoned_reclaim_time_v6) field from I(DHCPConfig) object."
type: dict
returned: Always
contains:
action:
description:
- "The inheritance setting for a field."
- "Valid values are:"
- "* I(inherit): Use the inherited value."
- "* I(override): Use the value set in the object."
- "Defaults to I(inherit)."
type: str
returned: Always
display_name:
description:
- "The human-readable display name for the object referred to by I(source)."
type: str
returned: Always
source:
description:
- "The resource identifier."
type: str
returned: Always
value:
description:
- "The inherited value."
type: int
returned: Always
allow_unknown:
description:
- "The inheritance configuration for I(allow_unknown) field from I(DHCPConfig) object."
Expand Down Expand Up @@ -2381,9 +2273,32 @@ def __init__(self, *args, **kwargs):
self.params["address"], netmask = self.params["address"].split("/")
self.params["cidr"] = int(netmask)

exclude = ["state", "csp_url", "api_key", "id"]
exclude = [
"state",
"csp_url",
"api_key",
"id",
"abandoned_reclaim_time",
"abandoned_reclaim_time_v6",
"echo_client_id",
]

self._payload_params = {k: v for k, v in self.params.items() if v is not None and k not in exclude}
self._payload = AddressBlock.from_dict(self._payload_params)

# Safely remove unwanted attributes
if self._payload.dhcp_config:
self._payload.dhcp_config.abandoned_reclaim_time = None
self._payload.dhcp_config.abandoned_reclaim_time_v6 = None
self._payload.dhcp_config.echo_client_id = None

# Handle inheritance sources safely
if self._payload.inheritance_sources and self._payload.inheritance_sources.dhcp_config:
inherited_dhcp_config = self._payload.inheritance_sources.dhcp_config
setattr(inherited_dhcp_config, "abandoned_reclaim_time", None)
setattr(inherited_dhcp_config, "abandoned_reclaim_time_v6", None)
setattr(inherited_dhcp_config, "echo_client_id", None)

self._existing = None

@property
Expand Down Expand Up @@ -2525,8 +2440,6 @@ def main():
dhcp_config=dict(
type="dict",
options=dict(
abandoned_reclaim_time=dict(type="int"),
abandoned_reclaim_time_v6=dict(type="int"),
allow_unknown=dict(type="bool"),
allow_unknown_v6=dict(type="bool"),
echo_client_id=dict(type="bool"),
Expand Down Expand Up @@ -2663,18 +2576,6 @@ def main():
dhcp_config=dict(
type="dict",
options=dict(
abandoned_reclaim_time=dict(
type="dict",
options=dict(
action=dict(type="str"),
),
),
abandoned_reclaim_time_v6=dict(
type="dict",
options=dict(
action=dict(type="str"),
),
),
allow_unknown=dict(
type="dict",
options=dict(
Expand Down
12 changes: 12 additions & 0 deletions plugins/modules/ipam_subnet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2436,6 +2436,18 @@ def __init__(self, *args, **kwargs):
exclude = ["state", "csp_url", "api_key", "id"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

As per Address block implementation , shouldn't this "exclude" contain abandoned_reclaim_time , abandoned_reclaim_time_v6 and echo_client_id too ?

Copy link
Collaborator Author

@AnilGadiyarHJ AnilGadiyarHJ Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

done

self._payload_params = {k: v for k, v in self.params.items() if v is not None and k not in exclude}
self._payload = Subnet.from_dict(self._payload_params)
# Safely remove unwanted attributes
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
# Safely remove unwanted attributes
# Unset unsupported DHCP configuration attributes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

if self._payload.dhcp_config:
self._payload.dhcp_config.abandoned_reclaim_time = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

The Documentation and Return for subnet should to be updated to remove these 3 attributes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

all the fields are required to be sent as part of the inheritance payload

self._payload.dhcp_config.abandoned_reclaim_time_v6 = None
self._payload.dhcp_config.echo_client_id = None

# Handle inheritance sources safely
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
# Handle inheritance sources safely
# Unset unsupported DHCP configuration attributes in the inherited DHCP config

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This piece of code was redundant, i have removed it

if self._payload.inheritance_sources and self._payload.inheritance_sources.dhcp_config:
inherited_dhcp_config = self._payload.inheritance_sources.dhcp_config
setattr(inherited_dhcp_config, "abandoned_reclaim_time", None)
setattr(inherited_dhcp_config, "abandoned_reclaim_time_v6", None)
setattr(inherited_dhcp_config, "echo_client_id", None)
self._existing = None

@property
Expand Down
4 changes: 0 additions & 4 deletions tests/integration/targets/ipam_address_block/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,10 +255,6 @@
dhcp_config:
lease_time:
action: override
abandoned_reclaim_time:
action: inherit
abandoned_reclaim_time_v6:
action: inherit
allow_unknown:
action: inherit
allow_unknown_v6:
Expand Down
26 changes: 3 additions & 23 deletions tests/integration/targets/ipam_subnet/tasks/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,12 @@
- subnet_info.objects | length == 1
- subnet_info.objects[0].ddns_generate_name == true

- name: "Create a Subnet with ddns_send_updates set to false"
- name: "Create a Subnet with ddns_send_updates set to true"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Its generally a good idea to test the attribute value other default values , hence in this case false would be a better input

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

infoblox.bloxone.ipam_subnet:
address: "10.0.0.0/24"
space: "{{ ip_space.id }}"
state: "present"
ddns_send_updates: "false"
ddns_send_updates: "true"
register: subnet
- name: Get information about the Subnet
infoblox.bloxone.ipam_subnet_info:
Expand All @@ -284,27 +284,7 @@
that:
- subnet is not failed
- subnet_info.objects | length == 1
- subnet_info.objects[0].ddns_send_updates == false

- name: "Create a Subnet with ddns_ttl_percent set to 25"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of deleting the Task , it would be a better idea to comment it out and add the reason as well as the ticket number indicating why the task is skipped

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

infoblox.bloxone.ipam_subnet:
address: "10.0.0.0/24"
space: "{{ ip_space.id }}"
state: "present"
ddns_ttl_percent: 25
register: subnet
- name: Get information about the Subnet
infoblox.bloxone.ipam_subnet_info:
filters:
address: "10.0.0.0"
space: "{{ ip_space.id }}"
cidr: 24
register: subnet_info
- assert:
that:
- subnet is not failed
- subnet_info.objects | length == 1
- subnet_info.objects[0].ddns_ttl_percent == 25
- subnet_info.objects[0].ddns_send_updates == true

- name: "Create a Subnet with ddns_update_on_renew set to true"
infoblox.bloxone.ipam_subnet:
Expand Down