Whirr
  1. Whirr
  2. WHIRR-713

Enable programmatic use of BYON via CacheNodeStoreModule

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.8.1
    • Fix Version/s: 0.8.2, 0.9.0
    • Component/s: core
    • Labels:
      None

      Description

      BYON is all well and good if you've already got your list of nodes in a YAML file somewhere, but a pain in the butt if you want to use something else to do your provisioning (or integrate with some other way of getting your nodes, from a service/db/whatever), since to use it programmatically, you need to serialize your nodes to YAML and then load them. This...is annoying.

      There's a CacheNodeStoreModule (to go with the default YamlNodeStoreModule) in jclouds for BYON - it'd be really handy to be able to take advantage of that.

      1. WHIRR-713.patch
        11 kB
        Andrew Bayer
      2. WHIRR-713.patch
        9 kB
        Andrew Bayer
      3. WHIRR-713.patch
        9 kB
        Andrew Bayer
      4. WHIRR-713.patch
        8 kB
        Andrew Bayer
      5. WHIRR-713.patch
        6 kB
        Andrew Bayer
      6. WHIRR-713.patch
        5 kB
        Andrew Bayer

        Activity

        Hide
        Andrew Bayer added a comment -

        Pushed to trunk and branch-0.8.

        Show
        Andrew Bayer added a comment - Pushed to trunk and branch-0.8.
        Hide
        Adrian Cole added a comment -

        +1

        Show
        Adrian Cole added a comment - +1
        Hide
        Andrew Bayer added a comment -

        Stupid git diff. Now actually has everything in the patch.

        Show
        Andrew Bayer added a comment - Stupid git diff. Now actually has everything in the patch.
        Hide
        Andrew Bayer added a comment -

        d'oh, I didn't include the test in the patch. New patch incoming - I wasn't sure how to add CacheNodeStoreModule with the constructor call in defaultModules, fwiw.

        Show
        Andrew Bayer added a comment - d'oh, I didn't include the test in the patch. New patch incoming - I wasn't sure how to add CacheNodeStoreModule with the constructor call in defaultModules, fwiw.
        Hide
        Adrian Cole added a comment -

        Andrew Bayer I think the CacheNodeStoreModule should go into the default modules on overrideApiMetadata. This way, you don't have to special case modules. otherwise looks good (once you find a way to test )

        Show
        Adrian Cole added a comment - Andrew Bayer I think the CacheNodeStoreModule should go into the default modules on overrideApiMetadata. This way, you don't have to special case modules. otherwise looks good (once you find a way to test )
        Hide
        Andrew Bayer added a comment -

        Removed some unused imports from the new ByonClusterControllerTest (and can I just say that I want to punch things over it being BYON in jclouds and Byon in Whirr - consistency, people!) fixing checkstyle errors, and tweaked ClusterSpec's constructor to now have the ability to pass in a byonNodes map. So the setter is mainly just gonna be used for copy and for testing, so I tossed a VisibleForTesting annotation on it.

        Show
        Andrew Bayer added a comment - Removed some unused imports from the new ByonClusterControllerTest (and can I just say that I want to punch things over it being BYON in jclouds and Byon in Whirr - consistency, people!) fixing checkstyle errors, and tweaked ClusterSpec's constructor to now have the ability to pass in a byonNodes map. So the setter is mainly just gonna be used for copy and for testing, so I tossed a VisibleForTesting annotation on it.
        Hide
        Andrew Bayer added a comment -

        Thanks to Adrian, I've got it working - I have to override the BYONApiMetadata. Woo.

        Show
        Andrew Bayer added a comment - Thanks to Adrian, I've got it working - I have to override the BYONApiMetadata. Woo.
        Hide
        Andrew Bayer added a comment -

        I've added a test, and have also made clusterName part of the key for ComputeCache, so you can have multiple ComputeServiceContexts for BYON at one time in one JVM. That said, the test fails - https://gist.github.com/abayer/1b8d8889c2377fa8807a - looks like the problem is that BYONApiMetadata pulls in YamlNodeStoreModule as one of its defaultModules, so I either need to find a way to remove it (which looks problematic at best) or I'll have to construct the ComputeServiceContext more manually. Boo.

        Show
        Andrew Bayer added a comment - I've added a test, and have also made clusterName part of the key for ComputeCache, so you can have multiple ComputeServiceContexts for BYON at one time in one JVM. That said, the test fails - https://gist.github.com/abayer/1b8d8889c2377fa8807a - looks like the problem is that BYONApiMetadata pulls in YamlNodeStoreModule as one of its defaultModules, so I either need to find a way to remove it (which looks problematic at best) or I'll have to construct the ComputeServiceContext more manually. Boo.
        Hide
        Andrew Bayer added a comment -

        listcluster's not a good example, as it actually operates entirely on the state store, rather than the compute service. I'll figure an alternative out.

        Show
        Andrew Bayer added a comment - listcluster's not a good example, as it actually operates entirely on the state store, rather than the compute service. I'll figure an alternative out.
        Hide
        Adrian Cole added a comment -

        I'd say make an offline test that shows this in action (ex listCluster). Add javadoc to suggest that the Map passed in is only ready once (lazy), so changes won't be visible if setBYONNodes are called after the cache is initialized.

        Show
        Adrian Cole added a comment - I'd say make an offline test that shows this in action (ex listCluster). Add javadoc to suggest that the Map passed in is only ready once (lazy), so changes won't be visible if setBYONNodes are called after the cache is initialized.
        Hide
        Andrew Bayer added a comment -

        This patch makes a few changes to ClusterSpec and ComputeCache (and fixes a snafu in my previous ComputeCache patch while I'm at it).

        It adds a new field in ClusterSpec, byonNodes, which is not set via the recipe properties at all. Instead, it's initialized as an empty map, and can be set later via setByonNodes. This provides a way to store the org.jclouds.byon.Nodes that are needed for CacheNodeStoreModule - since these are going to be programmatically determined anyway, we aren't going to want to set them in the config file in the first place.

        Then, in ComputeCache, when we're creating the compute service, we look to see if the provider is "byon" and byonNodes is not empty - if that's the case, we set the modules to include CacheNodeStoreModule using the byonNodes map. I need to do some more testing to be sure that it doesn't get confused by the lack of a YAML file (since YamlNodeStoreModule is still loaded, since it's default for BYON) but I'm fairly certain it works correctly. The result is that the compute service is init'd with a node list that spits out, tada, the Nodes we added to ClusterSpec.

        In actual use (well, in my actual use, at least), the compute service in question will need to be invalidated immediately after use, since if you have multiple byon clusters in a single JVM, it'll get confused due to the equals method not looking at the modules, but I may be able to work around that with dummy override properties...I need to look into that more.

        So yeah, this isn't a finished patch, but it's a good chunk of the way there.

        Show
        Andrew Bayer added a comment - This patch makes a few changes to ClusterSpec and ComputeCache (and fixes a snafu in my previous ComputeCache patch while I'm at it). It adds a new field in ClusterSpec, byonNodes, which is not set via the recipe properties at all. Instead, it's initialized as an empty map, and can be set later via setByonNodes. This provides a way to store the org.jclouds.byon.Nodes that are needed for CacheNodeStoreModule - since these are going to be programmatically determined anyway, we aren't going to want to set them in the config file in the first place. Then, in ComputeCache, when we're creating the compute service, we look to see if the provider is "byon" and byonNodes is not empty - if that's the case, we set the modules to include CacheNodeStoreModule using the byonNodes map. I need to do some more testing to be sure that it doesn't get confused by the lack of a YAML file (since YamlNodeStoreModule is still loaded, since it's default for BYON) but I'm fairly certain it works correctly. The result is that the compute service is init'd with a node list that spits out, tada, the Nodes we added to ClusterSpec. In actual use (well, in my actual use, at least), the compute service in question will need to be invalidated immediately after use, since if you have multiple byon clusters in a single JVM, it'll get confused due to the equals method not looking at the modules, but I may be able to work around that with dummy override properties...I need to look into that more. So yeah, this isn't a finished patch, but it's a good chunk of the way there.

          People

          • Assignee:
            Andrew Bayer
            Reporter:
            Andrew Bayer
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development