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.

Sign up for GitHub

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

Change site and machine namespace to use continent name #110

Open
wants to merge 2 commits into
base: upstream
Choose a base branch
from
Open

Change site and machine namespace to use continent name #110

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2306b5e
Change site and machine namespace to use continent name
jouvin Jul 13, 2018
584c978
Export the hub name as sysloc/region in the object plenary
jouvin Jul 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions lib/aquilon/worker/templates/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -629,18 +629,20 @@ def transaction(self, verbose=False):


def add_location_info(lines, dblocation, prefix=""):
Copy link

Choose a reason for hiding this comment

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

In general we agree - it is a good fix - but we have to make huge template change change to allow this to go out cause we have some sysloc template code that would start failing if we just roll this out... I suggest to create a separate PR for this if we want to release the other changes sooner :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll look it if makes sense but will probably not do anything before end of August anyway... What's your timeframe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@urbonegi Looking in more details, I don't understand this comment. You mean that adding sysloc/region will make your existing code failing? Else the modification doesn't change anything to the current template contents (except a minor sysloc property reordering, with rack name before the rack info).

# FIXME: sort out hub/region
for parent_type in ["continent", "country", "city", "campus", "building",
"bunker"]:
for parent_type in ["hub", "continent", "country", "city", "campus", "building",
"room", "bunker"]:
dbparent = getattr(dblocation, parent_type)
if dbparent:
pan_assign(lines, prefix + "sysloc/" + parent_type, dbparent.name)
# Hub is an exception where the associated sysloc property has a different name
if parent_type == "hub":
sysloc_property = "region"
else:
sysloc_property = parent_type
pan_assign(lines, prefix + "sysloc/" + sysloc_property, dbparent.name)

if dblocation.rack:
pan_assign(lines, prefix + "rack/name", dblocation.rack.name)
if dblocation.rack_row:
pan_assign(lines, prefix + "rack/row", dblocation.rack_row)
if dblocation.rack_column:
pan_assign(lines, prefix + "rack/column", dblocation.rack_column)
if dblocation.room:
pan_assign(lines, prefix + "sysloc/room", dblocation.room.name)
2 changes: 1 addition & 1 deletion lib/aquilon/worker/templates/city.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class PlenaryCity(Plenary):

@classmethod
def template_name(cls, dbcity):
return "%s/%s/%s/config" % (cls.prefix, dbcity.hub.fullname.lower(),
return "%s/%s/%s/config" % (cls.prefix, dbcity.continent.name,
Copy link

Choose a reason for hiding this comment

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

we cannot yet merge this cause we need to fix a 'bug' in city templates for timezones. but in general we agree with the change - just let us prepare for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem.

dbcity.name)

def body(self, lines):
Expand Down
2 changes: 1 addition & 1 deletion lib/aquilon/worker/templates/machine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class PlenaryMachineInfo(StructurePlenary):
@classmethod
def template_name(cls, dbmachine):
loc = dbmachine.location
return "%s/%s/%s/%s/%s" % (cls.prefix, loc.hub.fullname.lower(),
return "%s/%s/%s/%s/%s" % (cls.prefix, loc.continent.name,
Copy link

Choose a reason for hiding this comment

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

We have discussed this with Gabor just now: he suggest to take this even further and get rid of location attributes from the template path: and leave only (cls.prefix, dbmachine.label). Would this work for you?

Copy link
Contributor Author

@jouvin jouvin Jul 25, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

No problem for me... but will mean a directory with thousands or ten of thousands entries... Is it really what you want? I agree that the current hierarchy is probably too much but keeping the continent+building or continent+city or even just the city may be not so bad....

Copy link

Choose a reason for hiding this comment

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

What about creating a structure using the first letters of the dbmachine.label ?
For instance:

return "{0}/{1:.1}/{1:.2}/{1}".format(cls.prefix, dbmachine.label)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting suggestion, easy to implement... Not sure how diverse the machine names are, in particular their first letter. Ours tend to be something like rackXXYY where XX is the rack number and YY the slot number in the rack. Will defeat the directory hierarchy created from the first letters in our case... but probably not a major issue for us if everybody agrees it is the way to go.

Copy link
Member

Choose a reason for hiding this comment

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

Our machine names come from another system and start with one of: system, scarf, vm or test; so would have a similar problem, OTOH we only have 23000 hosts managed by the brokers, so not a huge problem.

Copy link
Member

@jrha jrha Jul 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

If we really need to break it up, how about using a modulus of the hardware_entity_id?

return "%s/%02x/%s" % (cls.prefix, dbmachine.id % 256, dbmachine.label)

Copy link
Member

Choose a reason for hiding this comment

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

Either that, or if you want something more ordered, how about groups of no more than 1000?

return "%s/%02d/%s" % (cls.prefix, dbmachine.id / 1000, dbmachine.label)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrha I think it would be good to have it easy to deduce from the machine information. dbmachine.id / 1000 is probably not the most convenient, even if it is predictible.

loc.building, loc.rack, dbmachine.label)

def __init__(self, dbobj, **kwargs):
Expand Down