Merge lp://staging/~chad.smith/landscape-client/swift-device-monitoring into lp://staging/~landscape/landscape-client/trunk

Proposed by Chad Smith
Status: Merged
Approved by: Chad Smith
Approved revision: 624
Merged at revision: 616
Proposed branch: lp://staging/~chad.smith/landscape-client/swift-device-monitoring
Merge into: lp://staging/~landscape/landscape-client/trunk
Diff against target: 629 lines (+583/-3)
4 files modified
landscape/message_schemas.py (+6/-2)
landscape/monitor/config.py (+1/-1)
landscape/monitor/swiftdeviceinfo.py (+181/-0)
landscape/monitor/tests/test_swiftdeviceinfo.py (+395/-0)
To merge this branch: bzr merge lp://staging/~chad.smith/landscape-client/swift-device-monitoring
Reviewer Review Type Date Requested Status
Alberto Donato Approve
Chris Glass (community) Approve
Jerry Seutter (community) Approve
Review via email: mp+144396@code.staging.launchpad.net

Commit message

Add separate swift-device-info plugin as a monitor plugin which will read any available swift ring files and pull swift device information from curl to the discovered local swift-recon sevice at http://<local_ip>:<swift_service_port>/recon/diskusage.

Once swift devices are discovered, send this info back to the landscape-server which will process the new swift-device-info messages.

The provider is disabled and logs entered if:
- swift config files don't exist on the server
- no local swift recon service is advertised or running

Description of the change

At long last...

This branch now separates swift-storage device awareness into a separate plugin from the MountInfo plugin. Submitted for review again because we have now separated all swift-related info into separate classes and tests and there was a bit of rework necessary to persist swift_device_info.

There is a new message introduced with the following schema defining all associated swift devices and whether or not they are actively mounted in the swift cluster.

SWIFT_DEVICE_INFO = Message("swift-device-info", {
    "swift-device-info": List(KeyDict({"device": utf8, "mounted": Bool()}))
    })

  The plugin determines whether or not it needs to run by checking to see if /etc/swift/object-server.conf exists on the system.

If we determine we are running on a swift storage node, we use openstack's swift.common.Ring lib to parse a ring file to determine which service ports are active on the local node.

Then we attempt to curl against http://<local_ip>:<service_port>/recon/diskusage which tells us what mounted devices are affiliated with swift. For example:

curl -i http://10.55.63.71:6000/recon/diskusage
HTTP/1.1 200 OK
Content-Type: application/json
Content-Length: 194
Date: Tue, 22 Jan 2013 21:45:07 GMT

[{"device": "loop0", "avail": 3176972288, "mounted": true, "used": 33767424, "size": 3210739712}, {"device": "loop1", "avail": 2103230464, "mounted": true, "used": 33767424, "size": 2136997888}]

There are probably better ways to test this, but I run Jerry's server branch locally to ensure landscape server could process the new client 'mount-info'... 'designated-service' messages.

Then you can build landscape-client; cd src/landscape-client/your-branch; make package;

and scp the debs to you vm instance, pointing /etc/landscape/client.conf back at your public IP address where the server is running, wait about 10 mins and you should see updated mount_info and free_space tracking of available MB of storage on the devices.

Jerry's branch for reference: https://code.launchpad.net/~jseutter/landscape/swift-storage-2

To post a comment you must log in.
Revision history for this message
Alberto Donato (ack) wrote :

Looks good! +1

Just minor comments:

#1:
+ if content is None:
+ return None

Maybe it's not a real case, but an empty string as content would pass this and make json.loads() fail after, it'd be safer to just use "if not content".

#2:
+ def test_get_swift_devices_no_swift_python_libs_available(self):

This test assumes swift libraries are not installed on the machine.

#3:
Please call tests that exercise private methods with the test_wb_ prefix.

review: Approve
Revision history for this message
Chris Glass (tribaal) wrote :

Looks good! +1

Too bad we have to rely on IP addresses to determine which host is which - but I can't think of a better solution right now.

review: Approve
620. By Chad Smith

merge trunk resolve config conflict

Revision history for this message
Jerry Seutter (jseutter) wrote :

+1 looks good

1. s/ swif / swift /

2. mb = lambda... is unused.

review: Approve
621. By Chad Smith

drop unused mp func, and comment correction

Revision history for this message
Chris Glass (tribaal) wrote :

Looks good! +100 again :)

review: Approve
622. By Chad Smith

dial back swiftdeviceinfo plugin frequency from every minute to mountinfo frequencies. Every 5 mins.

Revision history for this message
Alberto Donato (ack) wrote :

Looks good! +1

Just minor comments:

#1:
+ When the plugin has no data, it's various create_X_message()

typo "it's" -> "its"

#2:
+ def test_messages(self):

+ def test_messages_with_swift_data(self):

These two same to perform the same test. The docstring for the latter seems more precise than the other one.

#3:
+ self._swift_device_info_to_persist = None

I think you could set this to an empty list, so that it's valid even if not initialized elsewhere.

review: Approve
623. By Chad Smith

fixup ack minor review comments. Also ability to remove devices from the persist DB if they no longer exist in swift-recon output

624. By Chad Smith

per therve's review: add ability to log and disable swift plugin when node doesn't appear to be a swift-storage participant.

Revision history for this message
Chad Smith (chad.smith) wrote :

all clear with new log/disabled feature. tested on swift and non-swift instances to obtain expected output and disabled logs.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: