Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instructions to get this working out of the box #3

Open
brisbane opened this issue Aug 23, 2013 · 3 comments
Open

Instructions to get this working out of the box #3

brisbane opened this issue Aug 23, 2013 · 3 comments

Comments

@brisbane
Copy link
Member

All the recent changes to this torque module are all very nice. I don't necessarily think the modules should work out of the box, but I think there should be some instructions to make them work. One of the design goals I think should be that anyone could look in params.pp and see what parameters were needed. There are at least some parameters that are undocumented, e.g. "torque_queues"

Also I think this module should work when there are no queues yet defined on the server when running some of the Execs that are defined. This just seemed to need a bit of error handling in the custom ruby. I am not sure about pushing partial fixes that I cant possibly test, so I am leaving that for now. I seem to be getting the following error:
Error: Could not retrieve local facts: private method `split' called for nil:NilClass

Finally, there should be the option not to manage the maui config file or the queues. In the spirit of collaboration on all of this, I was indenting to add this and push it up as a pull request, but I don't think I am the best person to handle this

@rwf14f
Copy link
Contributor

rwf14f commented Aug 27, 2013

I agree, we need to add instructions on how to use the module. I've added some more comments for the torque parameters in params.pp.

The problem with the missing torque queues is fixed, it should work without defining queues now. I'm thinking about rewriting the queue configuration at some point to use a torque::server::queue class, but I don't have the time right now.

There is no need to add this as an option. It is up to the user on how to combine the classes. Wrapper classes such as torque::server are mere convenience classes for one general case, if you want to combine the modules differently then you can write your own wrapper class and only include the classes you want. For example you could use the following class:

class local::torque {
  file { '/var/spool/maui/maui.cfg': }
  class { 'torque::server::install': }
  class { 'torque::server::config': }
  class { 'torque::server::service': }
  class { 'torque::munge': }
  class { 'torque::maui::install': }
  class { 'torque::maui::service': }
}

The file definition is there to satisfy the dependency in maui::service.

@brisbane
Copy link
Member Author

I made a few changes to the torque module. I hope that I have been able to
make it work with hiera and to split out subsets of configuration without
affecting the way you use it.

I like the suggestion to use a wrapper class to get the aspects of the
functionality desired. For the moment I have resisted the temptation to
include a 'torque::server_simple' manifest in the class to install and
configure basic things, and leave the queues and maui.cfg as a manual
step. I am resisting using the full feature set of the module as I expect
the extra complexity will increase vulnerability to bugs. However, I am
running a Tier-3 and I suppose adding queues more frequently and altering
maui parameters, so keeping the config file operations as a manual step
makes more sense in my case.

However, when I try to add two queues at once I get the following error:

Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
Max open servers: 10239
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:

server commands

Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:

queue commands

Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
create queue normal
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal create queue normal
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
qmgr: Syntax error - cannot locate attribute
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal create queue normal
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr
update]/returns: ^
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal queue_type = Execution
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal resources_max.cput = 172:00:00
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal resources_max.walltime = 200:00:00
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal enabled = True
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal started = False
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:
set queue normal acl_group_enable = True
Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]/returns:

node commands

Notice: /Stage[main]/Torque::Server::Qmgrconfig/Exec[qmgr update]:
Triggered 'refresh' from 1 events
Notice: /Stage[main]/Pp_client::Mounts/Mount[/system]/ensure: ensure
changed 'unmounted' to 'mounted'

With this hiera configuration

torque::params::torque_qmgr_queues:
normal:
- queue_type = Execution
- resources_max.cput = 172:00:00
- resources_max.walltime = 200:00:00
- enabled = True
- started = False
- acl_group_enable = True
short:
- queue_type = Execution
- resources_max.cput = 12:00:00
- resources_max.walltime = 15:00:00
- enabled = True
- started = False
- acl_group_enable = True

On 27 August 2013 15:15, rwf14f [email protected] wrote:

I agree, we need to add instructions on how to use the module. I've added
some more comments for the torque parameters in params.pp.

The problem with the missing torque queues is fixed, it should work
without defining queues now. I'm thinking about rewriting the queue
configuration at some point to use a torque::server::queue class, but I
don't have the time right now.

There is no need to add this as an option. It is up to the user on how to
combine the classes. Wrapper classes such as torque::server are mere
convenience classes for one general case, if you want to combine the
modules differently then you can write your own wrapper class and only
include the classes you want. For example you could use the following class:

class local::torque {
file { '/var/spool/maui/maui.cfg': }
class { 'torque::server::install': }
class { 'torque::server::config': }
class { 'torque::server::service': }
class { 'torque::munge': }
class { 'torque::maui::install': }
class { 'torque::maui::service': }
}

The file definition is there to satisfy the dependency in maui::service.


Reply to this email directly or view it on GitHubhttps://github.com//issues/3#issuecomment-23339537
.

@rwf14f
Copy link
Contributor

rwf14f commented Aug 30, 2013

The problems with the queue configuration should be fixed now.

I've had a quick look through the changes and they shouldn't cause any problem. I haven't tested them myself. I use my own fork of the module and I haven't merged your changes into it yet.
I'm not sure if we should keep the hiera calls in params.pp in the master branch, I was hoping to keep it free of explicit hiera calls. I've been using hiera with APLs, so it was working with hiera before. There's also a performance issue with having explicit hiera calls in params.pp and using parameterized classes (APL) at the same time: puppet 3.x will try to lookup the same variable twice, causing the number of hiera lookups to at least double. For example if you use the torque::server::config class, it will inherit the params class and look up all variables in it. If you don't pass parameters to torque::server::config it then does another hiera lookup (APL) before using the values from params. I would prefer to use params just as a collection of all parameters of a module and their default values, plus some additional documentation (APL hiera names, parameter structure). I think that a wrapper approach would be better here, e.g.,

class { 'torque::server::config':
  torque_server => hiera('scope::torque_server', $torque::server::params::torque_server_name)
}

I would leave it to sites to put hiera calls into params, I think the module in HEP-puppet shouldn't.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants