Skip to content

nexedi / slapos

Go to a project
Merge request !422 opened by  

Integrate monitor and flock in backupserver software release

Please note that this MR needs this slapos.toolbox MR : slapos.toolbox!43 (merged)

I suppose also that we will need to update version of slapos.toolbox used by backupserver but I don't know how to do this (I have a local patch with develop = ..../slapos.toolbox).

As this is my first real MR for buildout, please fire all your comments ! (ping @vpelletier @jerome @Nicolas )

{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Thomas Gambier @tomo

    changed title from Integrate monitor in backupserver software release to Integrate monitor and flock in backupserver software release

    changed title from Integrate monitor in backupserver software release to Integrate monitor and flock in backupserver software release

    Toggle commit list
  • Vincent Pelletier @vpelletier commented
    Master

    I am not familiar with this SR (I think @romain is the only one who is).

    The flock pattern (check lock, actually acquire later) is bothering me a bit, but fixing that would require splitting the script in two I think, making the execution flow harder to read. So I guess it's fine.

    I am not sure what the for status in $STATUS_DIR/*.txt change is about in the flock commit.

    Other than this, the only SR-level remarks I could have are on stuff which is not directly related to your changes and just how this SR is (so likely not part of your task on this):

    • "old"-style (non-jinja2) templating being used in some places (less powerful, and I find it harder to read)
    • no update-hash-style file (could have made your life easier by automating checksum updates)
    • recipes I am not used to being involved (slapos has too many single-use recipes, not sure whether it applies to these)
    • rendering shell scripts, which carries a risk of command injection if not carefuly done

    So... fixing these along the way is welcome (from my point of view, anyway) when you have an opportunity (but no need to go out of your way to create the opportunity).

    Edited by Vincent Pelletier
  • Jérome Perrin @jerome commented
    Owner

    (I think @romain is the only one who is).

    I think @guillaume.hervier also worked on it.

    +1 for " update-hash-style file". This really makes working with slapos buildout profiles more confortable. See 8148384c or 8a8379f0 for example on how to make the change. Modifying a software release to use it is a bit painful manual step, but then it's the last time you have to run md5sum command to update or have to handle conflicts on md5sums.

    After a quick look, I did not see anything wrong with these changes (except that I'm not sure this code has tests)

  • Jérome Perrin
    @jerome started a discussion on the diff
    Resolved by Jérome Perrin
    software/backupserver/template-backup-script.sh.in
    25 24
    26 25 # Inform about backup status
    27 ${coreutils-output:rm} -f $${:status_log}
    28 26 if [ $RESULT -eq 0 ]
    29 27 then
    30 ${coreutils-output:echo} "`${coreutils-output:date} -u`,$${:statistic_log},$${:hostname} backup success" >> $${:status_log}
    31 ${coreutils-output:rm} -f $${:statistic_log}
    32 ${findutils-output:find} rdiff-backup-data/ -maxdepth 1 -name "session_statistic*" | ${coreutils-output:sort} | ${coreutils-output:tail} -n 1 | ${findutils-output:xargs} ${rdiff-backup-output:rdiff-backup} --calculate-average >> $${:statistic_log}
    28 ${coreutils-output:echo} "`${coreutils-output:date} --iso-8601=seconds -u`, $${:statistic_log}, $${:hostname}, backup success" >> $${:status_log}
    29 ${findutils-output:find} rdiff-backup-data/ -maxdepth 1 -name "session_statistic*" | ${coreutils-output:sort} | ${coreutils-output:tail} -n 1 | ${findutils-output:xargs} ${rdiff-backup-output:rdiff-backup} --calculate-average > $${:statistic_log}
    33 30 else
    34 ${coreutils-output:echo} "`${coreutils-output:date} -u`,$${:statistic_log},$${:hostname} backup failed" >> $${:status_log}
    31 ${coreutils-output:echo} "`${coreutils-output:date} --iso-8601=seconds -u`, $${:statistic_log}, $${:hostname}, backup failed" >> $${:status_log}
    35 32 fi
    36
    37 # python scripts/verify_with_sudo.py ./ $${:connection}:/
    Please register or sign in to reply
  • Jérome Perrin @jerome

    resolved all discussions

    resolved all discussions

    Toggle commit list
  • Thomas Gambier @tomo commented
    Master

    I am not sure what the for status in $STATUS_DIR/*.txt change is about in the flock commit.

    It's because the lock file I'm using is in the $STATUS_DIR and it ends with .lock extension. I didn't want the RSS generator to parse this file.

  • Kazuhiko Shiozaki
    @kazuhiko started a discussion on an old version of the diff
    Resolved by Thomas Gambier
    stack/slapos.cfg
    116 116 cliff = 2.4.0
    117 117 cmd2 = 0.7.0
    118 118 collective.recipe.template = 2.0
    119 croniter = 0.3.25
    • Kazuhiko Shiozaki @kazuhiko commented
      Owner

      which egg requires this egg ? if it is used in only certain SRs, better to put there. If it is widely used in many SRs, you can put here, but still I prefer having 'Required by:' comment.

    • Thomas Gambier @tomo commented
      Master

      It is used by the whole slapos.toolbox, all SR that will include slapos.toolbox will need this. See slapos.toolbox@b7e8eb1c

      That's why I thought that it's best to put it here.

    • Kazuhiko Shiozaki @kazuhiko commented
      Owner

      Hmm... For now, we don't put slapos.toolbox and its dependencies in stack/slapos.cfg but in manu (not all) SR configurations. It is indeed huge duplicate. So if you put something into stack/slapos.cfg, that can be a good idea, please put slapos.toolbox itself and all its dependencies there.

    • Thomas Gambier @tomo commented
      Master

      thanks to !427 (merged) I now have to change only stack/slapos.cfg

    Please register or sign in to reply
  • Vincent Pelletier @vpelletier commented
    Master

    It's because the lock file I'm using is in the $STATUS_DIR and it ends with .lock extension. I didn't want the RSS generator to parse this file.

    Makes sense indeed.

  • Thomas Gambier @tomo

    added 21 commits

    • d5af9cd4...29b2d7cc - 18 commits from branch nexedi:master
    • 15e1633a - cleanup the backupserver status file
    • 32b3b257 - Add monitor and promise to backupserver software release
    • 5a92da34 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 21 commits

    • d5af9cd4...29b2d7cc - 18 commits from branch nexedi:master
    • 15e1633a - cleanup the backupserver status file
    • 32b3b257 - Add monitor and promise to backupserver software release
    • 5a92da34 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 9 commits

    • 5a92da34...f427842b - 5 commits from branch nexedi:master
    • 9ab943d4 - cleanup the backupserver status file
    • 8f661b99 - version up: slapos.toolbox: 0.82
    • 44a8ff09 - Add monitor and promise to backupserver software release
    • 66ed017d - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 9 commits

    • 5a92da34...f427842b - 5 commits from branch nexedi:master
    • 9ab943d4 - cleanup the backupserver status file
    • 8f661b99 - version up: slapos.toolbox: 0.82
    • 44a8ff09 - Add monitor and promise to backupserver software release
    • 66ed017d - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    resolved all discussions

    resolved all discussions

    Toggle commit list
  • Thomas Gambier @tomo

    added 7 commits

    • 66ed017d...754acb1b - 2 commits from branch nexedi:master
    • ee5a0834 - slapos.toolbox version is now in stack/slapos.cfg
    • c8d24f67 - cleanup the backupserver status file
    • 05a7db90 - version up: slapos.toolbox: 0.82
    • c3a18d59 - Add monitor and promise to backupserver software release
    • 575f3a8a - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 7 commits

    • 66ed017d...754acb1b - 2 commits from branch nexedi:master
    • ee5a0834 - slapos.toolbox version is now in stack/slapos.cfg
    • c8d24f67 - cleanup the backupserver status file
    • 05a7db90 - version up: slapos.toolbox: 0.82
    • c3a18d59 - Add monitor and promise to backupserver software release
    • 575f3a8a - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Rafael Monnerat @rafael commented
    Owner

    If possible merge this, because all resilient tests are failing because they require this commit: 05a7db90

    (pin the eggs are missing... see: https://nexedijs.erp5.net/#/test_result_module/20181029-689A6111 )

    Edited by Rafael Monnerat
  • Thomas Gambier @tomo commented
    Master

    @rafael I don't understand why it would fail because without this commit, everything is still pointing to slapos.toolbox version 0.81.

    Are you sure, you are not talking about this commit : !432 (merged) ?

    Plus, it seems we can't see the log of failing tests with the testnode down (all testnodes in online are now poweredoff).

    Edited by Thomas Gambier
  • Jérome Perrin @jerome commented
    Owner

    error is:

    Installing monitor-eggs.
While:
  Installing monitor-eggs.
  Getting distribution for 'croniter'.
Error: Picked: croniter = 0.3.25

    This is because https://nexedi.erp5.net/test_suite_module/397 uses a git checkout of slapos.toolbox (I'm not sure why we use a git checkout here)

  • Thomas Gambier @tomo commented
    Master

    I'm confortable with merging this. @romain ?

  • Thomas Gambier @tomo

    added 79 commits

    • 575f3a8a...cfa7c7e0 - 76 commits from branch nexedi:master
    • eb278d9e - cleanup the backupserver status file
    • f7fcb28c - Add monitor and promise to backupserver software release
    • 9e57eb48 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 79 commits

    • 575f3a8a...cfa7c7e0 - 76 commits from branch nexedi:master
    • eb278d9e - cleanup the backupserver status file
    • f7fcb28c - Add monitor and promise to backupserver software release
    • 9e57eb48 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo commented
    Master

    OK, since @romain wasn't readyto merge this soon, I pushed 05a7db90 to master directly which is now cfa7c7e0.

  • Thomas Gambier @tomo

    added 4 commits

    • c05514c5 - 1 commit from branch nexedi:master
    • 6fbcf042 - cleanup the backupserver status file
    • 27d22ffe - Add monitor and promise to backupserver software release
    • 2fc0cff9 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 4 commits

    • c05514c5 - 1 commit from branch nexedi:master
    • 6fbcf042 - cleanup the backupserver status file
    • 27d22ffe - Add monitor and promise to backupserver software release
    • 2fc0cff9 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 56 commits

    • 2fc0cff9...8e03de68 - 53 commits from branch nexedi:master
    • 39040e99 - cleanup the backupserver status file
    • f5281a3c - Add monitor and promise to backupserver software release
    • 23c53d09 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 56 commits

    • 2fc0cff9...8e03de68 - 53 commits from branch nexedi:master
    • 39040e99 - cleanup the backupserver status file
    • f5281a3c - Add monitor and promise to backupserver software release
    • 23c53d09 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 3 commits

    • 7bf95f61 - cleanup the backupserver status file
    • dec1eb15 - Add monitor and promise to backupserver software release
    • 8b05f8a4 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 3 commits

    • 7bf95f61 - cleanup the backupserver status file
    • dec1eb15 - Add monitor and promise to backupserver software release
    • 8b05f8a4 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 3 commits

    • 2cae3004 - cleanup the backupserver status file
    • 2750719f - Add monitor and promise to backupserver software release
    • e67089be - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 3 commits

    • 2cae3004 - cleanup the backupserver status file
    • 2750719f - Add monitor and promise to backupserver software release
    • e67089be - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 30 commits

    • e67089be...c0c9bbe9 - 27 commits from branch nexedi:master
    • 0e116576 - cleanup the backupserver status file
    • b8eb2572 - Add monitor and promise to backupserver software release
    • 22a12698 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    added 30 commits

    • e67089be...c0c9bbe9 - 27 commits from branch nexedi:master
    • 0e116576 - cleanup the backupserver status file
    • b8eb2572 - Add monitor and promise to backupserver software release
    • 22a12698 - Prevent backup from running if previous backup didn't finish

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 3 commits

    • b05948b3 - Add monitor and promise to backupserver software release
    • d54a129a - Prevent backup from running if previous backup didn't finish
    • d6129afd - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 3 commits

    • b05948b3 - Add monitor and promise to backupserver software release
    • d54a129a - Prevent backup from running if previous backup didn't finish
    • d6129afd - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    marked as a Work In Progress from tomo/slapos@d6129afd

    marked as a Work In Progress from tomo/slapos@d6129afd

    Toggle commit list
  • Thomas Gambier @tomo

    added 3 commits

    • 79426a42 - Add monitor and promise to backupserver software release
    • 23fafd6f - Prevent backup from running if previous backup didn't finish
    • f94ae9db - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 3 commits

    • 79426a42 - Add monitor and promise to backupserver software release
    • 23fafd6f - Prevent backup from running if previous backup didn't finish
    • f94ae9db - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    • af64a657 - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 1 commit

    • af64a657 - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    • 433f1cad - WIP do not commit: force to use rdiff-backup version 1.3.4nxd2-SlapOSPatched001 …

    Compare with previous version

    added 1 commit

    • 433f1cad - WIP do not commit: force to use rdiff-backup version 1.3.4nxd2-SlapOSPatched001 …

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    • e571e45d - WIP do not commit: force to use rdiff-backup version 1.3.4nxd2-SlapOSPatched001 …

    Compare with previous version

    added 1 commit

    • e571e45d - WIP do not commit: force to use rdiff-backup version 1.3.4nxd2-SlapOSPatched001 …

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 1 commit

    • fb998b6c - Add needed egg for slapos.toolbox 0.84

    Compare with previous version

    added 1 commit

    • fb998b6c - Add needed egg for slapos.toolbox 0.84

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 6 commits

    • 167b4a13 - Add needed egg for slapos.toolbox 0.84
    • d610749f - Force to use rdiff-backup version 1.3.4nxd2-SlapOSPatched001 in backupserver SR
    • 4091bdb5 - cleanup the backupserver status file
    • 017372aa - Add monitor and promise to backupserver software release
    • 9880d282 - Prevent backup from running if previous backup didn't finish
    • 6040427b - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 6 commits

    • 167b4a13 - Add needed egg for slapos.toolbox 0.84
    • d610749f - Force to use rdiff-backup version 1.3.4nxd2-SlapOSPatched001 in backupserver SR
    • 4091bdb5 - cleanup the backupserver status file
    • 017372aa - Add monitor and promise to backupserver software release
    • 9880d282 - Prevent backup from running if previous backup didn't finish
    • 6040427b - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 5 commits

    • b7555f62 - Force to use rdiff-backup version 1.0.5+SlapOSPatched001 in backupserver SR
    • d96cf86b - cleanup the backupserver status file
    • 64600f08 - Add monitor and promise to backupserver software release
    • 27e317c1 - Prevent backup from running if previous backup didn't finish
    • cc7aff56 - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 5 commits

    • b7555f62 - Force to use rdiff-backup version 1.0.5+SlapOSPatched001 in backupserver SR
    • d96cf86b - cleanup the backupserver status file
    • 64600f08 - Add monitor and promise to backupserver software release
    • 27e317c1 - Prevent backup from running if previous backup didn't finish
    • cc7aff56 - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 2 commits

    • 74eadec5 - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses
    • 2faec2c3 - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 2 commits

    • 74eadec5 - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses
    • 2faec2c3 - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 91 commits

    • 2faec2c3...1c56e822 - 84 commits from branch nexedi:master
    • 7434e214 - Add needed egg for slapos.toolbox 0.84
    • f9f9b47c - [backupserver] Force to use rdiff-backup version 1.0.5+SlapOSPatched001
    • 63ff94b6 - [backupserver] cleanup the status file
    • c6e8df45 - [backupserver] add monitor and promise
    • 87816623 - [backupserver] prevent backup from running if previous backup didn't finish
    • a7cb9c38 - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses
    • c8ab8eaf - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    added 91 commits

    • 2faec2c3...1c56e822 - 84 commits from branch nexedi:master
    • 7434e214 - Add needed egg for slapos.toolbox 0.84
    • f9f9b47c - [backupserver] Force to use rdiff-backup version 1.0.5+SlapOSPatched001
    • 63ff94b6 - [backupserver] cleanup the status file
    • c6e8df45 - [backupserver] add monitor and promise
    • 87816623 - [backupserver] prevent backup from running if previous backup didn't finish
    • a7cb9c38 - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses
    • c8ab8eaf - WIP do not commit: use develop version of slapos.toolbox

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    added 6 commits

    • f8959cc1 - Increase slapos.toolbox version to 0.84
    • e4a3bec1 - [backupserver] Force to use rdiff-backup version 1.0.5+SlapOSPatched001
    • e77adabc - [backupserver] cleanup the status file
    • 482bcaa2 - [backupserver] add monitor and promise
    • 5c389b81 - [backupserver] prevent backup from running if previous backup didn't finish
    • 8d437fa7 - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses

    Compare with previous version

    added 6 commits

    • f8959cc1 - Increase slapos.toolbox version to 0.84
    • e4a3bec1 - [backupserver] Force to use rdiff-backup version 1.0.5+SlapOSPatched001
    • e77adabc - [backupserver] cleanup the status file
    • 482bcaa2 - [backupserver] add monitor and promise
    • 5c389b81 - [backupserver] prevent backup from running if previous backup didn't finish
    • 8d437fa7 - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses

    Compare with previous version

    Toggle commit list
  • Thomas Gambier @tomo

    unmarked as a Work In Progress

    unmarked as a Work In Progress

    Toggle commit list
  • Thomas Gambier @tomo commented
    Master

    finally ready to merge :) @romain

  • Thomas Gambier @tomo

    added 6 commits

    • f0a3d912 - 1 commit from branch nexedi:master
    • 2e5fe176 - [backupserver] Force to use rdiff-backup version 1.0.5+SlapOSPatched001
    • cd77528d - [backupserver] cleanup the status file
    • 92fc8911 - [backupserver] add monitor and promise
    • 92e90202 - [backupserver] prevent backup from running if previous backup didn't finish
    • 82304c1d - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses

    Compare with previous version

    added 6 commits

    • f0a3d912 - 1 commit from branch nexedi:master
    • 2e5fe176 - [backupserver] Force to use rdiff-backup version 1.0.5+SlapOSPatched001
    • cd77528d - [backupserver] cleanup the status file
    • 92fc8911 - [backupserver] add monitor and promise
    • 92e90202 - [backupserver] prevent backup from running if previous backup didn't finish
    • 82304c1d - [backupserver] don't force ssh -6 as ssh can distinguish between IPv6 and IPv4 addresses

    Compare with previous version

    Toggle commit list
  • Romain Courteaud @romain commented
    Owner

    This was manually merged

  • Romain Courteaud @romain

    closed

    closed

    Toggle commit list
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备14008524号