Project

General

Profile

Actions

Bug #3929

closed

rudder-jetty does not consider /etc/default/jetty as a config file and replace it silently during upgrade

Added by Nicolas PERRON over 11 years ago. Updated almost 10 years ago.

Status:
Released
Priority:
1 (highest)
Assignee:
Jonathan CLARKE
Category:
Packaging
Target version:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
Name check:
Fix check:
Regression:

Description

The package rudder-jetty replace silently the file /etc/default/jetty as it is not marked as a config file.

Actions #1

Updated by Nicolas PERRON over 11 years ago

  • Status changed from New to Pending technical review
  • Assignee set to Jonathan CLARKE
  • % Done changed from 0 to 100
  • Pull Request set to https://github.com/Normation/rudder-packages/pull/121

With /etc/default/jetty file set as a config file now, each modification will need a migration script.

Pull Request URL added: https://github.com/Normation/rudder-packages/pull/121

Jon, Could you review it please ?

Actions #2

Updated by Jonathan CLARKE over 11 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Jonathan CLARKE to Nicolas PERRON

I'm not sure I agree with this approach.

To my mind, jetty is, and should remain, an internal component of Rudder. Rudder users should not have to worry about jetty, no more than they have to worry about what Javascript libraries the webapp uses. Therefore, I am not surprised that we overried jetty's config files.

IIRC, the original need for this is to be able to change the amount of RAM allocated to the JVM? Could we not extract this from a more "normal" config file for Rudder somewhere? Maybe even a /etc/default/rudder-server-root?

Actions #3

Updated by Nicolas PERRON over 11 years ago

  • Assignee changed from Nicolas PERRON to Jonathan CLARKE

Jonathan CLARKE wrote:

I'm not sure I agree with this approach.

To my mind, jetty is, and should remain, an internal component of Rudder. Rudder users should not have to worry about jetty, no more than they have to worry about what Javascript libraries the webapp uses. Therefore, I am not surprised that we overried jetty's config files.

IIRC, the original need for this is to be able to change the amount of RAM allocated to the JVM? Could we not extract this from a more "normal" config file for Rudder somewhere? Maybe even a /etc/default/rudder-server-root?

Ok but how would you get the informations from /etc/default/rudder-server-root ? Sourcing it from /etc/default/jetty ? It seems bizarre to source a default file from another one doesn't it ?

Actions #4

Updated by Jonathan CLARKE over 11 years ago

Nicolas PERRON wrote:

Jonathan CLARKE wrote:

I'm not sure I agree with this approach.

To my mind, jetty is, and should remain, an internal component of Rudder. Rudder users should not have to worry about jetty, no more than they have to worry about what Javascript libraries the webapp uses. Therefore, I am not surprised that we overried jetty's config files.

IIRC, the original need for this is to be able to change the amount of RAM allocated to the JVM? Could we not extract this from a more "normal" config file for Rudder somewhere? Maybe even a /etc/default/rudder-server-root?

Ok but how would you get the informations from /etc/default/rudder-server-root ? Sourcing it from /etc/default/jetty ? It seems bizarre to source a default file from another one doesn't it ?

I'm not proposing a technical solution here, that's your job :p

My statement is simple: Rudder aims to be a simple to use configuration automation solution. Configuring RAM for Java in a Jetty config file does not fit that description. So we should not get our users to do this. However, it is apparently not possible to tell Java to "just use the RAM you need" (grrrrrr), so we need to be able to set that. That should appear to the user as a Rudder setting, not a jetty setting (they should never need to hear about jetty).

Actions #5

Updated by Jonathan CLARKE over 11 years ago

  • Assignee changed from Jonathan CLARKE to Nicolas PERRON
Actions #6

Updated by Nicolas PERRON over 11 years ago

  • Assignee changed from Nicolas PERRON to Jonathan CLARKE

Jonathan CLARKE wrote:

Nicolas PERRON wrote:

Jonathan CLARKE wrote:

I'm not sure I agree with this approach.

To my mind, jetty is, and should remain, an internal component of Rudder. Rudder users should not have to worry about jetty, no more than they have to worry about what Javascript libraries the webapp uses. Therefore, I am not surprised that we overried jetty's config files.

IIRC, the original need for this is to be able to change the amount of RAM allocated to the JVM? Could we not extract this from a more "normal" config file for Rudder somewhere? Maybe even a /etc/default/rudder-server-root?

Ok but how would you get the informations from /etc/default/rudder-server-root ? Sourcing it from /etc/default/jetty ? It seems bizarre to source a default file from another one doesn't it ?

I'm not proposing a technical solution here, that's your job :p

My statement is simple: Rudder aims to be a simple to use configuration automation solution. Configuring RAM for Java in a Jetty config file does not fit that description. So we should not get our users to do this. However, it is apparently not possible to tell Java to "just use the RAM you need" (grrrrrr), so we need to be able to set that. That should appear to the user as a Rudder setting, not a jetty setting (they should never need to hear about jetty).

OK, but the configuration of the JVM does not seem to me to be more clear if it's into the /etc/default/rudder-server-root instead of /etc/default/jetty. It's not possible to say "Eh, I want more RAM to be used in the JVM". There are at least two parameters for two differents zones of JVM: MaxPermSize and Xmx.

The best I can do is to use the same unfriendly names of /etc/default/jetty in /etc/default/rudder-server-root

Actions #7

Updated by Jonathan CLARKE over 11 years ago

  • Assignee changed from Jonathan CLARKE to Nicolas PERRON

Nicolas PERRON wrote:

Jonathan CLARKE wrote:

Nicolas PERRON wrote:

Jonathan CLARKE wrote:

I'm not sure I agree with this approach.

To my mind, jetty is, and should remain, an internal component of Rudder. Rudder users should not have to worry about jetty, no more than they have to worry about what Javascript libraries the webapp uses. Therefore, I am not surprised that we overried jetty's config files.

IIRC, the original need for this is to be able to change the amount of RAM allocated to the JVM? Could we not extract this from a more "normal" config file for Rudder somewhere? Maybe even a /etc/default/rudder-server-root?

Ok but how would you get the informations from /etc/default/rudder-server-root ? Sourcing it from /etc/default/jetty ? It seems bizarre to source a default file from another one doesn't it ?

I'm not proposing a technical solution here, that's your job :p

My statement is simple: Rudder aims to be a simple to use configuration automation solution. Configuring RAM for Java in a Jetty config file does not fit that description. So we should not get our users to do this. However, it is apparently not possible to tell Java to "just use the RAM you need" (grrrrrr), so we need to be able to set that. That should appear to the user as a Rudder setting, not a jetty setting (they should never need to hear about jetty).

OK, but the configuration of the JVM does not seem to me to be more clear if it's into the /etc/default/rudder-server-root instead of /etc/default/jetty. It's not possible to say "Eh, I want more RAM to be used in the JVM". There are at least two parameters for two differents zones of JVM: MaxPermSize and Xmx.

The best I can do is to use the same unfriendly names of /etc/default/jetty in /etc/default/rudder-server-root

Nicolas, you're missing the point here. My statement is simple: Rudder users should not know jetty even exists, so they should never have to edit /etc/default/jetty. However, it is often necessary to change the Java memory settings to have Rudder running. Please put these settings elsewhere - not in /etc/default/jetty, but I don't mind where else. /etc/default/rudder-server-root was just a suggestion, ignore it if you don't like it.

Actions #8

Updated by Nicolas PERRON about 11 years ago

  • Status changed from Discussion to Pending technical review
  • Assignee changed from Nicolas PERRON to Jonathan CLARKE

Jonathan CLARKE wrote:

Nicolas PERRON wrote:

Jonathan CLARKE wrote:

Nicolas PERRON wrote:

Jonathan CLARKE wrote:

I'm not sure I agree with this approach.

To my mind, jetty is, and should remain, an internal component of Rudder. Rudder users should not have to worry about jetty, no more than they have to worry about what Javascript libraries the webapp uses. Therefore, I am not surprised that we overried jetty's config files.

IIRC, the original need for this is to be able to change the amount of RAM allocated to the JVM? Could we not extract this from a more "normal" config file for Rudder somewhere? Maybe even a /etc/default/rudder-server-root?

Ok but how would you get the informations from /etc/default/rudder-server-root ? Sourcing it from /etc/default/jetty ? It seems bizarre to source a default file from another one doesn't it ?

I'm not proposing a technical solution here, that's your job :p

My statement is simple: Rudder aims to be a simple to use configuration automation solution. Configuring RAM for Java in a Jetty config file does not fit that description. So we should not get our users to do this. However, it is apparently not possible to tell Java to "just use the RAM you need" (grrrrrr), so we need to be able to set that. That should appear to the user as a Rudder setting, not a jetty setting (they should never need to hear about jetty).

OK, but the configuration of the JVM does not seem to me to be more clear if it's into the /etc/default/rudder-server-root instead of /etc/default/jetty. It's not possible to say "Eh, I want more RAM to be used in the JVM". There are at least two parameters for two differents zones of JVM: MaxPermSize and Xmx.

The best I can do is to use the same unfriendly names of /etc/default/jetty in /etc/default/rudder-server-root

Nicolas, you're missing the point here. My statement is simple: Rudder users should not know jetty even exists, so they should never have to edit /etc/default/jetty. However, it is often necessary to change the Java memory settings to have Rudder running. Please put these settings elsewhere - not in /etc/default/jetty, but I don't mind where else. /etc/default/rudder-server-root was just a suggestion, ignore it if you don't like it.

The Pull Request has been updated, could you review it please ?

Actions #9

Updated by Nicolas PERRON about 11 years ago

  • Target version changed from 2.4.9 to 2.4.10
Actions #10

Updated by Nicolas PERRON about 11 years ago

  • Target version changed from 2.4.10 to 2.4.11
Actions #11

Updated by Nicolas PERRON about 11 years ago

  • Target version changed from 2.4.11 to 2.4.12
Actions #12

Updated by Jonathan CLARKE about 11 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Jonathan CLARKE to Nicolas PERRON

This Pull Request does not look valid, and my comments above appear to have been ignored. This PR should not be merged.

Actions #13

Updated by Vincent MEMBRÉ about 11 years ago

  • Status changed from Discussion to In progress
  • Assignee changed from Nicolas PERRON to Matthieu CERDA
Actions #14

Updated by Matthieu CERDA about 11 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from Matthieu CERDA to Jonathan CLARKE
  • Pull Request changed from https://github.com/Normation/rudder-packages/pull/121 to https://github.com/Normation/rudder-packages/pull/173

New pull request created, using a file in /opt/rudder/etc as the config file and /etc/default/jetty as a regular file.

Actions #15

Updated by Matthieu CERDA about 11 years ago

  • Target version deleted (2.4.12)
For me, the solution to this is simple:
  • JCL considers the /etc/default/jetty file overwriting to be OK, but the memory values should be configurable at least.
  • NPE wanted to use /etc/default/rudder-server-root as a complement for this purpose.

As of what I understand, NPE tries to keep the /etc/default functionnality and JCL wants the file to be configurable if necessary, but only if there is no other choice. A regular user should not have to change this unless it is absolutely needed.

I adopted a compromise: The file /etc/default/jetty will stay and be configured by us only, but I enable the user to change at least the tunable memory settings in /opt/rudder/etc/rudder-jetty.conf if he wants to.

I keep sensible defaults in this file so a "standard" user will not even need to know it exists.

Actions #16

Updated by Matthieu CERDA about 11 years ago

  • Target version set to 2.8.0

This is a non-trivial feature change, it has no place in a bug fix only branch. Re-targetting to 2.6

Actions #17

Updated by Matthieu CERDA about 11 years ago

  • Pull Request changed from https://github.com/Normation/rudder-packages/pull/173 to https://github.com/Normation/rudder-packages/pull/174

Rebase complete.

Actions #18

Updated by Vincent MEMBRÉ about 11 years ago

  • Target version changed from 2.8.0 to 2.8.1

This won't be done until 2.8.1, maybe even later.

Actions #19

Updated by Nicolas PERRON about 11 years ago

  • Target version changed from 2.8.1 to 2.8.2
Actions #20

Updated by Dennis Cabooter about 11 years ago

Maybe I should not comment on this, since I'm just a normal user who knows nothing about Java or Jetty. I know Jetty exist, but I have never configured memory for it. To be honest, in whatever which file that could be done, I would never change memory settings unless someone from normation or the documentation advice me to and it's very clear for me what to change and where and why. Like the documentation advices about PostGres settings (which is also an internal part of Rudder).

Just my 2 cents :)

Actions #21

Updated by Vincent MEMBRÉ almost 11 years ago

  • Target version changed from 2.8.2 to 2.8.3
Actions #22

Updated by Matthieu CERDA almost 11 years ago

  • Target version changed from 2.8.3 to 2.9.3
  • Pull Request changed from https://github.com/Normation/rudder-packages/pull/174 to https://github.com/Normation/rudder-packages/pull/242

PR has been retargetted to 2.9, as requested.

Actions #23

Updated by Jonathan CLARKE almost 11 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Jonathan CLARKE to Matthieu CERDA

Matthieu CERDA wrote:

PR has been retargetted to 2.9, as requested.

The PR looks good to me, but there is no reason at the moment to target anything in 2.9. We either address old versions (2.6) or the next release (2.10), or by exception the latest non stable release (2.8). Please retarget this to 2.10.

Actions #24

Updated by Matthieu CERDA almost 11 years ago

  • Status changed from Discussion to In progress
  • Target version changed from 2.9.3 to 2.10.0~beta1
  • % Done changed from 100 to 80

OK, let's do this !

/me jumps around git branches

Actions #25

Updated by Matthieu CERDA almost 11 years ago

  • Status changed from In progress to Pending technical review
  • Assignee changed from Matthieu CERDA to Jonathan CLARKE
  • % Done changed from 80 to 100
  • Pull Request changed from https://github.com/Normation/rudder-packages/pull/242 to https://github.com/Normation/rudder-packages/pull/244

PR updated again !

Actions #26

Updated by Matthieu CERDA almost 11 years ago

  • Status changed from Pending technical review to Pending release

Applied in changeset commit:bd7e022a04cc31eb1b3a6bfefa249dce2f3f468f.

Actions #27

Updated by Jonathan CLARKE almost 11 years ago

Applied in changeset commit:9ecd436a4a8f0946d2cfdb3f19d8e37967374dc2.

Actions #28

Updated by Vincent MEMBRÉ over 10 years ago

  • Status changed from Pending release to Released

This bug has been fixed in Rudder 2.10.0~beta1, which was released today.
Check out:

The release announcement: http://www.rudder-project.org/pipermail/rudder-announce/2014-March/000084.html
The full ChangeLog: http://www.rudder-project.org/foswiki/bin/view/System/Documentation:ChangeLog210
Download information: https://www.rudder-project.org/site/get-rudder/downloads/
Actions #29

Updated by Benoît PECCATTE almost 10 years ago

  • Project changed from 34 to Rudder
  • Category set to Packaging
Actions

Also available in: Atom PDF