| Summary: | Incase boolean value is expected for Installer options, then w/o a value installer should just raise an error and should not proceed. | ||
|---|---|---|---|
| Product: | Red Hat Satellite | Reporter: | Sachin Ghai <sghai> |
| Component: | Installer | Assignee: | Martin Bacovsky <mbacovsk> |
| Status: | CLOSED DEFERRED | QA Contact: | Katello QA List <katello-qa-list> |
| Severity: | medium | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | 6.2.0 | CC: | bkearney, lzap, mmccune, ohadlevy, rjerrido, sghai, stbenjam, sthirugn |
| Target Milestone: | Unspecified | Keywords: | Triaged |
| Target Release: | Unused | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2017-03-27 20:47:58 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: | |
*** Bug 1333190 has been marked as a duplicate of this bug. *** Looks like you are missing "--foreman-plugin-discovery-install-images true" - it works just fine if you provide it. Looks like either kafo does not support this kind of syntax, or it is a bug for kafo. Discovery is a prime feature. So, there should be consistency between all installer options like either all should work with boolean value or help/error handling should be more intuitive. - Let's say when I install discovery plugin with following (note: no boolean value passed to --enable-foreman-plugin-discovery) then installer should raise an error that you missed parameter value, should be either true/false. But this time installer has installed plugin: ~]# satellite-installer --scenario satellite --enable-foreman-plugin-discovery Installing Done [100%] [..................................................................] Success! ~]# rpm -qa | grep discovery tfm-rubygem-foreman_discovery-5.0.0.7-1.el7sat.noarch Similar way, I tried --foreman-plugin-discovery-install-images option without any value. Installer ran without throwing any error. But this time no image was installed. [root@cloud-qe-3 ~]# satellite-installer --scenario satellite --foreman-plugin-discovery-install-images Installing Done [100%] [..................................................................] Success! ~]# rpm -qa | grep discovery tfm-rubygem-foreman_discovery-5.0.0.7-1.el7sat.noarch We should fix this by GA for consistency sake. This feature is up2date in all aspects and I think we should up2date this via installer too. Moreover, If I pass '--foreman-plugin-discovery-install-images true', it installs tarball not the image rpm. ~]# cd /var/lib/tftpboot/boot/ [root@cloud-qe-3 boot]# ll total 320364 drwxr-xr-x. 2 root root 68 Apr 28 08:12 fdi-image -rw-r--r--. 1 root root 238284800 May 26 12:23 fdi-image-latest.tar Please note that boot files in downstream "PXELinux global default" template are not configured with above naming conventions.Installation of discovery image *rpm* correctly set the boot file names in template lrwxrwxrwx. 1 root root 39 May 26 00:27 fdi-image-rhel_7-img -> foreman-discovery-image-3.1.1-7.iso-img lrwxrwxrwx. 1 root root 43 May 26 00:27 fdi-image-rhel_7-vmlinuz -> foreman-discovery-image-3.1.1-7.iso-vmlinuz Sachin, all boolean installer options require true/false, it's consistent across our product. Also, we have documented how FDI should be installed from RPM, this option is still valid and we are not removing it from the product if customer want to install upstream image (e.g. for a workaround). It works, so why to remove it? To me, this does not really look like a bug, unless I miss something. Sachin - ping? I would also challenge the question if this is a blocker or not. @Ohad: another bz related to discovery plugin install is in post state that will fix the issue related to discovery install via installer scenario. https://bugzilla.redhat.com/show_bug.cgi?id=1338000 Since we have a way to install plugin via installer so removing this blocker flag. As per IRC discussion with dev, here is the summary: 1) Value is always needed, the only expcetion are arguments for enabling/disabling modules e.g. --[no-]enable-foreman 2) the installer should fail if value was not provided but it might just use default value in this case. I'm not sure how we are going to fix this. But In any case, If values are needed and user is not passing any boolean value to a variable then installer should just throw an error and should not proceed. Here is the discussion snippet for record: <mhulan> lzap: sghai yes, the value is always needed, the only expcetion are arguments for enabling/disabling modules e.g. --[no-]enable-foreman <sghai> mhulan, lzap then if a user pass an option without any value, user should get an error.. though currently installer just proceed.. that's how I experience.. <sghai> I agreed its not just an issue with discovery options but in general for all <sghai> I'll just add a comment there and will set the severity low.. <sghai> mhulan, lzap ^ <lzap> sghai: yeah the UX is bad, I also vote for booleans without true/false for the future <mhulan> sghai: lzap as long as the argument clearly says what's the default then fine, I think clamp only supports booleans in form of --[no]-argument, I was under impression the installer fails if value was not provided @Bryan: Nop, I don't think its a change from 6.1. I do not see us adressing this in the next few releases. I am therefore closing this out. If you feel that this is a mistake, please feel free to re-open with additional information. |
Description of problem: here are the installer options to enable discovery plugin: # foreman-installer --help | grep discovery --[no-]enable-foreman-plugin-discovery Enable 'foreman_plugin_discovery' puppet module (default: true) = Module foreman_plugin_discovery: --foreman-plugin-discovery-image-name tarball with images (default: "fdi-image-latest.tar") --foreman-plugin-discovery-install-images should the installer download and setup discovery images --foreman-plugin-discovery-source-url source URL to download from (default: "http://downloads.theforeman.org/discovery/releases/latest/") --foreman-plugin-discovery-tftp-root tftp root to install image into (default: "/var/lib/tftpboot") 1) --enable-foreman-plugin-discovery only installs "tfm-rubygem-foreman_discovery-5.0.0.4-1.el7sat.noarch" package. It doesn't install smart-proxy package (rubygem-smart_proxy_discovery) 2) --foreman-plugin-discovery-install-images doesn't install foreman-discovery-image rpm. 3) --foreman-plugin-discovery-tftp-root doesn't work without an w/o default value "/var/lib/tftpboot" Please see the error below: # foreman-installer --scenario katello --foreman-plugin-discovery-install-images --foreman-plugin-discovery-tftp-root Parameter foreman-plugin-discovery-install-images invalid Error during configuration, exiting [ERROR 2016-04-21 03:14:10 main] Errors encountered during run: [ERROR 2016-04-21 03:14:10 main] "--foreman-plugin-discovery-tftp-root" is not a boolean. It looks to be a String [ERROR 2016-04-21 03:14:10 main] Parameter foreman-plugin-discovery-install-images invalid ]# foreman-installer --scenario katello --foreman-plugin-discovery-install-images --foreman-plugin-discovery-tftp-root /var/lib/tftpboot ERROR: too many arguments See: 'foreman-installer --help' Version-Release number of selected component (if applicable): sat6.2 beta snap9 How reproducible: Steps to Reproduce: 1. install discovery via installer options 2. 3. Actual results: Please see above Expected results: installer should provide all options to install all required packages for discovery: like we need: rubygem-smart_proxy_discovery-1.0.3-3.el7sat.noarch tfm-rubygem-foreman_discovery-5.0.0.4-1.el7sat.noarch foreman-discovery-image-3.1.1-3.el7sat.noarch tfm-rubygem-hammer_cli_foreman_discovery-0.0.2.1-1.el7sat.noarch All option should work as expected. Additional info: