PE-24898 Set puppet collection value associated with agent version #60

shaigy · 2018-08-06T23:19:24Z

Legacy agent installation job testing older agents with new PE versions
in Integration CI does not use frictionless installation method. Instead
it downloads the agent package from pm.puppetlabs.com. Agent
installation in that case fails since the puppet_collection value is set
to default 'PC1' instead of 'puppet5' causing the construction of wrong
'onhost_copied_file' path. Added a method to return the correct
puppet_collection depending on the agent version and use that value
instead of the default

puppetlabs-jenkins · 2018-08-06T23:19:26Z

Can one of the admins verify this patch?

shaigy · 2018-08-06T23:23:10Z

@jpartlow Here is the change I tried and installation passed with legacy_agent versions 2018.1.3%3A5.5.4 and 2018.1.2%3A5.5.2

jpartlow · 2018-08-07T17:21:03Z

lib/beaker-puppet/install_utils/puppet_utils.rb

@@ -72,6 +72,20 @@ def remove_puppet_paths_on(hosts)
          end
        end

+        #Given an agent_version, return the puppet collection associated with that agent version
+        #@param [String] agent_version version string or 'latest'
+        def get_puppet_collection(agent_version = 'latest')


Please add a spec to puppet_utils_spec.rb testing get_puppet_collection().

It looks like this would return PC1 for 'latest', but I'm not sure why? Oh I see, opts[:puppet_agent_version] defaults to 'latest'. That does preserve the previous behavior...well, actually they weren't connected before, so I guess we need to decide what 'latest' would mean, and to mean that would mean latest collection?

That's right, I was just preserving the old behavior here. Going through the file, it looks like the default collection value is set to PC1 everywhere. I was under the impression that 'latest' will correspond to latest collection which is PC1 now. The latest collection needs to be updated as we move to the next collection.

I will update the spec test

'latest' would be puppet6 at this point. PC1 was the original collection, then puppet5, and puppet6 is tracking the future release of puppet6.

Aha, 'latest' has a different meaning in that context then :) I took the word for its meaning.

hmm..but the get_puppet_collection() is called with value 'latest', not 'oldest' from install_puppet_agent_pe_promoted_repo_on ?

Sorry, I was trying to figure out if you were taking 'latest' to mean 'oldest', but maybe we're talking about different things.

jpartlow · 2018-08-07T17:21:52Z

lib/beaker-puppet/install_utils/foss_utils.rb

@@ -1293,7 +1293,7 @@ def install_puppet_agent_pe_promoted_repo_on( hosts, opts )
            opts[:download_url] = "#{opts[:pe_promoted_builds_url]}/puppet-agent/#{ pe_ver }/#{ opts[:puppet_agent_version] }/repos"
            opts[:copy_base_local]    ||= File.join('tmp', 'repo_configs')
            opts[:copy_dir_external]  ||= host.external_copy_base
-            opts[:puppet_collection] ||= 'PC1'
+            opts[:puppet_collection] ||= get_puppet_collection(opts[:puppet_agent_version])


Is it straight forward to add a foss_utils_spec.rb that verifies paths for different puppet_agent_versions?

I haven't looked at the spec test yet. Let me see if I can add that

There is no spec test for install_puppet_agent_pe_promoted_repo_on() in foss_utils.rb :(

@jpartlow Updated the PR to include a test for verifying the path

jpartlow · 2018-08-09T23:36:58Z

spec/beaker-puppet/install_utils/foss_utils_spec.rb

+      allow( subject ).to receive( :options ).and_return( opts )
+      allow( subject ).to receive( :scp_to )
+      allow( subject ).to receive( :configure_type_defaults_on ).with(host)
+      expect( subject ).to receive( :get_puppet_collection).with('agentversion').and_return(collection)


Beaker is horrible to unit test, since the method is all side effects, but it shouldn't be necessary to mock get_puppet_collection(), that's part of what we are testing after all, and it's just a function.

Yes. Everything after .with('agentversion') should be dropped if possible.

Hmm, that would cause nothing to be returned from get_puppet_collection. I was suggesting that it not be mocked it at all.

That's my mistake, I thought that spec was for the correct invocation of method #get_puppet_collection, not for the whole calling method. This does seem to be a lot of mocking for the method under consideration.

shaigy · 2018-08-10T18:11:28Z

@jpartlow @Dakta Dropped get_puppet_collection() and added tests for different agent versions and related puppet collection in the file paths instead

Dakta

Coverage of all codepaths and specificity of mocks. 👍

jpartlow · 2018-08-10T22:30:42Z

@shaigy have you verified that this patch fixes the legacy agent tests? (I think that's what this was for?)

shaigy · 2018-08-10T23:19:21Z

Yes, this is to resolve the legacy agent installation failures here
https://cinext-jenkinsmaster-enterprise-prod-1.delivery.puppetlabs.net/view/pe-integration/view/pe-2018.1.x/job/enterprise_pe-acceptance-tests_integration-system_pe_with-legacy-agents_nightly_2018.1.x/

I validated the changes in beaker 3.0 since we haven't sorted out the dependency issues yet with beaker 4.0 in pe_accceptance_test suite.
Validated with legacy_agent version 5.5.4 (which is currently failing in CI) and 5.5.3 and both of them passed.

$ LEGACY_AGENT_VERSION=2018.1.3%3A5.5.4 bundle exec beaker -h ../legacy-agent.cfg --pre-suite setup/install.rb --helper lib/beaker_helper.rb
...
setup/install.rb passed in 475.73 seconds
Test Suite: pre_suite @ 2018-08-10 15:50:52 -0700

  - Host Configuration Summary -


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

$ LEGACY_AGENT_VERSION=2018.1.3%3A5.5.3 bundle exec beaker -h ../legacy-agent.cfg --pre-suite setup/install.rb --helper lib/beaker_helper.rb
....
setup/install.rb passed in 644.65 seconds
Test Suite: pre_suite @ 2018-08-10 16:04:40 -0700

  - Host Configuration Summary -


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

Since we move to frictionless agent install in CI I think this is the only job that will use this method of installation.

Legacy agent installation job testing older agents with new PE versions in Integration CI does not use frictionless installation method. Instead it downloads the agent package from pm.puppetlabs.com. Agent installation in that case fails since the puppet_collection value is set to default 'PC1' instead of 'puppet5' causing the construction of wrong 'onhost_copied_file' path. Added a method to return the correct puppet_collection depending on the agent version and use that value instead of the default

Dakta · 2018-08-13T17:21:57Z

Jenkins, retest this please.

Dakta · 2018-08-13T17:22:44Z

Jenkins, retest this please.

cthorn42

LGTM

shaigy · 2018-08-13T18:40:31Z

@Dakta Got a +1 from Chris. Let me know if anything else is needed.

shaigy added the blocked label Aug 6, 2018

jpartlow reviewed Aug 7, 2018

View reviewed changes

shaigy force-pushed the PE-24898-find-puppet-collection-associated-with-agent-version branch 2 times, most recently from de331bf to 620bd36 Compare August 9, 2018 22:05

shaigy removed the blocked label Aug 9, 2018

jpartlow reviewed Aug 9, 2018

View reviewed changes

shaigy force-pushed the PE-24898-find-puppet-collection-associated-with-agent-version branch from 620bd36 to 3801bdb Compare August 10, 2018 18:06

Dakta approved these changes Aug 10, 2018

View reviewed changes

shaigy force-pushed the PE-24898-find-puppet-collection-associated-with-agent-version branch from 3801bdb to 8b56710 Compare August 10, 2018 23:24

shaigy force-pushed the PE-24898-find-puppet-collection-associated-with-agent-version branch from 8b56710 to 110bb6b Compare August 13, 2018 16:53

cthorn42 approved these changes Aug 13, 2018

View reviewed changes

Dakta merged commit 252215a into puppetlabs:master Aug 13, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PE-24898 Set puppet collection value associated with agent version #60

PE-24898 Set puppet collection value associated with agent version #60

shaigy commented Aug 6, 2018

puppetlabs-jenkins commented Aug 6, 2018

shaigy commented Aug 6, 2018

jpartlow Aug 7, 2018 •

edited

Loading

shaigy Aug 7, 2018

shaigy Aug 7, 2018

jpartlow Aug 7, 2018

shaigy Aug 7, 2018

jpartlow Aug 7, 2018

shaigy Aug 7, 2018

jpartlow Aug 7, 2018

jpartlow Aug 7, 2018

shaigy Aug 7, 2018

shaigy Aug 7, 2018

shaigy Aug 9, 2018

jpartlow Aug 9, 2018

Dakta Aug 10, 2018

jpartlow Aug 10, 2018

Dakta Aug 10, 2018

shaigy commented Aug 10, 2018

Dakta left a comment

jpartlow commented Aug 10, 2018

shaigy commented Aug 10, 2018

Dakta commented Aug 13, 2018

Dakta commented Aug 13, 2018

cthorn42 left a comment

shaigy commented Aug 13, 2018

PE-24898 Set puppet collection value associated with agent version #60

PE-24898 Set puppet collection value associated with agent version #60

Conversation

shaigy commented Aug 6, 2018

puppetlabs-jenkins commented Aug 6, 2018

shaigy commented Aug 6, 2018

jpartlow Aug 7, 2018 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shaigy commented Aug 10, 2018

Dakta left a comment

Choose a reason for hiding this comment

jpartlow commented Aug 10, 2018

shaigy commented Aug 10, 2018

Dakta commented Aug 13, 2018

Dakta commented Aug 13, 2018

cthorn42 left a comment

Choose a reason for hiding this comment

shaigy commented Aug 13, 2018

jpartlow Aug 7, 2018 •

edited

Loading