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 complete support for allowed_ips - issue #3 #4

Closed
wants to merge 9 commits into from

Conversation

dn0
Copy link
Member

@dn0 dn0 commented Nov 17, 2016

Implemented via M2N IPAddress.vms field, which lists additional VMs
associated with IP addresses that use the allowed_ips attribute.

    Implemented via M2N IPAddress.vms field, which lists additional VMs
    associated with IP addresses that use the allowed_ips attribute.
@dn0 dn0 added this to the 2.3.1 milestone Nov 17, 2016
Copy link
Contributor

@mbag mbag left a comment

Choose a reason for hiding this comment

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

Please see commnets

@@ -14,6 +14,7 @@ class NetworkIPSerializer(s.Serializer):
"""
ip = s.IPAddressField(strict=True)
hostname = s.CharField(read_only=True, required=False)
hostnames = s.ArrayField(read_only=True, required=False)
Copy link
Contributor

Choose a reason for hiding this comment

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

hostname and hostnames are two pretty similar names, please be more specific so the names of the variables offer additional info what will be stored in this field.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any suggestions?

@@ -932,7 +932,7 @@ class VmDefineNicSerializer(s.Serializer):
allow_mac_spoofing = s.BooleanField(default=False)
allow_restricted_traffic = s.BooleanField(default=False)
allow_unfiltered_promisc = s.BooleanField(default=False)
# allowed_ips = IPAddressArrayField(default=[]) # TODO: IPAddress model association for each additional IP address
allowed_ips = s.IPAddressArrayField(default=[], max_items=8)
Copy link
Contributor

Choose a reason for hiding this comment

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

Magic number.

@dn0 dn0 assigned mbag Nov 25, 2016
@dn0 dn0 removed this from the 2.3.1 milestone Nov 27, 2016
@dn0 dn0 assigned dn0 and unassigned mbag Dec 14, 2016
@dn0
Copy link
Member Author

dn0 commented Dec 24, 2016

Closing this one, There will be a new PR for merge into v2.4

@dn0 dn0 closed this Dec 24, 2016
dn0 added a commit that referenced this pull request Dec 24, 2016
    + added vm_uuid attribute - related to issue #23
dn0 added a commit that referenced this pull request Dec 28, 2016

    Implemented via M2N IPAddress.vms field, which lists additional VMs
    associated with IP addresses that use the allowed_ips attribute.

* Updated changelog - closes #3

* Added API documentation for allowed_ips attribute in vm_define_nic - Issue #3

* Added support for empty nics.*.allowed_ips - Issue #3

* Added variable for maximum allowed_ips - PR #4 - Issue #3

* Fixed updating (=removing) of allowed_ips on VM - related to TritonDataCenter/smartos-live#313 - issue #3

* Updated branch from master + removed from changelog - waiting for 2.4.0 - Issue #3

* Making sure that allowed_ips is a list - Issue #3

* Renamed GET /network/ip output attributes - Issue #3 (old PR #4)
    + added vm_uuid attribute - related to issue #23

* Fixed code readability issues found in PR #27 (issue #3)

* Split _update_vm_ip_association into two functions + added comments explaining operational attributes in VmDefineNicSerializer - issue #3

* Renamed _is_ip_used to _check_ip_usage - Issue # PR #27
YanChii added a commit that referenced this pull request Feb 17, 2019
fix source VM deletion after migration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants