New charms: midonet-host-agent, midonet-agnet, midonet-api

[JF Joly]

New charms for deploying midonet SDN.

[James Page (james-page)]

Merged the three new charm bugs into this bug to make tracking and review a bit easier.

[James Page (james-page)]

General observation across all three charms - its not really required to add copyright headers to all files; a copyright file should be provided for each charm which can detail copyright for individual files if required:

  http://bazaar.launchpad.net/~openstack-charmers/charms/trusty/neutron-gateway/next/view/head:/copyright

This is a machine readable format used across Debian and Ubuntu.

[James Page (james-page)]

metadata.yaml:

'maintainer' field needs to be an email address so that users of the charm know who to contact for bugs/support etc...

[James Page (james-page)]

Based on the proposed change to keystone:

  https://code.launchpad.net/~celebdor/charms/trusty/keystone/midonet_valid_service/+merge/258772

I think the midonet-api charm needs a bit of refactoring so that keystone becomes responsible for tenant/user server and endpoint registration. This will be alot less racey than trying todo it remotely from midonet-api.

[James Page (james-page)]

midonet-host-agent:

_install_rootwrap() installs nova-network to get (I think) the required rootwrap filters.

This feels a little brutal - maybe it might be better to ship the required rootwrap filters in the charm and copy them to the right place once you know the principle charm is nova-compute.

Also:

    if remote_unit and remote_unit.split('/')[0] == 'nova-compute':

is unsafe - there is no guarantee that the principle service will be called nova-compute (that's the charm name) - it could be nova-compute-az1 or something similar.

Its probably better to detect whether nova packages are installed - or maybe just whether the /etc/nova/rootwrap.d folder exists.

Also restarting nova-compute in the subordinate is generally frowned upon - we try to ensure that the charm responsible for the service does restarts. That said the nova-compute charm does not currently expose a good way of doing this (we can do that later), so I'd make the restart conditional on the need to install the rootwrap filter, otherwise every hook execution will restart nova-compute unconditionally.

This will create problems.

[James Page (james-page)]

midonet-host-agent:

hooks/relations.py looks like it wants merging into hooks/utils/relations.py ?

[James Page (james-page)]

midonet-host-agent:

Alot of the hooks contain:

import services
services.manage()

rather than having lots of copied of this, it might be neater to have a single executable py file with this in it, and symlink the required hooks to that.

[James Page (james-page)]

midonet-host-agent:

README.md - Usage section needs a bit of tidy - some of the formatting is a bit off - also;

    juju add-relation midonet-host_agent midonet-api
->
    juju add-relation midonet-host-agent midonet-api

[James Page (james-page)]

OK - so the need for midonet-repository confused me for a while - I see that its just used to propagate out 'source' type information to the other midonet charms for the puppet manifests to use.

This feels a bit awkward; reading the README's I understand the two principle sources to be:

  "upstream opensource"
  "downstream MEM"

presumably combined with a stable/unstable/testing 'stage' and an openstack_release, this maps to a well know set of repositories.

I would recommend that you a) attempt to consolidate these configuration options into a more end-user consumable set of options and b) push them into midonet-api and midonet-host-agent directly - having the extra charm and associated deployment overhead but does not add a huge amount of value and feels counter to the generally modelling of software in juju charms.

[James Page (james-page)]

Re my comments in #9 - including the keys for the midonet repositories within the charm is perfectly acceptable - we have precedent for this in other charms.

so 'downstream' 'stable' 'juno' could just map to a defined set of repositories, verified by the keys in the charm.

[James Page (james-page)]

midonet-api:

This charm provides configuration options for midonet-username/password/project-name - I think these will be superceeded by the keystone identity-service relation once added as this data will be provided by the keystone charm.

I'm also not sure of the intent of the 'api_ip' option - this feels like something the charm should just figure out. I'd probably also drop the api_port option as well - I know some of the other openstack charms do have this type of configuration, but generally it just lets end-users break things faster....

[James Page (james-page)]

FYI I've merged:
  https://code.launchpad.net/~celebdor/charms/trusty/keystone/midonet_valid_service/+merge/258772

The resolution of conflicts for trivial and I thought it would avoid any further round-trips!

[Kevin W Monroe (kwmonroe)]

Changed status to In Progress while Midonet considers James' initial review comments.

[James Page (james-page)]

I also discussed with celebdor on irc - he's reworking based on our conversation and the feedback in this bug report.

[Mark W Wenning (mwenning)]

Toni has or will be posting updates to these merges, setting back to New to get back on the review Q

[Charles Butler (lazypower)]

Moving this bug to in-progress as its not ready for re-review after corresponding with apuimedo in irc. WHen you're ready for another review please change the status of this bug to "fix-committed" and it will be reviewed shortly.

[Mark W Wenning (mwenning)]

Hi Charles and James,

First of all, I want to thank you both for the previous round of reviews and the very enlightening charm development discussions we've had in these past few months.I just merged Lucas' amulet tests to my development branch of the MidoNet specific charms and I would really appreciate it if you two could take a look at our charms so that they may be approved for the partner lab.:

https://code.launchpad.net/~celebdor/charms/trusty/midonet-api/trunk
https://code.launchpad.net/~celebdor/charms/trusty/midonet-agent/trunk
https://code.launchpad.net/~celebdor/charms/trusty/neutron-agents-midonet/trunk

Let me know what you think so that I can push the branches to ~midonet-charmers and we can have this (and the openstack charms that are undergoing review already) in the partner lab and store as soon as possible.

Regards,

Toni

[Charles Butler (lazypower)]

Greetings, after following up with Antoni at the beginning of ODS, I've taken some additional time to review the charms.

I did notice when standing up the cluster using the amulet tests in midonet-api, that I received an error fetching a package for : Unable to locate package python-neutron-plugin-midonet.

The logs preceding this indicate that there was an issue acquiring control of apt and apt is unable to add/update from the datastax and midonet source.

W: Failed to fetch http://repo.midonet.org/midonet/v2015.06/dists/stable/main/binary-amd64/Packages 503 Service Unavailable
Couldn't acquire DPKG lock. Will retry in 10 seconds.

This is reproducible by cloning the charm and executing bundletester from the root on any serverstack instance. I'm filing requests with IS to open these resources during the testing cycle.

This is a candidate for being noted in the README that `repo.midonet.com` will need to be added to any firewall ACLs.

`bundletester -Fvl DEBUG `

I'll resume reviews once I have been able to resolve our firewall issues.

[Ashley Lai (alai)]

From the OIL side the firewall issues are resolved. In OIL network we are able to deploy Midonet successfully but we are blocked on this bug https://bugs.launchpad.net/tpp-midokura/+bug/1522899.

[Antonio Rosales (arosales)]

Per comment 19 development continues around getting the processes running on nova-compute. Thus, I will put this bug in "In Progress." Once that development is complete and the charms are ready for review please update the status to "Fix Committed."

-thanks,
Antonio

[Ashley Lai (alai)]

One comment I have for the Midonet charms is currently the charms use "midonet-release: kilo/midonet-2015.06" option. The charms should get the openstack release from "openstack-origin" so that we can use the same bundle for different openstack releases.

The format is "openstack-origin: cloud:trusty-kilo", it will always be cloud:ubuntuRelease-openstackRelease.

[James Page (james-page)]

Ashley and I discussed on IRC; the semantics of values midonet-release express different intent than openstack-origin, so I think its best not to polute the midonet charms with this configuration option.

[Ashley Lai (alai)]

Bug https://bugs.launchpad.net/tpp-midokura/+bug/1522899 is fixed.

[James Page (james-page)]

Picking up testing and review.

[James Page (james-page)]

Other pertinent merges:

https://code.launchpad.net/~celebdor/charms/trusty/nova-cloud-controller/midonet/+merge/273717

[James Page (james-page)]

and:

https://code.launchpad.net/~celebdor/charms/trusty/neutron-api/midonet/+merge/273772

keystone and nova-compute don't have outstanding merges for midonet support AFAICT

[James Page (james-page)]

Also:

 https://code.launchpad.net/~celebdor/charm-helpers/midonet/+merge/274543

which still needs unit tests as described in the feedback.

[James Page (james-page)]

midonet-api::Amulet Tests

Had a few problems executing the amulet tests.

a) shared_secret -> shared-secret

The nova-cloud-controller charm branch has been updated inline with my previous feedback, so this charm needs an update.

b) neutron-api install failure

unit-neutron-api-0[9625]: 2015-12-17 10:19:06 INFO unit.neutron-api/0.install logger.go:40 Reading package lists...
unit-neutron-api-0[9625]: 2015-12-17 10:19:06 INFO unit.neutron-api/0.install logger.go:40 Building dependency tree...
unit-neutron-api-0[9625]: 2015-12-17 10:19:06 INFO unit.neutron-api/0.install logger.go:40 Reading state information...
unit-neutron-api-0[9625]: 2015-12-17 10:19:06 INFO unit.neutron-api/0.install logger.go:40 E: Unable to locate package python-neutron-plugin-midonet
unit-neutron-api-0[9625]: 2015-12-17 10:19:06 INFO unit.neutron-api/0.juju-log server.go:254 Couldn't acquire DPKG lock. Will retry in 10 seconds.

This actually related to a network access problem that I had - once that was sorted out, I was able to get the neutron-api service running; however it does highlight a bug in the neutron-api charm (apt_update is called in non fatal mode):

    https://bugs.launchpad.net/charms/+source/neutron-api/+bug/1527230

No action required as part of this review - we'll get that fixed under separate cover.

c) switch tests to use landed midonet changes

We still have outstanding changes for neutron-api and nova-cloud-controller, but keystone and nova-compute can use the main charm-store charms now I think.

d) memory requirements for units

Anything with midonet-agent installed appears to need > 2G of RAM to actually operate as the java heap size for midolman is not dynamically configured based on avaliable memory.

I increased the mem constraint on the services that use this sub to work around that in my testenv (default unit only has 1.5G of RAM).

e) wedged hook execution

Once everything was deployed I noticed:

root 9579 0.0 1.1 419792 17128 ? Ssl 12:23 0:00 /var/lib/juju/tools/machine-3/jujud machine --data-dir /var/lib/juju --machine-id 3 --debug
root 9623 0.0 1.3 331612 20420 ? Ssl 12:23 0:00 /var/lib/juju/tools/unit-midonet-api-0/jujud unit --data-dir /var/lib/juju --unit-name midonet
root 19731 0.0 1.5 84144 23392 ? S 12:34 0:00 \_ python /var/lib/juju/agents/unit-midonet-api-0/charm/hooks/host-relation-changed

host-relation-changed was never returning - this causes the amulet tests to hang as the state on the agent sticks a 'executing'

f) midonet-agent on nova-compute

midolman never actually go installed - this is not propagated back to the charm, so the deployment looks OK, but its not actually complete.

2015-12-17 12:30:01 INFO zookeeper-relation-changed ^[[1;31mError: /Stage[main]/Midonet::Midonet_agent::Install/Package[midolman]/ensure: change from purged to present failed: Execution of '/usr/bin/apt-get -q -y -o DPkg::Options::=--force-confold install midolman' returned 100: Reading package lists...
2015-12-17 12:30:01 INFO zookeeper-relation-changed Building dependency tree...
2015-12-17 12:30:01 INFO zookeeper-relation-changed Reading state information...
2015-12-17 12:30:01 INFO ...

[James Page (james-page)]

Looking at the hook hang:

Process 21428 attached
recvfrom(3,

FD 3 is:

3 -> socket:[37498]

[James Page (james-page)]

Looks like a hang in the TunnelHandling() call ? related to this connection:

tcp 0 0 10.5.37.237:48513 10.5.37.237:8080 ESTABLISHED

tcp6 251 0 10.5.37.237:8080 10.5.37.237:48513 ESTABLISHED

any ideas?

[James Page (james-page)]

Antoni is working on a topology change and charm fixes for w/c 18th January.

Parking further review and testing until then.

[Antoni Segura Puimedon (celebdor)]

Latest changes are:

MidoNet API

    https://code.launchpad.net/~celebdor/charms/trusty/midonet-api/trunk

The old complete amulet test has to be updated with fixes like those in:

    https://code.launchpad.net/~james-page/charms/trusty/midonet-api/review-fixes

and MidoNet Agent

    https://code.launchpad.net/~celebdor/charms/trusty/midonet-agent/trunk

Due to the pacemaker/corosync bug, I'm changing the neutron-api, neutron-agents-midonet and midonet-agent deployment to prevent the bug from reproducing in our deployments. I'll post the changes to neutron-api and neutron-agents-midonet so that neutron-api can run in an lxc container and only neutron-agents-midonet is outside with midonet-agent as a subordinate.

[James Page (james-page)]

Hi Toni

Could you also link to your preferred bundle for deployment? Would really
help with the testing and review process.

Cheers

James

On Thu, 21 Jan 2016 at 09:16 Antoni Segura Puimedon <email address hidden>
wrote:

> Latest changes are:
>
> MidoNet API
>
> https://code.launchpad.net/~celebdor/charms/trusty/midonet-api/trunk
>
> The old complete amulet test has to be updated with fixes like those in:
>
> https://code.launchpad.net/~james-page/charms/trusty/midonet-api
> /review-fixes
> <https://code.launchpad.net/~james-page/charms/trusty/midonet-api/review-fixes>
>
> and MidoNet Agent
>
> https://code.launchpad.net/~celebdor/charms/trusty/midonet-
> agent/trunk
>
>
> Due to the pacemaker/corosync bug, I'm changing the neutron-api,
> neutron-agents-midonet and midonet-agent deployment to prevent the bug from
> reproducing in our deployments. I'll post the changes to neutron-api and
> neutron-agents-midonet so that neutron-api can run in an lxc container and
> only neutron-agents-midonet is outside with midonet-agent as a subordinate.
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1453678
>
> Title:
> New charms: midonet-host-agent, midonet-repository, midonet-api
>
> Status in Juju Charms Collection:
> In Progress
>
> Bug description:
> New charms for deploying midonet SDN.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/charms/+bug/1453678/+subscriptions
>

[Antoni Segura Puimedon (celebdor)]

Proposed merge for neutron-api with Liberty support:

https://code.launchpad.net/~celebdor/charms/trusty/neutron-api/midonet_origins/+merge/283630

@James: I'll upload a bundle with constraints that could be used in a cloud later today. The problem is that it will be without neutron-agents-midonet because I'm still finishing the workaround for the pacemaker issue (that means no metadata nor dhcp for the spawned VMs).

[Antoni Segura Puimedon (celebdor)]

https://code.launchpad.net/~celebdor/charm-helpers/liberty/+merge/283707

for the previous merge proposal.

[James Page (james-page)]

https://code.launchpad.net/~celebdor/charm-helpers/liberty/+merge/283707 landed into charm-helpers.

[James Page (james-page)]

Toni

Please can you link to your reworked neutron-agents-midonet branch in this bug report please; I'll be reviewing this midonet charms over the next fews days.

[James Page (james-page)]

Changes to neutron-api under:

https://code.launchpad.net/~celebdor/charms/trusty/neutron-api/midonet_origins/+merge/283630

now landed into next - thankyou!

[James Page (james-page)]

Hi Toni

I took a run through the midonet-api charm this morning; lots of niggles in lint, unit and amulet tests still.

1) lint

$ make lint
hooks/relations.py:96:1: E101 indentation contains mixed spaces and tabs
hooks/relations.py:96:1: W191 indentation contains tabs
hooks/relations.py:97:1: W191 indentation contains tabs
hooks/relations.py:98:1: E101 indentation contains mixed spaces and tabs

2) unit tests

Python2 tests have the right deps installed in the venv, but are not platform agnostic - this probably needs some patching into the unit tests to ensure a consistent unit tests execution environment.

$ make test2
Starting Py2 tests...
PYTHONPATH=hooks .venv2/bin/nosetests -s --nologcapture unit_tests/
juju-log: ERROR: FATAL ERROR: Could not derive openstack release for this Ubuntu release: xenial
E.
======================================================================
ERROR: test_config_change (unit_tests.test_context.HookTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/.venv2/local/lib/python2.7/site-packages/mock/mock.py", line 1305, in patched
    return func(*args, **keywargs)
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/unit_tests/test_context.py", line 81, in test_config_change
    services.manage()
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/hooks/services.py", line 174, in manage
    ost_release = helpers_utils.get_os_codename_install_source(ost_origin)
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/hooks/charmhelpers/contrib/openstack/utils.py", line 178, in get_os_codename_install_source
    error_out(e)
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/hooks/charmhelpers/contrib/openstack/utils.py", line 163, in error_out
    sys.exit(1)
SystemExit: 1

----------------------------------------------------------------------
Ran 2 tests in 0.039s

FAILED (errors=1)

$ make test3
Starting Py3 tests...
PYTHONPATH=hooks .venv3/bin/nosetests -s --nologcapture unit_tests/
E: Could not open lock file /var/lib/dpkg/lock - open (13: Permission denied)
E: Unable to lock the administration directory (/var/lib/dpkg/), are you root?
E: Could not open lock file /var/lib/dpkg/lock - open (13: Permission denied)
E: Unable to lock the administration directory (/var/lib/dpkg/), are you root?
EEE
======================================================================
ERROR: Failure: ImportError (No module named 'yaml')
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/.venv3/lib/python3.5/site-packages/nose/failure.py", line 39, in runTest
    raise self.exc_val.with_traceback(self.tb)
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/.venv3/lib/python3.5/site-packages/nose/loader.py", line 418, in loadTestsFromName
    addr.filename, addr.module)
  File "/home/jamespage/src/charms/midonet/trusty/midonet-api/.venv3/lib/python3.5/site-packages/nose/importer.py", line 47, in importFromPath
    return self.importFromDir(dir_path, fqna...

[James Page (james-page)]

OK - so I was able to successfully deploy midonet-api with keystone and neutron-api, and then create some subnet and network objects in the services.

I'll move on next to test integration with nova-compute and midonet-agent (which I'll review next).

[James Page (james-page)]

Adding:

https://code.launchpad.net/~james-page/charms/trusty/neutron-agents-midonet/nova-metadata-support/+merge/285049

This will superceed the update to nova-cc to support metadata in the nova-cc charm for neutron; nova metadata services will be hosted on neutron-agents-midonet.

[James Page (james-page)]

I've been able to deploy midonet-api, midonet-agent and neutron-agents-midonet alongside the existing openstack charm set; the charms are all using the services framework and all grok OK; I'm sure we'll find further niggles and kinks as we start using them in anger, but that aside I think they are a great v1.

Remaining actions before promulgation across all three charms:

1) Tidy lint
2) Get unit tests mocking fully and passing
3) Tidy amulet tests to use landed changes to other openstack charms and ensure that these run to completion so we can verify deployments.

