Skip to content

Fixes #1317: BIGIP> Add support for lsnpool (CGNAT) #1321

Merged
caphrim007 merged 5 commits intoF5Networks:developmentfrom
PrashantSunkari:development
Nov 8, 2017
Merged

Fixes #1317: BIGIP> Add support for lsnpool (CGNAT) #1321
caphrim007 merged 5 commits intoF5Networks:developmentfrom
PrashantSunkari:development

Conversation

@PrashantSunkari
Copy link
Copy Markdown
Contributor

@caphrim007

  • What issues does this address?

Fixes #1317

  • What's this change do?

Adds support for following CGNAT related resources. These are needed to address basic cgnat usecase.

LSNpool collection and resource + tests
LSN log profile collection and resource + tests
Add missing ALG profile - IPsecALG

Helps with following user stories:

User is able to create LSN pool without lsn log profile
User is able to create LSN pool with lsn log profile
User is able to create a virtual and attach a lsn pool and ALG profile

  • Where should the reviewer start?

This work adds new functionality and can be reviewed in any order.
Recommended path:
LSN pool code(f5/bigip/tm/ltm/lsnpool.py) covered lsnpool + lsnpool log profile
Looking at tests (f5/bigip/tm/ltm/test/functional/test_lsnpool.py)
New profile added - f5/bigip/tm/ltm/profile.py

  • Any background context?

I work at F5 networks in CGNAT test org.

  • Tests run
  1. Added functional tests for lsnpool and lsn log profile
  2. Test results against BIGIP with 14.0.0 build
    cmd: py.test test_lsnpool.py --ignore=f5/ --bigip=172.27.77.41 -vs

test_lsnpool.py::TestLSNPool::test_create_no_args PASSED
test_lsnpool.py::TestLSNPool::test_create PASSED
test_lsnpool.py::TestLSNPool::test_create_with_logProfile_without_logPub PASSED
test_lsnpool.py::TestLSNPool::test_create_with_logProfile PASSED
test_lsnpool.py::TestLSNPool::test_refresh PASSED
test_lsnpool.py::TestLSNPool::test_update PASSED
test_lsnpool.py::TestLSNPool::test_exists PASSED
test_lsnpool.py::TestLSNPool::test_stats PASSED
test_lsnpool.py::TestLSNPool::test_create_with_nondefault_mode PASSED
test_lsnpool.py::TestLSNPool::test_create_with_invalid_mode PASSED
test_lsnpool.py::TestLSNPool::test_create_with_member PASSED
test_lsnpool.py::TestLSNPool::test_virtual_with_pool PASSED
test_lsnpool.py::TestLSNPool::test_collection PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_create_no_args PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_create PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_update PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_exists PASSED

Problem: Add support for lsnpool(CGNAT)

Analysis:
Added two things:
- LSN log profile collection and resource + corresponding tests
- Add missing ALG profile - IPsecALG

Tests:
- Added functional tests for lsnpool and lsn log profile
- Test results against BIGIP with 14.0.0 build
cmd: py.test test_lsnpool.py --ignore=f5/ --bigip=172.27.77.41 -vs

test_lsnpool.py::TestLSNPool::test_create_no_args PASSED
test_lsnpool.py::TestLSNPool::test_create PASSED
test_lsnpool.py::TestLSNPool::test_create_with_logProfile_without_logPub PASSED
test_lsnpool.py::TestLSNPool::test_create_with_logProfile PASSED
test_lsnpool.py::TestLSNPool::test_refresh PASSED
test_lsnpool.py::TestLSNPool::test_update PASSED
test_lsnpool.py::TestLSNPool::test_exists PASSED
test_lsnpool.py::TestLSNPool::test_stats PASSED
test_lsnpool.py::TestLSNPool::test_create_with_nondefault_mode PASSED
test_lsnpool.py::TestLSNPool::test_create_with_invalid_mode PASSED
test_lsnpool.py::TestLSNPool::test_create_with_member PASSED
test_lsnpool.py::TestLSNPool::test_virtual_with_pool PASSED
test_lsnpool.py::TestLSNPool::test_collection PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_create_no_args PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_create PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_update PASSED
test_lsnpool.py::TestLSNPoolLogProfile::test_exists PASSED
@caphrim007
Copy link
Copy Markdown
Contributor

@PrashantSunkari PR has test failures

Fixes F5Networks#1317

Problem: Fixing CI pipeline failures

Analysis:
Added following things:
- Fixed test cases failures on 11.5.4
- Fixed the flake8 issues
- Added unit tests

Tests:
-Ran functional and unit tests against 11.5.4, 11.6.1, 12.0.0, 14.0.0
Problem: 11.6.0 : Fixing CI pipeline failures

Analysis:
- LSN pool Stats functional test is fixed
- 'Flake8 f5' failures are addressed
- Syntax error corrected.

Tests:
-Ran functional and unit tests against 11.6.0. Tests passing now.
Problem:  Fixing flake8 failures in python3

Analysis:
- cmd: sed -i sed 's/\r$//' <file_name>
- Ref: [H903] Use only UNIX style newlines (``n``), not Windows style (``rn``)

Tests:
-Ran functional and unit tests.
@caphrim007
Copy link
Copy Markdown
Contributor

@PrashantSunkari are you satisfied with this?

Copy link
Copy Markdown
Contributor

@caphrim007 caphrim007 left a comment

Choose a reason for hiding this comment

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

I'm fine with the PR once the class names have been changed.

There were two extra files included in the PR which I think can be removed; python3.6 and fakefile.txt.

Comment thread python3.6 Outdated
@@ -0,0 +1 @@
/usr/bin/python No newline at end of file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remove this file

Comment thread f5/bigip/tm/ltm/lsnpool.py Outdated
from f5.bigip.resource import Resource


class LSNPools(Collection):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This class can be named Lsn_Pools to avoid the use of _format_resource_name later. The underscore is automatically replaced by a hyphen.

Be sure to reflect the name change of the class throughout the rest of this PR though

Comment thread f5/bigip/tm/ltm/lsnpool.py Outdated
self._meta_data['attribute_registry'] =\
{'tm:ltm:lsn-pool:lsn-poolstate': LSNPool}

def _format_resource_name(self):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not needed after Lsn_Pools change

Comment thread f5/bigip/tm/ltm/lsnpool.py Outdated
return "lsn-pools"


class LSNPool(Resource):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename to Lsn_Pool

Comment thread f5/bigip/tm/ltm/lsnpool.py Outdated
'tm:ltm:lsn-log-profile:lsn-log-profilestate': LSNLogProfile}


class LSNLogProfiles(Collection):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename to Lsn_Log_Profiles

Comment thread f5/bigip/tm/ltm/lsnpool.py Outdated
return "lsn-log-profiles"


class LSNLogProfile(Resource):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename to Lsn_Log_Profile

Problem:  Addressing review comments

Analysis:
- Addressing review comments

Tests:
- Ran functional and unit tests.
- Ran flake8 f5
@PrashantSunkari
Copy link
Copy Markdown
Contributor Author

@caphrim007: Thanks for review comments. Added a new commit addressing the feedback.

@caphrim007
Copy link
Copy Markdown
Contributor

lgtm

@caphrim007 caphrim007 merged commit 81ca562 into F5Networks:development Nov 8, 2017
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.

2 participants