Regression: Proprietary code needed to use NetApp with Cinder and Manila

[Thomas Bechtold]

NetApp recently released under a proprietary license netapp_lib [1] which is used in Cinder and Manila to talk to the NetApp storage via it's REST Api.
This is a regression compared to the Kilo release. In the Kilo release, the code to talk to the NetApp storage was integrated in Cinder and Manila and released under a Apache-2.0 license.

The changes done to use the netapp_lib code are:

- cinder: https://review.openstack.org/#/c/215700/
- manila: https://review.openstack.org/#/c/215353/

Using NetApp with Cinder and/or Manila now requires the user to download an extra lib which is also not in the global-requirements.txt file.
For distributors netapp_lib is not shippable due to the license so the user experience decreased compared to the Kilo integration.
I know that other vendors are doing something similar but in this case we have already working code which is under a apache-2.0 license. So there is no need to use the netapp_lib code.

My suggestion is to revert both commits and just use the code released under the Apache-2.0 license.

[1] https://pypi.python.org/pypi/netapp-lib

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/227268

[John Griffith (john-griffith)]

So yes, there are a number of vendors using external pypi libs, however when reviewing those additions the first thing we've usually done is verify that the licensing is open source and compatible. In this case that was not checked it appears and given that the license is proprietary I think that creates a problem in some cases.

I'm not certain, but we (OpenStack) as well as distros do not need to install/distribute this package, but can leave it to the end-user no? I'd need to defer to folks that are more well versed in licensing than myself but it seems to go against the philosophy or our community so I view it as a critical bug and support either a revert or if Netapp is willing a change of the license.

[Thomas Bechtold (toabctl)]

@John: When deploying i.e. Manila, you *need* to install the lib on the node where the manila-share process is running. Otherwise you can not use the manila (NetApp) backend. So this is something a deployer needs to install. And distributions want to install it to not make the deployment of OpenStack more complicated than it is already.

[John Griffith (john-griffith)]

@Thomas,

Got ya, thanks! To be honest, the bottom line is that regardless if there are legal issues or not, OpenStack has always made a point of fully open-source, NOT just the components in tree; everything about it.

This introduces a confusing case that goes against that philosophy. It also introduces a case where as you say it's a regression because we go from being full open source to now having a component that is proprietary. Seems like a step in the wrong direction IMHO.

The trick is that even though that patch was landed late in the cycle, it seems there were quite a few changes during feature freeze, so the revert is a bit of mess in terms of merge-conflicts, making things slightly more difficult unless we just remove the NetApp drivers altogether which I certainly don't like the idea of doing.

I've asked some folks from NetApp to take a look at fixing up the merge conflicts, we'll see what happens.

[Jeremy Stanley (fungi)]

It's also disappointing to see an OpenStack member company producing software which uses Oslo libraries, has its packaging based on PBR, et cetera... but then redistributing it under a very restrictive proprietary license agreement. Worse, it contains code _deleted_ from the openstack/cinder repository without any attribution. While that may be legal (I am not a lawyer) since the original copyright holders of those contributions are probably the same as those redistributing the proprietary replacement, it's pretty distasteful to basically move community-maintained source code out of OpenStack and into a vendor's non-freely-licensed library.

[Jeremy Stanley (fungi)]

Also, it took me about 5 minutes to find at least some source code from cinder which is not of NetApp provenance[*], yet was copied into the netapp_lib source with no attribution (from cinder/volume/drivers/netapp/eseries/client.py to netapp_lib/api/rest/rest.py).

[*] https://review.openstack.org/189724

[Duncan Thomas (duncan-thomas)]

We've reverted / removed calls to libraries before due to similar license issues, it definitely makes life difficult for distributors. Saying 'the end user can download it themselves' is not much of an answer, since they then need a way to plug what they've just downloaded into their chosen distribution - fewer and fewer people are rolling their own stuff now. I vote for a revert, and I'm trying to get a statement from our legal department about whether this affects our distribution adversely.

[Duncan Thomas (duncan-thomas)]

