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

Added modules to manage Endpoint IP and MAC Tags (DCNE-287) #723

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

Conversation

sajagana
Copy link
Collaborator

No description provided.

@sajagana sajagana linked an issue Feb 19, 2025 that may be closed by this pull request
@github-actions github-actions bot changed the title Added modules to manage Endpoint IP and MAC Tags Added modules to manage Endpoint IP and MAC Tags (DCNE-287) Feb 19, 2025
@sajagana sajagana requested a review from akinross February 20, 2025 10:09
@sajagana sajagana force-pushed the 714_endpoint_policy_tags branch from cfd740b to c333366 Compare February 20, 2025 12:45
Copy link
Collaborator

@akinross akinross left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +283 to +290
- query_with_vrf.current | length >= 1
- query_with_ip is not changed
- query_with_ip.current | length >= 2
- query_all_ip_tags_1 is not changed
- query_all_ip_tags_1.current != []
- query_all_ip_tags_1.current | length >= 3
- nt_query_ip_tag is not changed
- nt_query_ip_tag.current == []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we check to make sure the returned objects are the expected objects?
Should we also check to make sure the lengths are exactly what we expect?
eg.

Suggested change
- query_with_vrf.current | length >= 1
- query_with_ip is not changed
- query_with_ip.current | length >= 2
- query_all_ip_tags_1 is not changed
- query_all_ip_tags_1.current != []
- query_all_ip_tags_1.current | length >= 3
- nt_query_ip_tag is not changed
- nt_query_ip_tag.current == []
- query_with_vrf.current | length == 1
- query_with_vrf.current.0 == add_ip_tag_2.current
- query_with_ip is not changed
- query_with_ip.current | length == 2
- query_with_ip.current.0 == nm_update_ip_tag_1.current
- query_with_ip.current.1 == add_ip_tag_3.current
- query_all_ip_tags_1 is not changed
- query_all_ip_tags_1.current != []
- query_all_ip_tags_1.current | length == 3
- query_all_ip_tags_1.current.0 == nm_update_ip_tag_1.current
- query_all_ip_tags_1.current.1 == add_ip_tag_2.current
- query_all_ip_tags_1.current.2 == add_ip_tag_3.current
- nt_query_ip_tag is not changed
- nt_query_ip_tag.current == []

Comment on lines +290 to +296
- query_with_bd.current | length >= 1
- query_with_mac is not changed
- query_with_mac.current | length >= 2
- nt_query_mac_tag is not changed
- nt_query_mac_tag.current == []
- query_all_mac_tags_1 is not changed
- query_all_mac_tags_1.current | length >= 3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Checking the correct objects are returned would also be good here too.
Also can we check for exact lengths?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add modules for Endpoint Policy Tags (fvEpIp,fvEpMacTag) (DCNE-287)
3 participants