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

Fixes #10161 - Convert foreman-installer config to scenario #996

Conversation

mbacovsky
Copy link
Member

  • updated dependency constraint on Kafo
  • updated scenario paths

@mbacovsky
Copy link
Member Author

Depends on theforeman/foreman-installer#163

@mbacovsky mbacovsky force-pushed the rpm/foreman-installer_scenarios branch from 7ed994a to 3008252 Compare January 28, 2016 22:01
@mbacovsky
Copy link
Member Author

The script in %post (foreman-installer --scenario foreman --migrations-only > /dev/null) is there to apply migrations to Scenario config and answers. Also migration from "old style" configuration is involved. The migrations work similarly to rails db migrations. We start with basic config to make automatic migration from old style config possible and then scenario migrations are applied. The user ends up with up-to-date config and answer files after installation, can observe and change the values there before running installer.

@domcleal
Copy link
Contributor

domcleal commented Feb 9, 2016

This PR's not mergeable, please rebase it.

I tried to build it as-is, but hit the following error during parameter export:

** Execute /builddir/build/BUILD/foreman-installer-1.11.0/_build/options.asciidoc
/usr/share/gems/bin/kafo-export-params -c config/foreman.yaml -f asciidoc > /builddir/build/BUILD/foreman-installer-1.11.0/_build/options.asciidoc
/usr/share/gems/gems/kafo_parsers-0.0.5/lib/kafo_parsers/puppet_module_parser.rb:32:in `initialize': uninitialized constant KafoParsers::PuppetModuleParser::ModuleName (NameError)
    from /usr/share/gems/gems/kafo_parsers-0.0.5/lib/kafo_parsers/puppet_module_parser.rb:18:in `new'
    from /usr/share/gems/gems/kafo_parsers-0.0.5/lib/kafo_parsers/puppet_module_parser.rb:18:in `parse'
    from /usr/share/gems/gems/kafo-0.7.1/lib/kafo/puppet_module.rb:51:in `parse'
    from /usr/share/gems/gems/kafo-0.7.1/lib/kafo/configuration.rb:87:in `block in modules'
    from /usr/share/gems/gems/kafo-0.7.1/lib/kafo/configuration.rb:87:in `map'
    from /usr/share/gems/gems/kafo-0.7.1/lib/kafo/configuration.rb:87:in `modules'
    from /usr/share/gems/gems/kafo-0.7.1/bin/kafo-export-params:90:in `print_out'
    from /usr/share/gems/gems/kafo-0.7.1/bin/kafo-export-params:40:in `execute'
    from /usr/share/gems/gems/clamp-1.0.0/lib/clamp/command.rb:68:in `run'
    from /usr/share/gems/gems/clamp-1.0.0/lib/clamp/command.rb:133:in `run'
    from /usr/share/gems/gems/kafo-0.7.1/bin/kafo-export-params:135:in `'
    from /usr/share/gems/bin/kafo-export-params:23:in `load'
    from /usr/share/gems/bin/kafo-export-params:23:in `'
rake aborted!
Command failed with status (1): [/usr/share/gems/bin/kafo-export-params -c ...]

This seems to indicate that a file of some sort is missing, but the SRPM I'm using looks OK. Could you check this builds properly?

Edit: SRPM at https://fedorapeople.org/~domcleal/foreman/rpm_10161/

@mmoll
Copy link

mmoll commented Feb 9, 2016

the same is happening in the DEB build modeled after this PR.

@mbacovsky mbacovsky force-pushed the rpm/foreman-installer_scenarios branch from 3008252 to abed03d Compare February 10, 2016 19:50
@mbacovsky
Copy link
Member Author

I rebased the PR.
@domcleal, I was able to rebuild with my srpm but the srpm you've provided failed. My build is http://koji.katello.org/koji/taskinfo?taskID=420041.

While investigating it I found there is a bug in kafo_prsers preventing debugging (PR with a fix is theforeman/kafo_parsers#12). kafo_parsers originaly tried to complain about some missing manifest.
The difference between the two srpms were

  • puppet dependency (>=2.7 vs. >=3.0)
  • my foreman-installer tarball contained extra alternatives, extlib, inifile, puppetdb and staging modules (probably caused by generating it localy on Fedora). Fixed puppet_parser could reveal what manifest is missing. I'll know better tomorrow.

@mbacovsky
Copy link
Member Author

I created foreman-installer tarball on clean CentOS7 box and the package built okay with the data: http://koji.katello.org/koji/taskinfo?taskID=420604
I checked the tarball content and it also contains more packages then the tarball @domcleal used.
Interestingly enough also @mmoll met the same behaviour. Can you provide me your steps to build the foreman-installer tarball?

@mmoll
Copy link

mmoll commented Feb 11, 2016

@mbacovsky I didn't build it from hand but see the Jenkins output in #1015

@domcleal
Copy link
Contributor

Thanks, I've been able to build it now. I had an old _build/ dir in my foreman-installer checkout that was being re-used, so after the kafo_parsers fix I could see that it was missing a manifest that should've been present in more recent modules.

%files
%defattr(-,root,root,-)
%doc README.* LICENSE
%config %attr(600, root, root) %{_sysconfdir}/foreman/%{name}.yaml
%config(noreplace) %attr(600, root, root) %{_sysconfdir}/foreman/%{name}-answers.yaml
%config %attr(600, root, root) %{_sysconfdir}/foreman/installer-scenarios.d/foreman.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

This package needs to install and own the installer-scenarios.d directory as it isn't owned by anything else. Ideally perhaps it'd be under /etc/foreman-installer/scenarios.d/ to match the log file layout?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I updated this PR and foreman-installer PR to use /etc/foreman-installer/scenarios.d/.

@mbacovsky mbacovsky force-pushed the rpm/foreman-installer_scenarios branch from abed03d to d4738c9 Compare February 17, 2016 00:39
%files
%defattr(-,root,root,-)
%doc README.* LICENSE
%config %attr(600, root, root) %{_sysconfdir}/foreman/%{name}.yaml
%config(noreplace) %attr(600, root, root) %{_sysconfdir}/foreman/%{name}-answers.yaml
%dir %{_sysconfdir}/foreman-installer
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick, prefer %{name} rather than repeating foreman-installer here and below.

Copy link
Contributor

Choose a reason for hiding this comment

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

Changed in 5ea91c1.

@domcleal
Copy link
Contributor

[test] to verify it builds after the installer change was merged.

@domcleal
Copy link
Contributor

[test] fixed tests as I'd temporarily disabled a Jenkins job because of the change.

@domcleal
Copy link
Contributor

Merged as c3f00e7, thanks @mbacovsky.

@domcleal domcleal closed this Feb 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants