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

Change site and machine namespace to use continent name #110

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

Conversation

jouvin
Copy link
Contributor

@jouvin jouvin commented Jul 13, 2018

The hub fullname was used previously but as it was not exported to the object plenary, it was impossible to locate the site template (not included by default in the object plenary)

  • It is recommended to flush machine and site firt and deploy them in all domains (old templates are not removed by the broker) before flushing hosts.
  • Hosts must be flushed to use the new namespace

This PR also adds the hub information in the object plenary under /hardware/sysloc/region.

Fixes #104

Change-Id: I71e59ffa750a264b88ecf35ae3e6ebecd73f98e2

@jouvin jouvin changed the title [WIP] Change site and machine namespace to use continent name Change site and machine namespace to use continent name Jul 13, 2018
@jouvin jouvin requested review from ned21, jrha and urbonegi July 13, 2018 08:28
@ned21
Copy link
Contributor

ned21 commented Jul 17, 2018

I don't think hosts need to be rebuilt do they? It just needs a flush of the host plenaries and a co-ordindated update to all template domains.

Anyway this looks a sensible approach to me, but I think the unit tests will need updating too.

@jouvin
Copy link
Contributor Author

jouvin commented Jul 17, 2018

You are right, a flush is enough but a recompile also if you want to assess that everything is ok, isn't it?

@@ -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,

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

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

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.

@@ -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,

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.

@@ -629,18 +629,20 @@ def transaction(self, verbose=False):


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

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).

- Used to be hub fullname previously which is not exported to the object
plenary, making impossible to locate the site template (not included
by default in the object plenary)
- Hosts must be rebuilt to use the new namespace

Fixes quattor#104

Change-Id: I71e59ffa750a264b88ecf35ae3e6ebecd73f98e2
- Contributes to quattor#104

Change-Id: I061147728c376e722b1ffd67665921416e1291d8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants