Bug 1572947 - "hammer discovery provision" command with --parameters does not work
Summary: "hammer discovery provision" command with --parameters does not work
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Satellite
Classification: Red Hat
Component: Discovery Plugin
Version: 6.3.0
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: 6.6.0
Assignee: Lukas Zapletal
QA Contact: Jitendra Yejare
URL: http://projects.theforeman.org/issues...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2018-04-29 06:32 UTC by Hao Chang Yu
Modified: 2019-10-22 19:50 UTC (History)
10 users (show)

Fixed In Version: tfm-rubygem-hammer_cli_foreman_discovery-1.0.1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1756042 (view as bug list)
Environment:
Last Closed: 2019-10-22 19:50:16 UTC
Target Upstream Version:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Foreman Issue Tracker 23476 0 Normal Closed CLI command with --parameters and --interface options never work 2020-03-12 06:42:15 UTC
Red Hat Knowledge Base (Solution) 3664571 0 None None None 2018-11-01 11:07:13 UTC

Description Hao Chang Yu 2018-04-29 06:32:31 UTC
Description of problem:
The values of --parameters and --interface options for "hammer discovery provision" command never pass to the Satellite server. 


hammer discovery provision --name=<my hostname> --architecture x86_64 --hostgroup <my HG> --domain <my domain> --interface "primary=true, managed=true, provision=true, mac=<MAC>, name=<NAME>, identifier=<ID>, ip=<IP>" --parameters "a=b, c=d"

Satellite production log:
[app] [I] Started PUT "/api/v2/discovered_hosts/122" for ::1 at 2018-04-29 16:15:47 +1000
[app] [I] Processing by Api::V2::DiscoveredHostsController#update as JSON
[app] [I]   Parameters: {"discovered_host"=>{"name"=>"<my hostname>", "architecture_id"=>1, "domain_id"=>1, "hostgroup_id"=>2}, "apiv"=>"v2", "id"=>"122"}


The following patch should fix the missing host parameters issue:
------------------------------------------
# diff -u /tmp/discovery.rb  lib/hammer_cli_foreman_discovery/discovery.rb
--- /tmp/discovery.rb	2018-04-29 16:20:39.827007345 +1000
+++ lib/hammer_cli_foreman_discovery/discovery.rb	2018-04-29 16:22:44.648369253 +1000
@@ -111,6 +111,7 @@
       def request_params
         params = super
 
+        params['discovered_host']['host_parameters_attributes'] = parameter_attributes
         params['discovered_host']['ptable_id'] = option_partition_table_id unless option_partition_table_id.nil?
         params['discovered_host']['root_pass'] = option_root_password unless option_root_password.nil?
         params['discovered_host']['overwrite'] = option_overwrite unless option_overwrite.nil?
@@ -122,6 +123,17 @@
         params
       end
 
+      def parameter_attributes
+        return {} unless option_parameters
+        option_parameters.collect do |key, value|
+          if value.is_a? String
+            {"name"=>key, "value"=>value}
+          else
+            {"name"=>key, "value"=>value.inspect}
+          end
+        end
+      end
+
       build_options
     end
---------------------------------------

The "interface_attributes" doesn't look like a valid param for "PUT /api/v2/discovered_hosts/" API so I am not sure whether it should be removed from the hammer command or something else.

Comment 3 Lukas Zapletal 2018-05-02 07:22:29 UTC
Yeah, a good 6.3.z candidate, thanks for the patch.

Comment 4 pm-sat@redhat.com 2018-05-02 10:08:56 UTC
Upstream bug assigned to orabin@redhat.com

Comment 5 pm-sat@redhat.com 2018-05-02 10:09:01 UTC
Upstream bug assigned to orabin@redhat.com

Comment 6 pm-sat@redhat.com 2018-05-04 20:09:11 UTC
Upstream bug assigned to lzap@redhat.com

Comment 7 pm-sat@redhat.com 2018-05-04 20:09:16 UTC
Upstream bug assigned to lzap@redhat.com

Comment 13 Lukas Zapletal 2019-08-01 13:57:36 UTC
So here is my analysis. Command "hammer discovery provision" is essentially based on "hammer host edit" with few changes, however the core of the command is to take an existing (discovered) host, perform some edits and trigger provisioning on it. The missing --parameters was a bug, the extra --host-parameters-attributes needs to be removed it is just a name clash (host-parameters-attributes is just an internal API name for --parameters).

Now, "hammer host edit" have --interfaces option which allows users to *add* new interfaces, but it does not allow edit or delete of existing ones. For this, our API and hammer have a different set of commands via "hammer interface". Since discovery is based on "host edit" it also shows --interfaces option however it is pretty much useless - one could possibly only add new intefaces, however we haven't tested this at all and chances are this does not work at all. Support for editing interfaces is one of the most requested discovery plugin features for both UI and API but at the moment discovered hosts must be provisioned with the same interfaces as they were discovered with.

So the next step is to hide attributes --host-parameters-attributes and --interfaces from "hammer discovery provision" at least not to confuse users until we provide ability to edit interfaces for discovered hosts. That is totally different story because discovery is build around autoprovisioning where you cannot do interactive things (and it was the main idea and motivation to write this plugin).

I suggest to verify --parameters in this BZ and create new BZ to hide the attributes as described above. That one is just a cosmetic BZ, it should not change any behavior.

Comment 14 Lukas Zapletal 2019-08-01 15:00:47 UTC
Filed: https://bugzilla.redhat.com/show_bug.cgi?id=1736179

Comment 15 Jitendra Yejare 2019-09-04 09:17:36 UTC
Verified!

@ Satellite 6.6 snap 18


Steps:
---------
# hammer -v discovery provision --name="maceeb9ea4c6af5" --hostgroup="2YWUW1" --root-password="1dPRfCfYeV" --parameters="jyejare_before_fix=no,jyejare_after_fix=yes"'
Host created


Observation:
--------------
1. The discovered host is provisioned successfully along with parameters.
2. The QE automation test is passed -> tests/foreman/cli/test_discoveredhost.py::DiscoveredTestCase::test_positive_provision_pxe_host_with_parameters.

Comment 16 Jitendra Yejare 2019-09-04 09:19:13 UTC
Also, as per comment 14, the separate bug to remove interface option is been created and hence the interface issue no more blocks this issue from verifying.

Hence the bug is verified.

Comment 17 Bryan Kearney 2019-10-22 19:50:16 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHSA-2019:3172


Note You need to log in before you can comment on or make changes to this bug.