server_url if supplied by itself does not allow download_url to derive properly.

[brettswift]

There is a problem with param chaining in this module.

If I use the code below, server_url is set to the agent.pp class. Yet download_url is brought from params.pp, where server_url gets it's default http://builder.

So the code below fails as the server_url is not injected.

  class {'::teamcity::agent':
    agent_name            => "${hostname}-agent",
    manage_user           => true,
    manage_group          => true,
    server_url            => 'http://10.9.32.144',
  }

One clean pattern to achieve this is to have only your params come in through the main module class - init.pp.

Init.pp can derive it's defaults from params.pp (as agent.pp is doing now).

Then I only have to set server_url and let the download_url derive. Right now I have to set things explicitly deeper in the module than I should have to.

[grundic]

@brettswift, that's strange. I will investigate this issue as soon as possible. And thank you for provided ideas.

[grundic]

@brettswift I can't reproduce your case. Could you please give more details? Here is my try:

vagrant@vagrant-ubuntu-wily-64:~$ cat issue_10.pp
class {'::teamcity::agent':
  agent_name            => '${hostname}-agent',
  manage_user           => true,
  manage_group          => true,
  server_url            => 'https://teamcity.jetbrains.com'
}
vagrant@vagrant-ubuntu-wily-64:~$ sudo puppet apply issue_10.pp
Warning: Config file /etc/puppet/hiera.yaml not found, using Hiera defaults
Notice: Compiled catalog for vagrant-ubuntu-wily-64 in environment production in 1.10 seconds
Notice: /Stage[main]/Teamcity::Agent::Install/Exec[extract-agent-archive]/returns: executed successfully
Notice: /Stage[main]/Teamcity::Agent::Install/File[/opt/build-agent/bin/agent.sh]/mode: mode changed '0644' to '0755'
Notice: /Stage[main]/Teamcity::Agent::Install/File[agent-config]/ensure: defined content as '{md5}b7b353774bd93642e668e5d75f32b911'
Notice: /Stage[main]/Teamcity::Agent::Install/Exec[chown-agent-dir]: Triggered 'refresh' from 1 events
Notice: /Stage[main]/Teamcity::Agent::Config/File[/lib/systemd/system/build-agent.service]/ensure: created
' to 'https://teamcity.jetbrains.com':Config/Ini_setting[[] serverUrl]/value: value changed 'http://localhost:8111/
' to '${hostname}-agent'mcity::Agent::Config/Ini_setting[[] name]/value: value changed '
Notice: /Stage[main]/Teamcity::Agent::Service/Service[build-agent]/ensure: ensure changed 'stopped' to 'running'
Notice: /Stage[main]/Teamcity::Agent::Service/Exec[systemd_reload]: Triggered 'refresh' from 1 events
Notice: Finished catalog run in 0.76 seconds
vagrant@vagrant-ubuntu-wily-64:~$
vagrant@vagrant-ubuntu-wily-64:~$ grep jetbrains /opt/build-agent/conf/buildAgent.properties
serverUrl=https://teamcity.jetbrains.com
vagrant@vagrant-ubuntu-wily-64:~$

[brettswift]

Hmm. What version of puppet are you on? Maybe it's a puppet 4 thing? I believe I was on 4.3.2. ( PE 2015.2)

On Sun, Feb 21, 2016 at 02:09 Grigory Chernyshev notifications@github.com wrote:

@brettswift https://github.com/brettswift I can't reproduce your case. Could you please give more details? Here is my try:

vagrant@vagrant-ubuntu-wily-64:~$ cat issue_10.pp class {'::teamcity::agent': agent_name => '${hostname}-agent', manage_user => true, manage_group => true, server_url => 'https://teamcity.jetbrains.com' } vagrant@vagrant-ubuntu-wily-64:~$ sudo puppet apply issue_10.pp Warning: Config file /etc/puppet/hiera.yaml not found, using Hiera defaults Notice: Compiled catalog for vagrant-ubuntu-wily-64 in environment production in 1.10 seconds Notice: /Stage[main]/Teamcity::Agent::Install/Exec[extract-agent-archive]/returns: executed successfully Notice: /Stage[main]/Teamcity::Agent::Install/File[/opt/build-agent/bin/agent.sh]/mode: mode changed '0644' to '0755' Notice: /Stage[main]/Teamcity::Agent::Install/File[agent-config]/ensure: defined content as '{md5}b7b353774bd93642e668e5d75f32b911' Notice: /Stage[main]/Teamcity::Agent::Install/Exec[chown-agent-dir]: Triggered 'refresh' from 1 events Notice: /Stage[main]/Teamcity::Agent::Config/File[/lib/systemd/system/build-agent.service]/ensure: created' to 'https://teamcity.jetbrains.com':Config/Ini_setting[[] serverUrl]/value: value changed 'http://localhost:8111/' to '${hostname}-agent'mcity::Agent::Config/Ini_setting[[] name]/value: value changed ' Notice: /Stage[main]/Teamcity::Agent::Service/Service[build-agent]/ensure: ensure changed 'stopped' to 'running' Notice: /Stage[main]/Teamcity::Agent::Service/Exec[systemd_reload]: Triggered 'refresh' from 1 events Notice: Finished catalog run in 0.76 seconds vagrant@vagrant-ubuntu-wily-64:~$ vagrant@vagrant-ubuntu-wily-64:~$ grep jetbrains /opt/build-agent/conf/buildAgent.properties serverUrl=https://teamcity.jetbrains.com vagrant@vagrant-ubuntu-wily-64:~$

— Reply to this email directly or view it on GitHub https://github.com/grundic/puppet-teamcity/issues/10#issuecomment-186778414 .

[grundic]

Looks like Puppet 4 issue. Will investigate further.

[brettswift]

There were some relaxed auto lookup behaviours in puppet 3.

I'm surprised the code as is worked on puppet 3.

We've had a lot of success by forcing all Params into init.pp and using Params.pp.

It makes the code a bit easier to follow for people new to the module as well

On Mon, Feb 22, 2016 at 02:43 Grigory Chernyshev notifications@github.com wrote:

Looks like Puppet 4 issue. Will investigate further.

— Reply to this email directly or view it on GitHub https://github.com/grundic/puppet-teamcity/issues/10#issuecomment-187096782 .

[grundic]

I've made a fix in separate branch, could you please check it?

[brettswift]

I'll see if I can.. I did a quick test of different modules and had success with another one on puppet4, so I'm not set up to test this.. but I'll see what I can do tomorrow.

The code looks like it'd work.

Another tiny code review item which I try to do myself is avoid a lot of conditional logic in one file. Separate conditional logic from puppet resources. I've solved it in the past by having the logic just run "include ", and that class can just do it's thing without any logic.

You end up with quite a few more manifests but it's easier to read.

I've tried to do that on my sensu_suite module, but I don't support windows like this module does.. so it's a little simpler on that module.

I may switch back to this one at some point. Just trying to help!

[grundic]

@brettswift, thank you for your feedback, appreciate it very much. Refactoring of multiple "if" statements is next what I was going to do, but after implementing some tests. I'll take a look at your module and to elasticsearch one and try to implement best practices.

As you can see, this module is old enough, so there are some historical problems :)

[brettswift]

Good stuff! On Tue, Feb 23, 2016 at 00:12 Grigory Chernyshev notifications@github.com wrote:

@brettswift https://github.com/brettswift, thank you for your feedback, appreciate it very much. Refactoring of multiple "if" statements is next what I was going to do, but after implementing some tests. I'll take a look at your module and to elasticsearch one and try to implement best practices.

As you can see, this module is old enough, so there are some historical problems :)

— Reply to this email directly or view it on GitHub https://github.com/grundic/puppet-teamcity/issues/10#issuecomment-187577532 .

[grundic]

Released new version with all requirements. Tell me if something is not working, or you miss some additional functionality.

[brettswift]

Hey ! Thanks! I definitely will. I appreciate the attention on this. On Wed, Mar 2, 2016 at 15:37 Grigory Chernyshev notifications@github.com wrote:

Closed #10 https://github.com/grundic/puppet-teamcity/issues/10.

— Reply to this email directly or view it on GitHub https://github.com/grundic/puppet-teamcity/issues/10#event-575422376.

[brettswift]

Well I did try this on a CentOS box. I like that there is windows support here - although picked a different module as it already had the server support.

Just coming back to this now, and have another failure on CentOS. I'll create an issue for that - but just wanted to update this thread and let you know the fix for puppet 4 gets through the compiler for me as well.