Should we discuss third party libs, cinder driver shims and licensing at the summit?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/227524

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/227532

[OpenStack Infra (hudson-openstack)]

Change abandoned by Thomas Bechtold (<email address hidden>) on branch: master
Review: https://review.openstack.org/227268
Reason: NetApp will provide another patch to solve the license issue.

[Jeremy Stanley (fungi)]

The netapp_lib 2015.9.25 wheel on PyPI now includes a copy of the Apache license and acknowledgement of the origin of some of the software in a new NOTICE.txt file.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/227968

[OpenStack Infra (hudson-openstack)]

Change abandoned by John Griffith (<email address hidden>) on branch: master
Review: https://review.openstack.org/227524
Reason: We've got a revert of the lib, so all is well.

Thanks!!

[OpenStack Infra (hudson-openstack)]

Change abandoned by John Griffith (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/227532
Reason: NetApp has a version of the revert that fixes the merge conflicts in flight, so we can abandon these fortunately.

https://review.openstack.org/#/c/227427/

[Joshua Harlow (harlowja)]

A little sad by this, but not sure whats legal or isn't.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/227968
Committed: https://git.openstack.org/cgit/openstack/manila/commit/?id=f925c049007fbd5ec71a4ec59de1120d35ad66af
Submitter: Jenkins
Branch: master

commit f925c049007fbd5ec71a4ec59de1120d35ad66af
Author: Clinton Knight <email address hidden>
Date: Thu Sep 24 17:27:04 2015 -0400

    Revert netapp_lib dependency in NetApp cDOT Manila drivers

    The community asked us to remove the dependency on a PyPI
    library with a proprietary license. This commit does that
    while preserving some minor refactoring that was done at the
    same time.

    Reverts commit ca6168d01bea405718bbf6a36d4bb75c23d1b783.

    Closes-Bug: #1499334
    Change-Id: I1a4b1b2b2151bb222ae5c77b9b317e7505255067

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/227427
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=ff81307ca4c796a66f4bd584b4633db07dcf2a16
Submitter: Jenkins
Branch: master

commit ff81307ca4c796a66f4bd584b4633db07dcf2a16
Author: Goutham Pacha Ravi <email address hidden>
Date: Fri Sep 25 01:52:43 2015 -0400

    Revert use of netapp_lib from NetApp Drivers

    This patch cleanly reverts the changes made via the
    commit: e681ba2a99995dcd999e6539bb1222f8a1ac8adc
    cleanly and mitigates the conflicts that would
    occur with git-revert on the said commit.

    The revert is solely for changes pertaining to the use
    of the external library, netapp_lib. Minor code refactors
    from the prior change are retained.

    Unit test coverage has been increased for ZAPI and REST
    interface code in netapp/dataontap/client/api.py and
    netapp/eseries/client.py.

    Closes-Bug: #1499334
    Change-Id: Icead7e168e1c7187840de87c69365d26aedd5924

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/228159

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/228160

[OpenStack Infra (hudson-openstack)]

Change abandoned by Thomas Bechtold (<email address hidden>) on branch: stable/liberty
Review: https://review.openstack.org/228159
Reason: Needs some other backports, first. Redo the cherry-pick later.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/228159
Committed: https://git.openstack.org/cgit/openstack/manila/commit/?id=b28c8fccf93ef2adb08685e3efa1b5d72b0c7f94
Submitter: Jenkins
Branch: stable/liberty

commit b28c8fccf93ef2adb08685e3efa1b5d72b0c7f94
Author: Clinton Knight <email address hidden>
Date: Thu Sep 24 17:27:04 2015 -0400

    Revert netapp_lib dependency in NetApp cDOT Manila drivers

    The community asked us to remove the dependency on a PyPI
    library with a proprietary license. This commit does that
    while preserving some minor refactoring that was done at the
    same time.

    Reverts commit ca6168d01bea405718bbf6a36d4bb75c23d1b783.

    Closes-Bug: #1499334
    Change-Id: I1a4b1b2b2151bb222ae5c77b9b317e7505255067
    (cherry picked from commit f925c049007fbd5ec71a4ec59de1120d35ad66af)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/228160
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=bdfe5fa04e8e452c7262f265255b272f8fcfaa1d
Submitter: Jenkins
Branch: stable/liberty

commit bdfe5fa04e8e452c7262f265255b272f8fcfaa1d
Author: Goutham Pacha Ravi <email address hidden>
Date: Fri Sep 25 01:52:43 2015 -0400

    Revert use of netapp_lib from NetApp Drivers

    This patch cleanly reverts the changes made via the
    commit: e681ba2a99995dcd999e6539bb1222f8a1ac8adc
    cleanly and mitigates the conflicts that would
    occur with git-revert on the said commit.

    The revert is solely for changes pertaining to the use
    of the external library, netapp_lib. Minor code refactors
    from the prior change are retained.

    Unit test coverage has been increased for ZAPI and REST
    interface code in netapp/dataontap/client/api.py and
    netapp/eseries/client.py.

    Closes-Bug: #1499334
    Change-Id: Icead7e168e1c7187840de87c69365d26aedd5924
    (cherry picked from commit ff81307ca4c796a66f4bd584b4633db07dcf2a16)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/235282

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/235328

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/235282
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=0f7d289a2899082802499a498cba34a3abca321d
Submitter: Jenkins
Branch: master

commit 61026d4e4f2a58dd84ffb2e4e40ab99860b9316a
Author: Davanum Srinivas <email address hidden>
Date: Tue Oct 6 18:45:12 2015 -0700

    Squashed commit of WebOb 1.5 and oslo.db fixes

    Add testresources and testscenarios used by oslo.db fixture

    If we use oslo.db fixtures, we'll need these 2 packages or
    the next version of oslo.db release will break us.

    Closes-Bug: #1503501
    Change-Id: I8facdaf69c79b1b1ae4f9f64e9856e12f14440ed
    (cherry picked from commit ec40c3b6ddbfa4912ca6e612558efaef6043f3ae)

    Fix test_misc for WebOb 1.5

    WebOb 1.5 was released at 2015-10-11. With this new version,
    webob.exc.WSGIHTTPException() constructor now fails with a KeyError
    when the HTTP status code is 0.

    test_exceptions_raise() of test_misc tries to instanciate all
    exceptions of cinder.exception. The problem is that
    ConvertedException uses a default HTTP status code of 0.

    Modify the default HTTP status code of ConvertedException to 400 to
    fix the unit test. The bug is only in the test,
    cinder/api/openstack/wsgi.py copies an existing HTTP code:

        Fault(exception.ConvertedException(code=ex_value.code, ...)

    Closes-Bug: #1505153
    Change-Id: I1aec8038774828d48da4b0e831b390e33243809a
    (cherry picked from commit 867fccf833ffc597aa986cb6ff1b3b5c1101b9ba)

    Change default Exception code to 500

    As a part of the fix for LP#1505153, the invalid
    response of 0 was changed to 400 to fix a running
    issue with the new version of WebOb.

    It was pointed out after the fact however that a
    500 might be more appropriate here.

    Additionally, other projects have implemented a 500
    as the default so for the sake of consistency we should
    consider doing the same.

    This patch just changes the 400 to a 500 as the default
    code.

    Conflicts:
            cinder/tests/unit/test_exception.py

    NOTE(mriedem): The conflict is due to 17802086f not being
    in stable/liberty.

    Change-Id: Ie486dc49c927f9b50f07c1fc562e89c090924a40
    Closes-Bug: #1505488
    (cherry picked from commit 9a7cbe540c94d54194194f4747a52b8211f6372e)

commit 5268a8d61beee1b18d14fd3738e763a0a61b00f1
Author: Xing Yang <email address hidden>
Date: Wed Sep 23 11:22:43 2015 -0400

    Fix VMAX live migration problem

    Live migration fails for VMAX because the host is not taken into
    account when determining whether a volume is masked or not. For
    live migration it is necessary to know which host a volume is masked
    to. This patch fixes the problem by checking which host the volume
    is masked to.

    Change-Id: I693e29f7b26145bd1fd357b7d98377e26bfdfed8
    Closes-Bug: #1498964
    (cherry picked from commit 794e27c78255c94084b54821d44a36fd8f2096d7)

commit 2c09897f569a786998fd1ab50e42d81e9a2093a0
Author: OpenStack Proposal Bot <email address hidden>
Date: Sat Oct 3 06:22:46 2015 +0000

    Imported Tr...

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/235328
Committed: https://git.openstack.org/cgit/openstack/manila/commit/?id=ec94d6929ea8e1b4ce10dc91cd4954ece808668c
Submitter: Jenkins
Branch: master

commit f1eded1fbcaf309e1c9a4be3f8a14bd25daa3e46
Author: Gaurang Tapase <email address hidden>
Date: Mon Oct 12 20:47:19 2015 +0530

    Fix usage of dependencies

    Manila is broken is threee places, so fix them:
    1) test 'test_misc' with WebOb 1.5

    WebOb 1.5 was released at 2015-10-11. With this new version,
    webob.exc.WSGIHTTPException() constructor now fails with a KeyError
    when the HTTP status code is 0.

    test_exceptions_raise() of test_misc tries to instantiate all
    exceptions of manila.exception. The problem is that
    ConvertedException uses a default HTTP status code of 0.

    Modify the default HTTP status code of ConvertedException to 400 to
    fix the unit tests.

    2) Add dependency for 'testresources' that is required by migration
    tests.

    3) Remove 2 unit tests related to testing of oslo.policy lib
    functionality that should not be tested in Manila. It started failing
    because under-the-hood behaviour was changed in new realese 0.12.0

    Closes-Bug: #1505153
    Closes-Bug: #1505374

    (cherry picked from commit 9c99814ce5943bd4c33bf3650b832666e31b3411)

    -- squashed with another change to get tests to pass on stable/liberty --

    Fix broken unit tests

    With release of six module version 1.10.0 several our unit tests
    started to fail because of usage of not strict constructions.

    Changes:
    1) Manila unit test
    "manila.tests.share.test_api.ShareAPITestCase.test_extend_quota_error"
    used str for int substitution. So, use int data for int substitution.

    2) Module 'manila.share.drivers.hp.hp_3par_mediator' was using
    LOG.exception function when no traceback were exist it led to
    AttributeError on py34. So, replace all usages of 'LOG.exception'
    with 'LOG.error' where no raised exceptions exist.

    Change-Id: Ic5b37bfb9d939d03f6ff68bc53d134bf9e5f996e
    Closes-Bug: #1503969
    (cherry picked from commit f38b8d4efd1f68f4ea29747f7377e0936f61d89c)

    --

    Change-Id: I0f28f3c3fb2c7eec1bafc3a617344990f86810cf

commit 151b691bb2d4baa436913924df60b8c197f91463
Author: Valeriy Ponomaryov <email address hidden>
Date: Thu Oct 1 12:42:05 2015 +0300

    Fix display of availability-zone for manila-manage command

    Commands "manila-manage service list" and "manila-manage host list"
    were displaying availability zone instance instead of its name.

    Such bug appeared after implementation of Availability zone model.
    So, fix it by providing 'name' field of availability zone model.

    Change-Id: I14c3451380df01853183aed265344b1783c95939
    Closes-Bug: #1499677

commit 77455a5ac6be828e8dfd3e75566eaff2823595d4
Author: Csaba Henk <email address hidden>
Date: Wed Sep 30 14:57:00 2015 +0200

    glusterfs_native: use dynamic-auth option if available

    With dynamic-auth restarting the volume is not necessary
    in deny_access.

    Change-Id: Ic25af1795c279b34370...

[Thierry Carrez (ttx)]

This issue was fixed in the openstack/cinder 8.0.0.0b1 development milestone.

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/manila 2.0.0.0b1 development milestone.