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

De-duplicate NSX security groups #180

Merged
merged 2 commits into from Jan 29, 2019
Merged

De-duplicate NSX security groups #180

merged 2 commits into from Jan 29, 2019

Conversation

EleanorRigby
Copy link
Contributor

@EleanorRigby EleanorRigby commented Dec 6, 2018

Description

Security groups for NSX-V has multiple sources

  • vm_type
  • bosh groups in create_vm env hash
  • nsx lbs

We need to gather all nsx-security-groups and deduplicate before calling API to add vm to these nsx-security-groups. It saves execution time. More robust code as we do not need to rely on some fragile logic to not raise error if return code is 203 (vm already exists in Security group).

The issue is caused when upgrading from NSX-V 6.3.x to 6.4.2.
6.4.2 NSX manager's security group API return code is different from 6.3.x

Related PR and Issues

Fixes issue Object vm-32660 is already a member of Security5.

Impacted Areas in Application

Adding NSX-Security groups

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Performance Improvement

@cfdreddbot
Copy link

✅ Hey EleanorRigby! The commit authors and yourself have already signed the CLA.

@EleanorRigby EleanorRigby force-pushed the ta-securityGroups branch 3 times, most recently from 24dd561 to c1be442 Compare December 10, 2018 23:20
@EleanorRigby EleanorRigby changed the title [WIP][Pending Testing] De-duplicate NSX security groups De-duplicate NSX security groups Dec 10, 2018
@EleanorRigby
Copy link
Contributor Author

@nehagjain15 : Can you review it once more?

Copy link
Contributor

@nehagjain15 nehagjain15 left a comment

Choose a reason for hiding this comment

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

test refactoring

 - Add error code 311 to VM-belongs-to-NSX-Security group check
 - Add unit tests
Performance improvement by ignoring all computation if nsxv
config is not specified in global configuration.
As a result if there is no global config for nsxv , all nsxv related
config in cloud config is skipped without raising any error.
@EleanorRigby EleanorRigby dismissed nehagjain15’s stale review January 29, 2019 18:50

Test refactoring is done to sufficient levels. NSX-V is getting obsolete amongst our user and can do with less refactoring (in favor of more important feature development).

@EleanorRigby EleanorRigby merged commit 02f68f5 into master Jan 29, 2019
@EleanorRigby EleanorRigby deleted the ta-securityGroups branch June 3, 2019 17:21
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.

None yet

3 participants