Project

General

Profile

Actions

Bug #3865

closed

rudder-upgrade prints an unnecessary warning: "WARNING: Some event log are still based on an old file format (file format 1), please upgrade first to 2.6 to make this migration"

Added by Olivier Mauras over 9 years ago. Updated almost 8 years ago.

Status:
Released
Priority:
4
Category:
Packaging
Target version:
Severity:
UX impact:
User visibility:
Effort required:
Priority:
Regression:

Description

  1. /opt/rudder/bin/rudder-upgrade
    INFO: Checking LDAP service status... OK
    INFO: Checking PostgreSQL service status... OK
    WARNING: Some event log are still based on an old file format (file format 1), please upgrade first to 2.6 to make this migration
    INFO: Modifying system group entries in LDAP if necessary... Done

I indeed have never installed another version than 2.7.0

Actions #1

Updated by Nicolas PERRON over 9 years ago

  • Assignee set to François ARMAND
  • Priority changed from N/A to 3
  • Target version set to 2.7.1

Olivier Mauras wrote:

  1. /opt/rudder/bin/rudder-upgrade
    INFO: Checking LDAP service status... OK
    INFO: Checking PostgreSQL service status... OK
    WARNING: Some event log are still based on an old file format (file format 1), please upgrade first to 2.6 to make this migration
    INFO: Modifying system group entries in LDAP if necessary... Done

I indeed have never installed another version than 2.7.0

François can you look into this please ? I'm not sure but I think this is due to the webapp

Actions #2

Updated by François ARMAND over 9 years ago

Yeah, it is due to the webapp, will look to it.

Actions #3

Updated by François ARMAND over 9 years ago

OK, so in fact it is not from the webapp, but from the migration script (line around 199).

The problem is that in 2.3 -> 2.4, we migrated "addPending" elements, saying that a deployement is pending, to "addPendingDeployement" elements.

But somewhere, the code was not updated in the generated XML, so that we STILL have the old 2.3 elements for addPending. That's really horrible.

Moreover, it's a risky change (a lot of place are "eventlog format" dependant: displaying them, etc. And are likelly to break.

So I propose to simply ignore these in 2.4 -> 2.7, and do the migration for 2.8.

And the correction would be to just remove the line:

RES2=$(su - postgres -c "psql -t -d rudder -c \"select count(*) from (select xpath('/entry/addPending',data) AS x from eventlog) as Y where array_upper(x, 1) > 0;\"")

which appears 2 times in rudder packages: rudder-webapp/SOURCES/rudder-upgrade (migration script).

Actions #4

Updated by François ARMAND over 9 years ago

  • Status changed from New to Discussion
  • Assignee changed from François ARMAND to Nicolas CHARLES

Nicolas, what do you think ?

Actions #5

Updated by Nicolas CHARLES over 9 years ago

  • Assignee changed from Nicolas CHARLES to François ARMAND

As discussed with Francois, it makes indeed no sense to correct it in 2.7.
This data is never used, and it's risky to create a new migration script for that.

We'd better ignore this, and in 2.8 create a larger migration script that would migrate the addPending, without taking into account the format version of the eventlog

Actions #6

Updated by François ARMAND over 9 years ago

  • Status changed from Discussion to 8
  • Assignee changed from François ARMAND to Nicolas PERRON
  • Target version changed from 2.7.1 to 2.4.8

OK, so the "correct the migration logic" is here: #3893".

For that one, Nicolas, could you update the migration script as discribed in comment #3 ?

It need to be done in 2.4.

Actions #7

Updated by François ARMAND over 9 years ago

  • Project changed from 24 to 34
Actions #8

Updated by Nicolas PERRON over 9 years ago

  • Status changed from 8 to Pending technical review
  • % Done changed from 0 to 100
  • Pull Request set to https://github.com/Normation/rudder-packages/pull/109

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

Jon, could you review it please ?

Actions #9

Updated by Jonathan CLARKE over 9 years ago

  • Status changed from Pending technical review to Discussion
  • Assignee changed from Nicolas PERRON to François ARMAND

Could you clarify what the impact of this bug is, if any? I don't know what a addPending is so doubt any of our users do :-)

Actions #10

Updated by François ARMAND over 9 years ago

  • Status changed from Discussion to 8
  • Assignee changed from François ARMAND to Nicolas PERRON

In the pull request, ALL the migration is deleted. Only the part about "addPending" should.

Actions #11

Updated by François ARMAND over 9 years ago

Jonathan CLARKE wrote:

Could you clarify what the impact of this bug is, if any? I don't know what a addPending is so doubt any of our users do :-)

The bug is about a migration that can not be done anymore because it should be only relevant for 2.3 -> 2.4 upgrade. But what I discover is that we NEVER changed the actual data saved from the old format to the new one (at the data generation point). So actually, we still have the old data in the database, and that's why the migration script say "that's not normal".

But actually, the not-migrated-datas are not used anywhere in Rudder. The are details information about the "add pending deployment" event, which is generated when a deployment is started (iether automatically or by the user) while an other is still processing. So the second one is queued, so that it is execute after the first is finished. And if again one another deployment is requested, then it is ignored (only one pending is sufficient). The details is here to know if the "pending deployment" was actually added or ignored because one is already pending.

So, as the data are not used (only saved in base for a brilliant future), there is no point to try to migrate in 2.4, 2.6 (2.7) (because that mean addind complexe/risky code to stable releases), and so the bug correction is just to remove the check for the status of that data.

Hope it's clearer now.

Actions #12

Updated by Nicolas PERRON over 9 years ago

  • Status changed from 8 to Discussion
  • Assignee changed from Nicolas PERRON to François ARMAND

François ARMAND wrote:

Jonathan CLARKE wrote:

Could you clarify what the impact of this bug is, if any? I don't know what a addPending is so doubt any of our users do :-)

The bug is about a migration that can not be done anymore because it should be only relevant for 2.3 -> 2.4 upgrade. But what I discover is that we NEVER changed the actual data saved from the old format to the new one (at the data generation point). So actually, we still have the old data in the database, and that's why the migration script say "that's not normal".

But actually, the not-migrated-datas are not used anywhere in Rudder. The are details information about the "add pending deployment" event, which is generated when a deployment is started (iether automatically or by the user) while an other is still processing. So the second one is queued, so that it is execute after the first is finished. And if again one another deployment is requested, then it is ignored (only one pending is sufficient). The details is here to know if the "pending deployment" was actually added or ignored because one is already pending.

So, as the data are not used (only saved in base for a brilliant future), there is no point to try to migrate in 2.4, 2.6 (2.7) (because that mean addind complexe/risky code to stable releases), and so the bug correction is just to remove the check for the status of that data.

Hope it's clearer now.

Not sure to understand. If the migration never worked, why not removing it instead of removing one of its two check used to trigger the migration ?

Actions #13

Updated by François ARMAND over 9 years ago

  • Assignee changed from François ARMAND to Nicolas PERRON

The migration for the part about addpending never worked. All other migration about fileFormat evolution DO work, and we DO need them.

Actions #14

Updated by Nicolas PERRON over 9 years ago

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

François ARMAND wrote:

The migration for the part about addpending never worked. All other migration about fileFormat evolution DO work, and we DO need them.

OK. Done.

Jon could you review it now this seems clearer ?

Actions #15

Updated by Jonathan CLARKE over 9 years ago

  • Subject changed from Strange warning from rudder-upgrade to rudder-upgrade prints an unnecessary warning: "WARNING: Some event log are still based on an old file format (file format 1), please upgrade first to 2.6 to make this migration"
  • Priority changed from 3 to 4

Thanks for clarifying François, this makes it clear to me that this is not an impacting bug, just a minor annoyance (unwanted WARNING in stdout). I will validate the Pull Request, and have updated the bug's title.

Actions #16

Updated by Nicolas PERRON over 9 years ago

  • Status changed from Pending technical review to Pending release

Applied in changeset commit:38956043fdf64835e250d7f400f797ea4f00783a.

Actions #17

Updated by Nicolas PERRON over 9 years ago

Applied in changeset commit:9e011c54d413bb2c32b2f90666a0dc7fdaed8a49.

Actions #18

Updated by Jonathan CLARKE over 9 years ago

Applied in changeset commit:7a3894d9ea4e856985264bda077bef904b76d5b1.

Actions #19

Updated by Nicolas PERRON over 9 years ago

Wrong bug number of previous commits. I've fixed them

Actions #20

Updated by Nicolas PERRON over 9 years ago

  • Status changed from Pending release to Released

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

Actions #21

Updated by Benoît PECCATTE almost 8 years ago

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

Also available in: Atom PDF