search

romana / core

Romana core components - Micro services written in Go.
Apache License 2.0
47 stars 11 forks source link

Ipam allocate endpoints #85

Closed cgilmour closed 8 years ago

cgilmour commented 8 years ago

This PR changes the behaviour of the "legacy allocate IP" path in the IPAM service. Now, it takes a "clusterType" parameter with values either 'kubernetes' or 'openstack'.

For kubernetes, it will expect a tenantName parameter, and compare it to the Name field in the tenant data. For openstack/devstack, it will expect a tenantID parameter, and compare it to the ExternalID field in the tenant data.

cgilmour commented 8 years ago

Tested with kubernetes + romana/kube#10: All steps completed successfully.

cgilmour commented 8 years ago

Tested with devstack + networking-romana#12: Instances came up with addresses.

jbrendel commented 8 years ago

Generally, I don't like our core services to have some environment specific code in them. This should be handled by adaptors.

As far as I can tell, the only difference between the cluster-types is how the tenant is looked up.

Instead of letting IPAM figure out how a tenant for a specific environment is to be looked up, we could instead offer a tenant_by_id and a tenant_by_name query option. That's generic and independent of the environment.

The calling code (in networking_romana or in the CNI plugin) can then use whatever query option works for them. That way, we avoid environment specific code in our core components.

cgilmour commented 8 years ago

So should the IPAM requests be: (same path different parameters) networking-romana: /allocateIP?tenant=id&tenantID=4238979234&segmentName=... CNI: /allocateIP?tenant=name&tenantName=tenant-a&segmentName=... Or (different path, different parameters) networking-romana: /allocateIPUsingTenantID?tenantID==4238979234&segmentName=... CNI: /allocateIPUsingTenantName?tenantName=tenant-a&segmentName=...

Or: both should query topology and tenant, build the real value, post to IPAM /endpoints and extract the IP from the response?

debedb commented 8 years ago

WE can leave it as is and fix up in 0.8.3 with proper calls (driver looking things up via external ID).

jbrendel commented 8 years ago

Why not:

/allocateIP?tenantID=4238979234&segmentName=...

and

/allocateIP?tenantName=tenant-a&segmentName=...

?

Basically, use whatever works for you. No need to declare a type.

cgilmour commented 8 years ago

Going to proceed with this and matching PRs on networking-romana and kube. This needs to be merged to master also.