Bug 1438666
Summary: | /distribution/inventory fails if the lab DHCP server puts unqualified hostnames in DHCP option 12 | ||
---|---|---|---|
Product: | [Retired] Beaker | Reporter: | Dan Callaghan <dcallagh> |
Component: | general | Assignee: | Jonathan Toppins <jtoppins> |
Status: | CLOSED CURRENTRELEASE | QA Contact: | Dan Callaghan <dcallagh> |
Severity: | unspecified | Docs Contact: | |
Priority: | unspecified | ||
Version: | 24 | CC: | dcallagh, jtoppins, mjia, nhorman, rjoost |
Target Milestone: | 24.3 | Keywords: | Patch |
Target Release: | --- | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | If docs needed, set a value | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2017-05-30 07:08:06 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Description
Dan Callaghan
2017-04-04 06:17:40 UTC
I don't have an example of the error from /distribution/inventory handy right now, but it would be good to paste one in here for future searchability. Simplest solution might be to just swap $HOSTNAME for $(hostname -f) in /distribution/inventory. an alternate solution may be to mandate that your dhcp server provide fqdns in option 12. dnsmasq has exactly that option in --dhcp-fqdn It would be nice to give the user the option to override the HOSTNAME parameter easily. That way if the beaker user/admin does not get a choice in what dhcp server they use a user is still able to easily make the inventory update task run. I was thinking of something like this: $ git --no-pager log -1 -p commit 786f5a27b416834980f7b82a1721aceae5cade6d Author: Jonathan Toppins <jtoppins> Date: Mon Apr 10 18:32:45 2017 -0400 add an additional option to force override the task to use the FQDN instead of the hostname the box gets configured with during install diff --git a/Client/src/bkr/client/commands/cmd_update_inventory.py b/Client/src/bkr/client/commands/cmd_update_inventory.py index 69b9cf2a6962..d15bb63889e0 100644 --- a/Client/src/bkr/client/commands/cmd_update_inventory.py +++ b/Client/src/bkr/client/commands/cmd_update_inventory.py @@ -123,6 +123,12 @@ def options(self): action='store_true', help='Wait on job completion', ) + self.parser.add_option( + '--force-hostname', + default=False, + action='store_true', + help='Force hostname to be the same as provided on commandline' + ) def run(self, *args, **kwargs): @@ -132,6 +138,7 @@ def run(self, *args, **kwargs): xml = kwargs.get('xml') prettyxml = kwargs.get('prettyxml') wait = kwargs.get('wait') + force_hostname = kwargs.get('force-hostname') self.set_hub(**kwargs) requests_session = self.requests_session() submitted_jobs = [] @@ -139,7 +146,8 @@ def run(self, *args, **kwargs): for fqdn in args: res = requests_session.post('jobs/+inventory', json={'fqdn':fqdn, - 'dryrun':dryrun}) + 'dryrun':dryrun, + 'forcehost':force_hostname}) try: res.raise_for_status() except HTTPError, e: diff --git a/Server/bkr/server/jobs.py b/Server/bkr/server/jobs.py index 82f378df564b..44a328835add 100644 --- a/Server/bkr/server/jobs.py +++ b/Server/bkr/server/jobs.py @@ -1218,7 +1218,7 @@ def update_job_status(id): @auth_required def submit_inventory_job(): """ - Submit a inventory job with the most suitable distro selected automatically. + Submit an inventory job with the most suitable distro selected automatically. Returns a dictionary consisting of the job_id, recipe_id, status (recipe status) and the job XML. If ``dryrun`` is set to ``True`` in the request, the first three @@ -1226,6 +1226,7 @@ def submit_inventory_job(): :jsonparam string fqdn: Fully-qualified domain name for the system. :jsonparam bool dryrun: If True, do not submit the job + :jsonparam bool forcehost: If True, force HOSTNAME parameter to be the same as fqdn """ if 'fqdn' not in request.json: raise BadRequest400('Missing the fqdn parameter') @@ -1234,6 +1235,11 @@ def submit_inventory_job(): dryrun = request.json['dryrun'] else: dryrun = False + if 'forcehost' in request.json: + forcehost = request.json['forcehost'] + else: + forcehost = False + try: system = System.by_fqdn(fqdn, identity.current.user) except DatabaseLookupError: @@ -1246,8 +1252,11 @@ def submit_inventory_job(): job_details = {} job_details['system'] = system job_details['whiteboard'] = 'Update Inventory for %s' % fqdn + job_details['fqdn'] = fqdn + with convert_internal_errors(): - job_xml = Job.inventory_system_job(distro, dryrun=dryrun, **job_details) + job_xml = Job.inventory_system_job(distro, dryrun=dryrun, + forcehost=forcehost, **job_details) r = {} if not dryrun: r = system.find_current_hardware_scan_recipe().__json__() diff --git a/Server/bkr/server/model/scheduler.py b/Server/bkr/server/model/scheduler.py index b67e40755791..6fc53be5d117 100644 --- a/Server/bkr/server/model/scheduler.py +++ b/Server/bkr/server/model/scheduler.py @@ -977,7 +977,8 @@ def provision_system_job(cls, distro_trees, pick='auto', **kw): return job @classmethod - def inventory_system_job(cls, distro_tree, dryrun=False, **kw): + def inventory_system_job(cls, distro_tree, dryrun=False, + forcehost=False, **kw): """ Create a new inventory job and return the job XML. If this is not a dry run, then also submit the job """ @@ -1013,6 +1014,10 @@ def inventory_system_job(cls, distro_tree, dryrun=False, **kw): recipe.tasks.append(install_task) # Add inventory task inventory_task = RecipeTask.from_task(Task.by_name(u'/distribution/inventory')) + if forcehost: + inventory_task.params = [ + RecipeTaskParam("HOSTNAME", kw.get('fqdn')), + ] recipe.tasks.append(inventory_task) recipeSet.recipes.append(recipe) job.recipesets.append(recipeSet) I will continue what my RFC code and take a crack at getting this upstream. Jonathan, do you want to try using $(hostname -f) in /distribution/inventory first? Fixing it there will be much easier than doing all this extra work on the server side to inject a HOSTNAME parameter. Hi Dan, I can change to using $(hostname -f). The issue I was seeing was that at least on my box typing in 'hostname -f' still only returned the hostname and not the full fqdn. My thought process here was to allow the inventory collection to work even if the network is not configured as it should be. Okay fair enough. I guess even hostname -f won't work properly if search domains are missing from DHCP or something else like that. Ultimately the Beaker server knows the "authoritative" FQDN which the system is recorded in the database as. And we always want /distribution/inventory to report back to Beaker using the same hostname so we can match it up. So maybe we should just unconditionally always fill in HOSTNAME parameter for /distribution/inventory? I think no need to add an extra parameter for it. If you could post a patch to Gerrit that would be great. :-) Let me know if you have any issues using Gerrit and I will help out. (In reply to Dan Callaghan from comment #9) > I think no need to add an extra parameter for it. I meant, no need for a new XMLRPC argument or command line option. I think we can just fill in the HOSTNAME parameter unconditionally. Beaker 24.3 has been released. Turns out this never actually did anything, see bug 1559667. However the problem is (probably) solved more simply by bug 1346068. |