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

Usage enforcement upstream #31

Draft
wants to merge 1 commit into
base: upstream
Choose a base branch
from
Draft

Conversation

jakecoll
Copy link

No description provided.

@jakecoll jakecoll requested a review from diurnalist May 28, 2020 02:19
@jakecoll jakecoll force-pushed the usage-enforcement-upstream branch 14 times, most recently from 9b90a6e to 8ebb6d3 Compare May 29, 2020 15:33
Copy link

@diurnalist diurnalist left a comment

Choose a reason for hiding this comment

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

This is looking very nice. Smart to move much of the base plugin stuff into a base class; that will really help in the future. Left some comments. The main thing that I am anticipating this needing is unit test coverage for the filter mechanism and the filters themselves. I'd start with that, and then we can see what to do about the base plugin and the new functions this adds to all the plugins.

blazar/enforcement/__init__.py Outdated Show resolved Hide resolved
def get_reservation_allocations_by_fip_ids(fip_ids, start_date, end_date,
lease_id=None, reservation_id=None):
session = get_session()
border0 = start_date <= models.Lease.end_date

Choose a reason for hiding this comment

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

What is the type of start_date? I'm surprised this works; I would have thought that you'd need to do it like:

models.Lease.end_date >= start_date

... because this looks like it has to be an operator overload, the way it's working here. And I thought those used the implementation defined by the first item's type.

Copy link
Author

Choose a reason for hiding this comment

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

Yea, I was never too sure myself. Upstream uses this border implementation for get_reservation_allocations_by_host_ids and I implemented it this way for floating ips for consistency.

blazar/enforcement/enforcement.py Outdated Show resolved Hide resolved
blazar/enforcement/enforcement.py Outdated Show resolved Hide resolved
blazar/enforcement/enforcement.py Outdated Show resolved Hide resolved
blazar/manager/service.py Show resolved Hide resolved
blazar/manager/service.py Show resolved Hide resolved
blazar/manager/service.py Show resolved Hide resolved
start = datetime.datetime.utcnow()
end = datetime.date.max

# To reduce overhead, this method only executes one query

Choose a reason for hiding this comment

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

I don't quite get this comment - what is the alternative?

Copy link
Author

Choose a reason for hiding this comment

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

This was a comment left over from a copy and paste from query_host_allocations to implement query_fip_allocations. I believe it's referring to the query in the db layer.

blazar/plugins/instances/instance_plugin.py Outdated Show resolved Hide resolved
@jakecoll jakecoll force-pushed the usage-enforcement-upstream branch 3 times, most recently from 0c0e345 to 7a62f4c Compare June 19, 2020 12:20
Implements filter based architexture for usage enforcement. Two filters
are included initially. MaxReservationsLengthFilter allows operators
to define the maximum reservation length of a lease in seconds.
The ExternalServiceFilter sends lease values and allocation
candidates to an external HTTP service where policies can be defined.
All filters must be enabled in blazar.conf in an enforcement group.
@diurnalist diurnalist marked this pull request as draft August 21, 2020 14:40
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