Skip to content
This repository was archived by the owner on Jul 16, 2020. It is now read-only.

BAT enhancements for storage#997

Merged
tpepper merged 7 commits into
ciao-project:masterfrom
rbradford:storage-bat-enhancements
Jan 9, 2017
Merged

BAT enhancements for storage#997
tpepper merged 7 commits into
ciao-project:masterfrom
rbradford:storage-bat-enhancements

Conversation

@rbradford

Copy link
Copy Markdown
Contributor

This PR adds some new BAT tests for storage that test the different source types whilst also testing the new size reporting and calculation.

This identified some issues also fixed by this PR.

@kaccardi to review ciao-controller change as owner
@markdryan to review bat changes as owner

The source type for cloning the volume from should be specified with
-source_type not -source-type.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
When creating a volume from image using the Open Stack volume API it is
necessary to derive the new rbd image from the rbd image snapshot of the
Ciao image.  This is the same method as volumes created by booting
instances.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
@coveralls

coveralls commented Jan 6, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.005%) to 66.943% when pulling 2778085 on rbradford:storage-bat-enhancements into 225e633 on 01org:master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 66.938% when pulling 2778085 on rbradford:storage-bat-enhancements into 225e633 on 01org:master.

Add tests to test creating volumes from images and from other volumes
using "ciao-cli volume add".

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
Address some mismatches between the numbers of arguments used in the
format string relative to the number of arguments given.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
@rbradford rbradford force-pushed the storage-bat-enhancements branch from 2778085 to 7d60bc1 Compare January 6, 2017 18:04
@coveralls

coveralls commented Jan 6, 2017

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.009%) to 66.948% when pulling 7d60bc1 on rbradford:storage-bat-enhancements into 225e633 on 01org:master.

@tpepper tpepper left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What's the rationale for everything being a snapshot? Is this needed because each user needs to be able to write, and you can only have one writer?

break
}

volumeUUID, err := bat.AddVolume(ctx, "", imageUUID, "image", &bat.VolumeOptions{})

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.

If there are no images this will probably fail which would be a bit misleading. Might be best to skip the rest of the test if imageUUID == "" or len(images) == 0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated version of the patch now creates a test image to use.

@markdryan

Copy link
Copy Markdown
Contributor

Works fine for me although I did have one comment about one of the tests. Thanks for the go vet fixes. I'm working on a PR that enables static anaylsis on the BAT tests, and fixes a few other small issues.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 66.938% when pulling 4988fc7 on rbradford:storage-bat-enhancements into 225e633 on 01org:master.

1 similar comment
@coveralls

coveralls commented Jan 9, 2017

Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 66.938% when pulling 4988fc7 on rbradford:storage-bat-enhancements into 225e633 on 01org:master.

By moving this function from the image_bat package to the bat package it
can be used by other BAT test packages.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
This removes the requirement that the cluster is populated with at least
one image before running BAT.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
Throughout the ciao code base we're trying to stardise on MiB rather
than MB.

Signed-off-by: Rob Bradford <robert.bradford@intel.com>
@rbradford rbradford force-pushed the storage-bat-enhancements branch from 4988fc7 to 90f91a7 Compare January 9, 2017 14:51
@rbradford

rbradford commented Jan 9, 2017

Copy link
Copy Markdown
Contributor Author

@tpepper:

What's the rationale for everything being a snapshot? Is this needed because each user needs to be able to write, and you can only have one writer?

We always use a snapshot for images. This fix should really have been part of #811 but we didn't have any test coverage of "ciao-cl volume add -source_type image -source " until this BAT test so it slipped through. Without this change that command fails as ciao-conttoller tries to find a file on disk.

Using snapshots gives us CoW behaviour of images allowing the underlying blocks to be shared across different volumes. This both saves space but also improves the cachability as more blocks are common across volumes.

@coveralls

coveralls commented Jan 9, 2017

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.04%) to 66.917% when pulling 90f91a7 on rbradford:storage-bat-enhancements into 6112b5b on 01org:master.

@tpepper tpepper merged commit 755eab7 into ciao-project:master Jan 9, 2017
@rbradford rbradford deleted the storage-bat-enhancements branch February 21, 2017 15:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants