Circular deps not detected

[PierreR]

I have experienced circular deps runtime error when running puppet agent on a node.

Is it something that puppetresources is supposed to catch up early ?

If so, I can provide use case when it happen.

[bartavelle]

I think it should error early. I don't have much time these days but will try to see how hard it is to fix this problem.

[bartavelle]

So there is indeed code that should find circular dependencies, but it didn't find yours. That means there is as sort of dependency that is not accurately tracked by language-puppet. Do you have this example available?

[PierreR]

I believe something like this should fail:

   user { 'jenkins':
    managehome => false,
    gid  => '501',
  }
  group { 'jenkins':
    ensure => present,
    gid => '501';
  }
  file {
    "${data_dir}/jenkins":
      ensure => directory,
      owner  => 'jenkins',
      group  => 'jenkins';
    '/var/lib/jenkins':
      ensure  => 'link',
      target  => "${data_dir}/jenkins",
      require => File["${data_dir}/jenkins"],
      before  => User['jenkins'];
}

I know it does pass puppetresources but I haven't double-checked the fact that it would fail on a puppet master (though it seem logical to believe it should fail as the first file resource has an implicit deps on user through the 'owner' property).

[bartavelle]

I am writing a fix for this, but it has been a while since I used puppet. Does language-puppet complain if you specify an owner that doesn't have a corresponding user resource declaration?

[PierreR]

I am pretty sure it does. Do you want me to check that out ?

[bartavelle]

Nope it's what I thought too

[PierreR]

FWIW there is this file to customize knowing users and groups (and an --extra-tests flag to disable such checks altogether): https://github.com/bartavelle/language-puppet/blob/master/tests/defaults.yaml

[bartavelle]

Can you check the last commit didn't break everything ?

[PierreR]

I have tested it with a couple of nodes and it looks alright. If you release the fix in hackage, it will be easier for me to test it across all my nodes. Thanks

[bartavelle]

Done