(PUP-7309) acceptance: hiera merge strategies #5692

er0ck · 2017-03-06T23:51:42Z

test hiera3 and hiera5 merge strategies using hiera_hash and lookup
functions

edit: added a commit for hiera_array and merge_hash_arrays

joshcooper · 2017-03-07T06:40:25Z

acceptance/tests/lookup/merge_strategies.rb

+
+  teardown do
+    step "remove global hiera.yaml" do
+      on(master, "rm #{confdir}/hiera.yaml")


The global hiera.yaml is installed by packages, so we shouldn't remove it, and the test should use a temp hiera.yaml instead (or backup/restore).

oh right. thanks. i'll backup/restore

test hiera3 and hiera5 merge strategies using hiera_hash and lookup functions

* this covers a regression reported in PUP-7286 * ensure hiera_xxx uses merge behavior from hiera config * ensure lookup uses merge behavior per-lookup, or per key

joshcooper · 2017-03-07T20:17:37Z

acceptance/tests/lookup/merge_strategies.rb

+    SITE
+
+    on(master, "chmod -R 775 #{fq_tmp_environmentpath}")
+    on(master, "chmod -R 775 #{master_confdir}")


Is changing the permissions on the real confdir going to affect/break later tests that run puppet apply/agent and assume there are no changes? Do we just need to make the newly generated hiera.yaml ownable by puppet:puppet?

yeah, sloppy. i'll be more restrictive

johnduarte · 2017-03-07T21:00:17Z

acceptance/tests/lookup/merge_strategies.rb

+
+  teardown do
+    step "restore default global hiera.yaml" do
+      on(master, "mv #{hiera_conf_backup} #{master_confdir}/hiera.yaml")


This teardown step should ensure that the hiera_conf_backup file is present beforehand. If the backup step below should fail to create the backup file, then the teardown will erroneously fail.

😐 meh

i'm not clear why that's important. in particular because i stole this from your test O:)
although you have acceptable exit codes of 0 and 1

That's one reason why I slightly prefer generating temp files and pointing the test to the temp file. Less to worry about when cleaning up, and less likely to break test that follow.

yeah, but you can't do that here without reloading/restarting the server. i'm trying to avoid that in our tests so we can later remove #with_puppet_running_on.

i agree it's a tradeoff and we should probably make a decision on if you can touch the default install/config. we might even make it difficult to do, in the tooling or on the SUT in the pre-suite steps.

we should do some profiling with the proposed beaker profiler

I admire the direction, but I'm not sure that skipping restart (at least HUP) is safe, due to caching in hiera and/or the compiler. So I'm ok to merge this PR, but something to think about before removing with_puppet_running_on.

johnduarte · 2017-03-07T21:11:34Z

acceptance/tests/lookup/merge_strategies.rb

+  - array1:
+      key1: val1
+      key2: val2
+    YAML


nitpick, this is the only indented heredoc terminator in this PR

johnduarte · 2017-03-07T21:17:24Z

acceptance/tests/lookup/merge_strategies.rb

+
+  with_puppet_running_on(master,{}) do
+    agents.each do |agent|
+      step "agent lookups #{agent.hostname}, hiera3" do


Would this test be more readable and maintainable if it was split into two tests, one for v3 and one for v5?

i believe the opposite. putting it in one place makes it more discoverable, readable and maintainable, IMO

it's just on the verge of being too long 🦈

the one argument i'm thinking of for splitting it in two is that it could be easier to remove the v3 testing when it comes time. but this is still very easy in this test, and extremely difficult in other tests like lookup/lookup.rb :-\

[skip ci]

…ation [skip ci]

johnduarte

👍

er0ck · 2017-03-08T00:21:46Z

please do not merge until after PA 1.9.3 ships.
edit: 1.9.3 🚢 shipped

joshcooper · 2017-03-13T21:15:38Z

@er0ck could you verify this test works when there is an agent on the master?

er0ck · 2017-03-13T21:24:14Z

    "HOSTS": {
        "r0pibn118h5p96h.delivery.puppetlabs.net": {
            "pe_dir": null,
            "pe_ver": null,
            "pe_upgrade_dir": null,
            "pe_upgrade_ver": null,
            "hypervisor": "vmpooler",
            "platform": "el-7-x86_64",
            "template": "redhat-7-x86_64",
            "roles": [
                "master",
                "agent",
                "default"
            ],
            "host_tags": {},
            "user": "root",
            "group": "puppet",
            "pathseparator": ":",
            "vmhostname": "r0pibn118h5p96h.delivery.puppetlabs.net",
            "puppetbindir": "/opt/puppetlabs/bin",
            "privatebindir": "/opt/puppetlabs/puppet/bin",
            "distmoduledir": "/etc/puppetlabs/code/modules",
            "sitemoduledir": "/opt/puppetlabs/puppet/modules"
        },
        "kmkbp73wgtc5uc0.delivery.puppetlabs.net": {
            "pe_dir": null,
            "pe_ver": null,
            "pe_upgrade_dir": null,
            "pe_upgrade_ver": null,
            "hypervisor": "vmpooler",
            "platform": "ubuntu-14.04-amd64",
            "template": "ubuntu-1404-x86_64",
            "roles": [
                "agent",
                "aio"
            ],
            "host_tags": {},
            "user": "root",
            "group": "puppet",
            "pathseparator": ":",
            "vmhostname": "kmkbp73wgtc5uc0.delivery.puppetlabs.net",
            "puppetbindir": "/opt/puppetlabs/bin",
            "privatebindir": "/opt/puppetlabs/puppet/bin",
            "distmoduledir": "/etc/puppetlabs/code/modules",
            "sitemoduledir": "/opt/puppetlabs/puppet/modules"
        }
    },

...

tests/lookup/merge_strategies.rb passed in 27.37 seconds
      Test Suite: tests @ 2017-03-13 21:21:56 +0000

      - Host Configuration Summary -


              - Test Case Summary for suite 'tests' -
       Total Suite Time: 27.37 seconds
      Average Test Time: 27.37 seconds
              Attempted: 1
                 Passed: 1
                 Failed: 0
                Errored: 0
                Skipped: 0
                Pending: 0
                  Total: 1

      - Specific Test Case Status -

Failed Tests Cases:
Errored Tests Cases:
Skipped Tests Cases:
Pending Tests Cases:

joshcooper reviewed Mar 7, 2017

View reviewed changes

Eric Thompson added 2 commits March 7, 2017 18:30

(PUP-7309) acceptance: hiera merge strategies

821d2d8

test hiera3 and hiera5 merge strategies using hiera_hash and lookup functions

(PUP-7309) acceptance: hiera_array and lookup with merge strategies

4f88e08

* this covers a regression reported in PUP-7286 * ensure hiera_xxx uses merge behavior from hiera config * ensure lookup uses merge behavior per-lookup, or per key

er0ck force-pushed the acceptance/stable/PUP-7309-acceptance_hiera_merge_strategies branch from 60ddc69 to 4f88e08 Compare March 7, 2017 18:32

er0ck requested a review from johnduarte March 7, 2017 19:45

joshcooper reviewed Mar 7, 2017

View reviewed changes

johnduarte suggested changes Mar 7, 2017

View reviewed changes

Eric Thompson added 2 commits March 7, 2017 23:18

(maint) acceptance: fixup teardown, don't 775 entire confdir

f52961a

[skip ci]

(PUP-7309) acceptance: better merge_hash_arrays and hiera_array valid…

b6dfb13

…ation [skip ci]

johnduarte approved these changes Mar 8, 2017

View reviewed changes

er0ck added blocked PRs blocked on work external to the PR itself Merge After Unfreeze labels Mar 8, 2017

er0ck changed the title ~~(PUP-7309) acceptance: hiera merge strategies~~ (PUP-7309) DO NOT MERGE acceptance: hiera merge strategies Mar 8, 2017

er0ck removed blocked PRs blocked on work external to the PR itself Merge After Unfreeze labels Mar 13, 2017

er0ck changed the title ~~(PUP-7309) DO NOT MERGE acceptance: hiera merge strategies~~ (PUP-7309) acceptance: hiera merge strategies Mar 13, 2017

joshcooper merged commit f207bbf into puppetlabs:stable Mar 13, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(PUP-7309) acceptance: hiera merge strategies #5692

(PUP-7309) acceptance: hiera merge strategies #5692

er0ck commented Mar 6, 2017 •

edited

Loading

joshcooper Mar 7, 2017

er0ck Mar 7, 2017

joshcooper Mar 7, 2017

er0ck Mar 7, 2017

johnduarte Mar 7, 2017

er0ck Mar 7, 2017

joshcooper Mar 7, 2017

er0ck Mar 7, 2017

joshcooper Mar 7, 2017

johnduarte Mar 7, 2017

johnduarte Mar 7, 2017

er0ck Mar 7, 2017

er0ck Mar 7, 2017

er0ck Mar 13, 2017

johnduarte left a comment

er0ck commented Mar 8, 2017 •

edited

Loading

joshcooper commented Mar 13, 2017

er0ck commented Mar 13, 2017

(PUP-7309) acceptance: hiera merge strategies #5692

(PUP-7309) acceptance: hiera merge strategies #5692

Conversation

er0ck commented Mar 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnduarte left a comment

Choose a reason for hiding this comment

er0ck commented Mar 8, 2017 • edited Loading

joshcooper commented Mar 13, 2017

er0ck commented Mar 13, 2017

er0ck commented Mar 6, 2017 •

edited

Loading

er0ck commented Mar 8, 2017 •

edited

Loading