-
Notifications
You must be signed in to change notification settings - Fork 24
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
Fixes #9012: Remove migration scripts for 2.10-3.0 in 4.0 #1072
Fixes #9012: Remove migration scripts for 2.10-3.0 in 4.0 #1072
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great improvement in term of lisibility.
Appart from my comment on the first STEP, everything looks good to me
@@ -341,9 +328,6 @@ upgrade_ncf() { | |||
fi | |||
|
|||
if [ -f ${CONFIGURATION_REPOSITORY}/ncf/ncf.conf ]; then | |||
# 2.9.0 & 2.11.11, 3.0.5: Make sure that ncf uses the right logger bundles (_logger_default and logger_rudder) | |||
STEP="Make sure that ncf uses the right logger bundles (_logger_default and logger_rudder)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the comment should stay, but in a modified form, reflecting what is done below:
Make sure that ncf uses the right logger bundles (_log_default and log_rudder)"
if ! cat ${CONFIGURATION_REPOSITORY}/ncf/ncf.conf | perl -p0e "s/\n/%%%%%/g" | egrep -q "classes.community_edition[^%]*%%%%%cfengine_port=5309"; then | ||
cat ${CONFIGURATION_REPOSITORY}/ncf/ncf.conf | perl -p0e "s/\n/%%%%%/g" | sed "s/\(classes.community_edition[^%]*%%%%%cfengine_port=\)[0-9]\+/\15309/" | perl -p0e "s/%%%%%/\n/g" > ${CONFIGURATION_REPOSITORY}/ncf/ncf.conf.new | ||
mv ${CONFIGURATION_REPOSITORY}/ncf/ncf.conf.new ${CONFIGURATION_REPOSITORY}/ncf/ncf.conf | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, nice catch. it took me a while to understand that it was indeed not needed (ncf.conf has a lot of magic)
@@ -670,21 +557,6 @@ upgrade_inventory() { | |||
# Check if the LDAP has been initialised | |||
LDAP_EXISTS=$(${LDAPSEARCH} -b "cn=rudder-configuration" -s base dn 2>/dev/null | grep -c "dn: cn=rudder-configuration" || true) | |||
|
|||
## 2.4.0 : Change attribute from dn:ruleId=inventory-all,ou=Rules,ou=Rudder,cn=rudder-configuration |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was about time to remove it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a comment
# - 3.0.0 : File Format migration from 5 to 6 | ||
# - 3.0.0 : Add the insertionId column in the ReportsExecution table and fill its value for last nodes run | ||
# - 3.0.0 : Replace varchar by text in the database | ||
# - 3.0.2 : Check for the PostgreSQL version (>= 8.4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i didn't see that one removed below, are you sure you want to remove it ?
2c6b2c1
to
7dbc4e3
Compare
Commit modified |
@ncharles: could you review the modification and merge if OK? |
${PSQL} -d rudder -f ${RUDDER_UPGRADE_TOOLS}/dbMigration-2.11-3.0-set-migration-needed-flag-for-EventLog.sql > /dev/null 2>&1 | ||
fi | ||
|
||
: | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not remove the function?
OK, merging this PR |
https://www.rudder-project.org/redmine/issues/9012