Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource allocator interface changes #36

Merged
merged 6 commits into from
Apr 6, 2015
Merged

Conversation

mapuri
Copy link
Contributor

@mapuri mapuri commented Mar 27, 2015

  • Introduce the ResourceAllocator and Resource Interfaces to formalize
    resource management
  • Resource allocator provides the 'logically centralized' management of
    different resources.
  • Provide an etcd based implementation of ResourceAllocator, need to
    add locking/synchronization logic.
  • Resource is a state with logic of allocating and deallocating values
    specific to the underlying resource.
  • Provide vlan, vxlan and subnet resource implementations of Resource
    based on the existing gstate functions.

Remaining items to come in this PR:

  • integrate the above implementation with gstate
  • need to move certain methods from the Resource interface to State itself.
  • might need more methods in ResourceAllocator like cross-resource sanity
    checks (like disallowing overlapping vlans across tenants etc)

Allocate() (interface{}, error)
Deallocate(interface{}) error
}

Choose a reason for hiding this comment

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

Resource Interface:

  • State should be moved out of resource interface (as alluded in the code). I'd suggest we specify the State Driver and State Config during Init() and then just call allocate/de-allocate? State Config can define the path/hierarchy given to a specific resource to manage its own hierarchy there in. If State Driver is passed during Init then we'd not need SetStateDriver() call.
  • A Get() or Read() routine is perhaps okay, but would not take State parameter I suppose.
  • SetId() seem to suggest that I can allocate a resource first and then set its id, so then how do I key the resource in between? Should we just say that keep the Id must be specified during NewResource() type of function?
  • ReadAll is a need only because we organize this as resource allocator
  • Resources Dependency: how do we capture that to allocate one resource we need depend on another one. For example, IP allocation may depend on IP Subnet allocation.
  • Resource being global or local: I think we shouldn't explicitly call it out to be global, because allocator package can be used by local/host entity as well. This would mean that agent on the host can perhaps use the resource allocation for local elements that are not cluster-wide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

State should be moved out of resource interface (as alluded in the code).

I am not sure if I understood. I model Resource interface as a (config) State along with associated logic for allocation and de-allocation. So in that sense it seem reasonable to reuse the same State interface instead of defining Read/Write methods of this own. Does the code seem to imply otherwise?

State Config can define the path/hierarchy given to a specific resource to manage its own hierarchy there in. If State Driver is passed during Init then we'd not need SetStateDriver() call.

IMO moving knowledge about hierachy/path outside the State exposes the low level details of the state organization to the caller. This information is better kept inside the State. However, there are some common parts to all the State like Id and StateDriver, which I think can be moved to the State interface itself (as I also point to in my comment). I don't want to have Init() method for State since it seems similar to having a constructor for entire State which will differ between different implementations.

SetId() seem to suggest that I can allocate a resource first and then set its id, so then how do I key the resource in between?

Since the resource get's defined through ResourceAllocator so this is avoided. However, I realize that same effect might appear when I move this method to the State interface. I think to make it more clear we can call it InitId(), InitStateDriver() instead.

ReadAll is a need only because we organize this as resource allocator

Yes, that is one case. I think ReadAll is needed for all State(s) (even though we need to figure how to address it for other data stores). But use ReadAll kind of function else where as well. Example, we depend on ReadAll() for reading existing configurations to compute deltas etc. We can also use it for dumping (in pretty format) all system-state for debugging etc.

Resources Dependency: how do we capture that to allocate one resource we need depend on another one. For example, IP allocation may depend on IP Subnet allocation.

The way I have defined, each resource is defined independently and the information passed during it's definition shall be enough to initialize it. So, in this case the IP resource will require a Subnet to be provided in it's definition, which in turn will require that Subnet to have been allocated.

Resource being global or local: I think we shouldn't explicitly call it out to be global,

Yes, the ResourceAllocator shall be usable for both local and global resource. The difference really comes from which implementation is called where. Example, a local ResourceAllocator need not implement state synchronization algorithms.

Choose a reason for hiding this comment

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

State should be moved out of resource interface (as alluded in the code).

I am not sure if I understood. I model Resource interface as a (config) State along with associated logic for allocation and de-allocation. So in that sense it seem reasonable to reuse the same State interface instead of defining Read/Write methods of this own. Does the code seem to imply otherwise?

Maintaining state is a natural effect of resource allocation/deallocation/init, etc. Then resource is merely an object that can be allocated, deallocated, updated, etc. The associated logic, its state state need not be made as part of the interface. However it needs to know the stateDriver therefore it can be passed during Init()

IMO moving knowledge about hierachy/path outside the State exposes the low level details of the state organization to the caller. This information is better kept inside the State. However, there are some common parts to all the State like Id and StateDriver, which I think can be moved to the State interface itself (as I also point to in my comment). I don't want to have Init() method for State since it seems similar to having a constructor for entire State which will differ between different implementations.

There is a logical hierarchy of various objects, now whether state interprets and maintains the same hierarchy can be left out of the logical hierarchy. Right now the key hierarchy is split among various files and no central place to know the central hierarchy. I 'd not be very concerned if we create the logical hierarchy using dir/file syntax or some other notation.

The way I have defined, each resource is defined independently and the information passed during it's definition shall be enough to initialize it. So, in this case the IP resource will require a Subnet to be provided in it's definition, which in turn will require that Subnet to have been allocated.

Sure. I was hoping that resource allocation can happen in a nested manner i.e. one resource allocation can trigger another resource to be allocated. For example if an load-balancer resource is allocated, it may automatically suggest allocating an IP address from the pool.

Yes, the ResourceAllocator shall be usable for both local and global resource. The difference really comes from which implementation is called where. Example, a local ResourceAllocator need not implement state synchronization algorithms.

ok; let me ask the question differently. If I use vlanResource on the host for local vlan pool management, do I need a different implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The associated logic, its state state need not be made as part of the interface. However it needs to know the stateDriver therefore it can be passed during Init()

I would like to reuse the State as a underlying interface for whatever can hold state. This has certain benefits like it reuses a lot of interfaces without having to define new ones; it provides a uniform way to access the information in the system; it promotes a uniform (State based) design for the major constructs in our system i.e we think about our constructs as State and associated behaviors.

I 'd not be very concerned if we create the logical hierarchy using dir/file syntax or some other notation.

Are you concerned about overlapping paths between different states (as they are defined in different files) in etcd? I guess as of now that hasn't been an issue since the states we have defined have been for different functions like driver, resources etc and that might be the case going forward as well. In my view, the paths are not first order citizens (since we hide them in State). We should make sure that higher level functions that build on these states don't make assumptions on this hierarchy.

For example if an load-balancer resource is allocated, it may automatically suggest allocating an IP address from the pool.

hmm, may be the definition of resource needs to be clarified. What does a load-balancer resource implies in your mind, is it just like an endpoint with IP address derived from a subnet pool allocated for it's network previously (at time of network definition) Or does it also involves auto defining a network and then grabbing an IP in that network? In either case there is a stage of defining the network and subnet allocation can occur at that point, right?

If I use vlanResource on the host for local vlan pool management, do I need a different implementation?

I think it depends on where the ResourceAllocator logic can be run to manage these vlans. If the local vlans are an independent set that can be managed locally then a different implementation of ResourceAllocator (running at netplugin level) might be needed. But if the vlanResource needs knowledge of centrally allocated vlans (as is the case for us today), then this ResourceAllocator will be same as the one for the cluster (i.e. it runs at netmaster level). An example of local/host-level resources that will need a different ResourceAllocator implementation (run at netplugin level) I can think of are network queues/buffers. Vxlan-only networking (i.e. no mixed mode at all) can potentially make use of a host-only ResourceAllocator as well for just allocating local-vlans per host but in that case I don't see benefit of having two implementations since single implementation can allocate same vlans on all hosts without having to worry about overlaps.

@mapuri
Copy link
Contributor Author

mapuri commented Apr 2, 2015

@jainvipin just FYI I have updated the diffs for changes to integrate gstate with resource-allocator interface and incorporating review-comments.

I have tried to push the changes as multiple commits for ease of review. Kindly let me know your feedback.

- Introduce the ResourceAllocator and Resource Interfaces to formalize
  resource management
- Resource allocator provides the 'logically centralized' management of
  different resources.
- Provide an etcd based implementation of ResourceAllocator, need to
  add locking/synchronization logic.
- Resource is a state with logic of allocating and deallocating values
  specific to the underlying resource.
- Provide vlan, vxlan and subnet resource implementations of Resource
  based on the exisitng gstate functions.

Remaining items to come in this PR:
- integrate the above implementation with gstate
- need to move certain methods from the Resource interface to State itself.
- might need more methods in ResourceAllocator like cross-resource sanity
  checks (like disallowing overlapping vlans across tenants etc)
Added a env flag CONTIV_SOE that can be specified with the
make system-test and regress-test targets to stop the test on first
failure and leave the system in same state for debugging. This flag
shall be useful for debugging test issues seen during development
and local testing.
- Also did some cleanup around leftover (dead)code in netd.go which for
  handling gstate events inside netplugin.
- One big change around resource specification is that the checks have
  become strict. Example, a vxlan-range is not allowed to be configured
  if the vnid range exceeds 4k vlans (needed for vxlan functionality).
  On similar lines the default for an empty vlan range (specified in tenant
  config) now results in not creating a vlan resoure at all (unlike reserving
  1-4095 vlan tags earlier.)
  This change allows failing earlier than later. It also requires a more
  explicit configuration (for vlan and vxlan ranges) than assuming
  pre-configured defaults.
@@ -13,7 +13,7 @@
"DefaultNetType" : "vxlan",
"SubnetPool" : "11.1.0.0/16",
"AllocSubnetLen" : 24,
"Vxlans" : "10001-20000",
"Vxlans" : "10001-14000",

Choose a reason for hiding this comment

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

Why reduce vxlan ranges to 4k wide? Does it have to do with bitmap lib? Because this was working for larger nubmer of bitmaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is result of stricter configuration checks that got introduced as part of resource management. Essentially a vxlan resource needs a vlan to be burnt and right now that vlan is required on all hosts (as we place the network everywhere), so the resource check ensures that we have enough free vlans when a vxlan resource is defined otherwise it fails.

Going forward, once we figure a way to say burn host local vlans or not use vlans at all this check can be relaxed.

…mmon structure

Add ReadAll() method to the core.State Interface and provide a generic
implementation for all States
mapuri added a commit that referenced this pull request Apr 6, 2015
Resource allocator interface changes
@mapuri mapuri merged commit 02dd505 into master Apr 6, 2015
@mapuri mapuri deleted the mapuri/resourceAllocator branch April 6, 2015 01:13
dseevr pushed a commit to dseevr-dev/netplugin that referenced this pull request Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants