External_node_v2 refactor!

Development
#1

I'm joining this project now. I made inquiries a few months ago, and
finally our internal puppet management team green-lighted me for the
enhancements that needed to be made.

The main focus point and primary obstacle to my enhancements is
external_node_v2.rb (puppet-foreman subproject). I thank +domcleal for
pointing me in direction of this file. It looks like it was originally
authored by +ohadlevy and most recently by +mmoll. What I need to do is as
follows: We use The Foreman purely as an ENC to puppet – pretty much
nothing else. We assign classes to hosts, and tag hosts as belonging to
hostgroups and organizations.

TheForeman's hostgroups feature does not integrate well with puppet. This
would seem to be an obvious use-case, but for whatever reason, we're (UIBK)
somewhat alone here. A hostgroup in TheForeman consists of an entire
hierarchy of hostgroups. When TheForeman is queries as an ENC for a
particular node, we get something like:

hostgroup: base/base_uibk/base_osdb/postgresql/postgresql_95

What we want is that each hostgroup triggers a different Hiera file to be
read. Our hiera.yaml file contains directives like this:

:hierarchy:

  • "%{::environment}/hiera/base"
  • "%{::environment}/hiera/repos"
  • "%{::environment}/hiera/nodes/%{::clientcert}"
  • "%{::environment}/hiera/hostgroups/%{::foreman_hostgroup}"
  • "%{::environment}/hiera/hostgroups/%{::foreman_hostgroup_parent5}"
  • "%{::environment}/hiera/hostgroups/%{::foreman_hostgroup_parent4}"
  • "%{::environment}/hiera/hostgroups/%{::foreman_hostgroup_parent3}"
  • "%{::environment}/hiera/hostgroups/%{::foreman_hostgroup_parent2}"
  • "%{::environment}/hiera/hostgroups/%{::foreman_hostgroup_parent1}"
  • "%{::environment}/hiera/organizations/%{::foreman_organization}"

The result is that the hiera files will be processed in the following order:

<organization-specific>
hostgroups/base.yaml
hostgroups/base_uibk.yaml
hostgroups/base_osdb.yaml
hostgroups/postgresql.yaml
hostgroups/postgresql_95.yaml
nodes/<certname>.yaml
repos.yaml
base.yaml

But there is no easy/clean way to get the hiera backend to process the
hostgroup fact in such a way.

We can use the foreman to load a fact file on the client which will then
report its facts. However, on the first run, the fact file will be empty
and incomplete. This creates probelms for deployment.

After discussions with domcleal, the simplest solution seems to be to
modify the external_node_v2.rb file, which is replaced as node.rb on the
production system.

I have created such a patch … BUT I feel the strong need to refactor the
code. A lot of this code does things that are completely mysterious to me
(what's "push facts" for?), so I need to get a discussion going with all
the maintainers of this file.

#2

https://theforeman.org/manuals/1.12/index.html#3.5.5FactsandtheENC
briefly describes these functions at the bottom of the section.

If you wish to refactor it, go ahead and submit pull requests. Best to
be precise about what it's changing and why, plus try to split it into
logical changes separate to whatever features you're implementing.

··· On 19/07/16 13:19, otheus uibk wrote: > After discussions with domcleal, the simplest solution seems to be to > modify the external_node_v2.rb file, which is replaced as node.rb on the > production system. > > I have created such a patch .... BUT I feel the strong need to refactor > the code. A lot of this code does things that are completely mysterious > to me (what's "push facts" for?)

–
Dominic Cleal
dominic@cleal.org

#3

> I'm joining this project now. I made inquiries a few months ago, and
> finally our internal puppet management team green-lighted me for the
> enhancements that needed to be made.
>

Welcome! :slight_smile:

> I have created such a patch … BUT I feel the strong need to refactor
> the code. A lot of this code does things that are completely mysterious to
> me (what's "push facts" for?), so I need to get a discussion going with all
> the maintainers of this file.
>

In addition to what Dominic said, I'll throw two more options out there for
you:

  1. Most of the developers hang out in #theforeman-dev on Freenode IRC [1] -
    you're more than welcome to join us to chat about what you're working on.

  2. What Dominic said about sending pull requests is absolutely fine, but if
    you feel the need to follow a design process, we do have an RFCs repo [2]
    which can be used to work on designs. This is not mandatory, but can be
    handy for detailed discussions.

Cheers!
Greg

[1] Foreman :: Support
[2] https://github.com/theforeman/rfcs

··· On 19 July 2016 at 13:19, otheus uibk wrote:
#4

The critical nature of node.rb has taken me down the rabbit hole of
functional development, which in the ruby/puppet world means rspec. But in
order to refactor properly, what I really need here is to test the behavior
of the script as an executable. The current rspec file does unit testing
on some of the internal methods of that file, treated as a module. Some
googling brought me to the ARUBA project, which is a sub project of
cucumber. I'm very keen on the whole BDD cycle in general, but
unfortunately, the style of documentation of that project makes it very
difficult for me to proceed. It's clear that if I were to test
external_node_v2.rb using cucumber/aruba, I will need to add quite a few
files and it's a lot of work. Which I don't mind doing. But before I
proceed, I ask: Are there objections to using cucumber / aruba within this
project? Are there alternatives for testing a program as a black box? (In
our case, the testing harness will need to start a temporary HTTP service
to provide test files, in lieu of an ability to set up a mock).

#5

>
> In addition to what Dominic said, I'll throw two more options out there
> for you:
>
> 1) Most of the developers hang out in #theforeman-dev on Freenode IRC [1]
> - you're more than welcome to join us to chat about what you're working on.
>

I'm a bit unhappy with using IRC. I don't understand how you can have
meaningful conversation there that last longer than a few minutes.

> 2) What Dominic said about sending pull requests is absolutely fine, but
> if you feel the need to follow a design process, we do have an RFCs repo
> [2] which can be used to work on designs. This is not mandatory, but can be
> handy for detailed discussions.
>

OK

#6

>
> Foreman :: Manual
> briefly describes these functions at the bottom of the section.

Thanks for pointing me to the specific doc! The documentation is certainly
better and more detailed than I expected.

After, I still don't get:

  • what SETTINGS[:facts] is for. Here's the comment:

    # send facts to Foreman - enable &#39;facts&#39; setting to activate
# if you use this option below, make sure that you don&#39;t send facts

to foreman via the rake task or push facts alternatives.

This looks like process_all_facts but for the given certname only. It
uploads the facts to the #{url} via the mapped hostname, and then downloads
them from the given certname. What's the use-case here? In the docs,
:facts is always set to true, but I'm not sure it actually does anything
most of the time. (that might be just our environment, though)

  • What is no_env for? I see what it does: it removes "environment" from the
    facts that are reported. I see what the documentation says about it. The
    questions are (1) shouldn't this be in SETTINGS? and (2) how does the
    presence force puppet to use a particular environment?

  • Why is the watch functionality incorporated into this script? Shouldn't
    this really be a separate task/script altogether? (I'm thinking of
    partitioning of activities via sudo, for instance)

#7

> Foreman :: Manual
> <Foreman :: Manual>
> briefly describes these functions at the bottom of the section.
>
>
> Thanks for pointing me to the specific doc! The documentation is
> certainly better and more detailed than I expected.
>
> After, I still don't get:
>
> * what SETTINGS[:facts] is for. Here's the comment:
>
> # send facts to Foreman - enable 'facts' setting to activate
> # if you use this option below, make sure that you don't send
> facts to foreman via the rake task or push facts alternatives.
>
> This looks like process_all_facts but for the given certname only. It
> uploads the facts to the #{url} via the mapped hostname, and then
> downloads them from the given certname. What's the use-case here? In
> the docs, :facts is always set to true, but I'm not sure it actually
> does anything most of the time. (that might be just our environment, though)

It doesn't download facts, it downloads the ENC output. It should be
uploading facts to Foreman, then retrieving the ENC output that contains
lists of classes, parameters etc in YAML, which may depend on the facts
previously uploaded.

This is the normal fact upload method if the ENC is in use, not using
process_all_facts/push.

> * What is no_env for? I see what it does: it removes "environment" from
> the facts that are reported. I see what the documentation says about
> it. The questions are (1) shouldn't this be in SETTINGS? and (2) how
> does the presence force puppet to use a particular environment?

(1) Yes, probably - it pre-dates the settings file.
(2) Agents make two calls to the master during a run that cause the ENC
script to run. The first is an environment ("node") query. If the ENC
script returns an environment then the agent and master will use that
configured environment, ignoring anything the agent has set on the CLI
or in puppet.conf. This allows users to disable that on the Puppet
master by passing --no-environment.

> * Why is the watch functionality incorporated into this script?
> Shouldn't this really be a separate task/script altogether? (I'm
> thinking of partitioning of activities via sudo, for instance)

Perhaps. It's probably just convenience to put them together, given that
this script has all of the code to upload and track facts. If it was
split into many files (e.g. library and binaries) it should probably
also move into a package so it's easier to deploy.

··· On 19/07/16 18:23, otheus uibk wrote:

–
Dominic Cleal
dominic@cleal.org

#8

Having written being in favor of Cucumber / Aruba, I do have doubts about
it: the documentation is severely lacking, and while I finally found a
decent guide, it's not clear to me using this will provide the tactical
advantage I hoped for.

··· On Thursday, July 21, 2016 at 2:54:09 AM UTC+2, otheus uibk wrote: > > The critical nature of node.rb has taken me down the rabbit hole of > functional development, which in the ruby/puppet world means rspec. But in > order to refactor properly, what I really need here is to test the behavior > of the script _as an executable_. The current rspec file does unit testing > on some of the internal methods of that file, treated as a module. Some > googling brought me to the ARUBA project, which is a sub project of > cucumber. I'm very keen on the whole BDD cycle in general, but > unfortunately, the *style* of documentation of that project makes it very > difficult for me to proceed. It's clear that if I were to test > external_node_v2.rb using cucumber/aruba, I will need to add quite a few > files and it's a lot of work. Which I don't mind doing. But before I > proceed, I ask: Are there objections to using cucumber / aruba within this > project? Are there alternatives for testing a program as a black box? (In > our case, the testing harness will need to start a temporary HTTP service > to provide test files, in lieu of an ability to set up a mock). >
#9

Hello

I don't think there's too much functionality in this script. Maybe a unit test
for what's worth of testing would be enough? In such case I'd recommend using
minitest. It has some stubbing built-in but maybe webmock [1] might be useful
for stubbing out communication with Foreman.

[1] https://github.com/bblimke/webmock

··· -- Marek

On Thursday 21 of July 2016 01:50:02 otheus uibk wrote:

Having written being in favor of Cucumber / Aruba, I do have doubts about
it: the documentation is severely lacking, and while I finally found a
decent guide, it’s not clear to me using this will provide the tactical
advantage I hoped for.

On Thursday, July 21, 2016 at 2:54:09 AM UTC+2, otheus uibk wrote:

The critical nature of node.rb has taken me down the rabbit hole of
functional development, which in the ruby/puppet world means rspec. But in
order to refactor properly, what I really need here is to test the
behavior
of the script as an executable. The current rspec file does unit testing
on some of the internal methods of that file, treated as a module. Some
googling brought me to the ARUBA project, which is a sub project of
cucumber. I’m very keen on the whole BDD cycle in general, but
unfortunately, the style of documentation of that project makes it very
difficult for me to proceed. It’s clear that if I were to test
external_node_v2.rb using cucumber/aruba, I will need to add quite a few
files and it’s a lot of work. Which I don’t mind doing. But before I
proceed, I ask: Are there objections to using cucumber / aruba within this
project? Are there alternatives for testing a program as a black box? (In
our case, the testing harness will need to start a temporary HTTP service
to provide test files, in lieu of an ability to set up a mock).