Make worker node volume type and IOPS configurable

[deliahu]

Description

Allow users to configure worker node volume type and IOPS in cluster.yaml. Potential field names can be volume_type and volume_iops.

• Discover what's allowed and how to validate here and here
• eksctl config: https://eksctl.io/usage/schema/

[tthebst]

hi,

I'm currently looking at this. Do you also want to update the pricing depending on the storage type or is the price just a rough guidance?

[deliahu]

@tthebst awesome! Yes, we would like to update the cluster pricing depending on the storage type if that's feasible. Good catch!

Here is where we add it to the CLI output: https://github.com/cortexlabs/cortex/blob/master/cli/cmd/lib_cluster_config.go#L296

That pulls from this file:
https://github.com/cortexlabs/cortex/blob/master/pkg/lib/aws/resource_metadata.go#L3358

Which is generated by running this file:
https://github.com/cortexlabs/cortex/blob/master/pkg/lib/aws/gen_resource_metadata.py#L128
(you can run it with go generate from the pkg/lib/aws/ directory)

So the path to take would probably be to modify gen_resource_metadata.py to add the necessary pricing data to resource_metadata.go, and then consume that data in lib_cluster_config.go (based on the user's configuration).

Does that all make sense? Let us know if you have any other questions!

[tthebst]

@deliahu

Yes makes sense. And I think I know the general approach. But I encountered one problem with Provisioned IOPS SSD (io1) , which is priced by GB an IOPS. The AWS pricing API doesn't provide this pricing I think (which is strange , see). Do you have any idea to resolve this?

[deliahu]

@tthebst Sorry, I just assumed that the pricing data would be there, but I did not confirm!

At first I couldn't find it, but after digging in a bit more, I may have found it (I have not cross-referenced with the AWS web console).

I found this in the JSON:

"ZS6E9ESZZKGW2ZVG" : {
  "sku" : "ZS6E9ESZZKGW2ZVG",
  "productFamily" : "System Operation",
  "attributes" : {
    "servicecode" : "AmazonEC2",
    "location" : "US West (Oregon)",
    "locationType" : "AWS Region",
    "provisioned" : "Yes",
    "group" : "EBS IOPS",
    "groupDescription" : "IOPS",
    "usagetype" : "USW2-EBS:VolumeP-IOPS.piops",
    "operation" : "",
    "servicename" : "Amazon Elastic Compute Cloud",
    "volumeApiName" : "io1"
  }
},

The SKU can be used to find this in the json:

"ZS6E9ESZZKGW2ZVG" : {
  "ZS6E9ESZZKGW2ZVG.JRTCKXETXF" : {
    "offerTermCode" : "JRTCKXETXF",
    "sku" : "ZS6E9ESZZKGW2ZVG",
    "effectiveDate" : "2020-04-01T00:00:00Z",
    "priceDimensions" : {
      "ZS6E9ESZZKGW2ZVG.JRTCKXETXF.6YS6EN2CT7" : {
        "rateCode" : "ZS6E9ESZZKGW2ZVG.JRTCKXETXF.6YS6EN2CT7",
        "description" : "$0.065 per IOPS-month provisioned - US West (Oregon)",
        "beginRange" : "0",
        "endRange" : "Inf",
        "unit" : "IOPS-Mo",
        "pricePerUnit" : {
          "USD" : "0.0650000000"
        },
        "appliesTo" : [ ]
      }
    },
    "termAttributes" : { }
  }
},

Is that the data you are looking for?

If so, feel free to add whichever attribute filters you think make sense to find the SKU, similar to how we filter for the other products.

[tthebst]

Oh, thanks for the headsup. I searched under storage.

[tthebst]

I'm currently encountering an issue. I generated a new resource metadata file, which works. I'm now working on the config validations. I generated the metadata file such that the IOPS pricing is zero for all storages expect io1 and standard. Now when i want to validate volume_iops this depends on volume_type. With the current validation system, it is not possible to reference other config values in the validation except the one to be validated. I don't really know where the best place to validate volume_iops. Do you have any opinion on this?
You can also look at my fork to see the my changes.
I thought maybe one could create a top level volume config option like:

volume_conf:
   - volume_type
   - volume_iops

Tim

[deliahu]

@tthebst I'm glad you reached out! I've given it some thought, and I think this could make sense regarding the fields:

instance_volume_size: 50  # already exists
instance_volume_type: general_purpose  # or gp2 if you prefer the short hand
instance_volume_iops: 3000  # only applicable for instance_volume_type: provisioned_iops

On the go side of things, instance_volume_type should be an "enum", which we have implemented like this (which refers to this file, which you can copy-paste with minor modifications).

instance_volume_iops should be an int pointer, which is optional for the user to pass in. In clusterconfig.Validate(), you can add a check: if the volume type is provisioned_iops and instance_volume_iops is nil, you can set it to 3000; if the volume type is not provisioned_iops and instance_volume_iops is not nil, you return an error.

Does that make sense?

[tthebst]

@deliahu I'm now finalizing the changes. Just one more thing. The manager node I assume should still use "gp2" as storgage type. Is this correct?

Also where do you write your tests?

Tim

[deliahu]

@tthebst Awesome, I'm looking forward to checking it out!

Yes, let's stick with gp2 for the manager node.

Go tests are in the same directory as the source files, and in our case we've been keeping them in the same package too. We don't have full test coverage (especially on the relevant code for this PR), but here is an example of one of our tests if you'd like to add one: https://github.com/cortexlabs/cortex/blob/master/pkg/lib/zip/zip_test.go