I'll attach my kilo and juno test deployment bundles to this bug report so people can see what's been tested.

[James Page (james-page)]

Marking 'Incomplete' and assigning bug to Toni for completion of the comments in #42; Toni - once you're happy all is good, set back to new and re-assign to me with an appropriate comment.

Thanks for all of your work on these charms so far - feels real close to promulgation now!

[Antoni Segura Puimedon (celebdor)]

@James:
I pushed new versions of all three charms with unit tests fixed.

I also hope that the amulet tests are fixed. However, due to a bug in the openstack juju provider that is not opening the security groups for the open ports, the tunnel zone handling that happens via midonet-api port 8080 could not be verified. I'll try to get access to some aws environment (as in my maas env I don't have nearly enough machines to test it).

[James Page (james-page)]

I've performed final review and testing using amulet over the last few days; all looking good so have promulgated all three charms to the charm store - will injest in the next few hours.

The charms make some limited use of status - it would be nice as a follow up for this to be a bit more in-depth (assessing the status of relations at the end of each hook execution), and to provide implementations of the 'update-status' hook which runs every 5 mins to validate that daemons are running.

[James Page (james-page)]

Also the propagation of problems encountered in the underlying puppet modules up into the charm could be better; I had some challenges figuring out why stuff was not installed when we have some issues in our test environment with access to repositories.