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

NC | Online upgrade improvements #8638

Merged

Conversation

romayalon
Copy link
Contributor

@romayalon romayalon commented Dec 31, 2024

Explain the changes

  1. config_dir_resturcutre upgrade script - Added to the accounts and access keys backup directories a suffix based on the package_from_version. (access keys backup is being removed during the upgrade).
noobaa-core % sudo tree -a /etc/noobaa.conf.d
/etc/noobaa.conf.d
├── .backup_accounts_dir_5.16.5 // <- notice here that there is a 5.16.5 suffix which is the from version
├── .noobaa-config-nsfs
│   └── lost+found
├── access_keys
├── accounts_by_name
├── buckets
├── identities
└── system.json
  1. Removed last_failure from the config_directory upgrade history on a successful upgrade so it'll be less confusing.
  2. Added partial docs.

Issues: Fixed #xxx / Gap #xxx

  1. Missing CLI docs.
  2. Fixed partially NC | Online upgrade improvements #8586
  3. Adding automatic tests - added for bullet 2, and added to the improvements automatic tests for bullet 1.

Testing Instructions:

Manual artificial test -

  1. start noobaa 5.16.z - sudo node noobaa-core/src/cmd/nsfs.js --debug, stop noobaa by CTRL+C
  2. git checkout 5.18.0
  3. start noobaa 5.18.0 - sudo node noobaa-core/src/cmd/nsfs.js --debug, stop noobaa by CTRL+C
  4. add the following faulty script to noobaa-core/src/upgrade/nc_upgrade_scripts/1.0.0/fail.js
/* Copyright (C) 2024 NooBaa */
'use strict';
async function run() {
    console.log('script number 3');
    throw new Error('script number 3 failed')
}
module.exports = {
    run,
    description: 'dummy upgrade script file 1'
};
  1. run config dir upgrade - noobaa-cli upgrade start --expected_version 5.18.0 --expected_hosts hostname1
    and expect a failure
  2. edit noobaa-core/src/upgrade/nc_upgrade_scripts/1.0.0/fail.js and remove the error throw -
/* Copyright (C) 2024 NooBaa */
'use strict';
async function run() {
    console.log('script number 3');
-   throw new Error('script number 3 failed')
}
module.exports = {
    run,
    description: 'dummy upgrade script file 1'
};
  1. run config dir upgrade - noobaa-cli upgrade start --expected_version 5.18.0 --expected_hosts hostname1
    and expect a successful upgrade
  2. check last_failure does not exist anymore in system.json config directory section -
 sudo cat /etc/noobaa.conf.d/system.json | jq .
{
  "hostname1": {
    "current_version": "5.18.0",
    "upgrade_history": {
      "successful_upgrades": [
        {
          "timestamp": 1735662123696,
          "from_version": "5.16.5",
          "to_version": "5.18.0"
        }
      ]
    }
  },
  "config_directory": {
    "phase": "CONFIG_DIR_UNLOCKED",
    "config_dir_version": "1.0.0",
    "upgrade_package_version": "5.18.0",
    "upgrade_history": {
      "successful_upgrades": [
        {
          "timestamp": 1735662139958,
          "completed_scripts": [
            "noobaa-core/src/upgrade/nc_upgrade_scripts/1.0.0/config_dir_restructure.js",
            "noobaa-core/src/upgrade/nc_upgrade_scripts/1.0.0/fail.js"
          ],
          "running_host": "hostname1",
          "config_dir_from_version": "0.0.0",
          "config_dir_to_version": "1.0.0",
          "package_from_version": "5.16.5",
          "package_to_version": "5.18.0"
        }
      ]
    }
  }
}
  1. check that the backup directory has the old version suffix -
sudo tree -a /etc/noobaa.conf.d
/etc/noobaa.conf.d
├── .backup_accounts_dir_5.16.5
├── .noobaa-config-nsfs
│   └── lost+found
├── access_keys
├── accounts_by_name
├── buckets
├── identities
└── system.json
  • Doc added/updated
  • Tests added

@romayalon romayalon mentioned this pull request Dec 31, 2024
12 tasks
@romayalon romayalon requested a review from shirady January 2, 2025 16:20
@romayalon romayalon force-pushed the romy-online-upgrade-improvements branch from 26b75f1 to 9550e20 Compare January 2, 2025 16:22
Copy link
Contributor

@shirady shirady left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/upgrade/nc_upgrade_manager.js Show resolved Hide resolved
src/upgrade/nc_upgrade_manager.js Show resolved Hide resolved
docs/NooBaaNonContainerized/Upgrade.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/Upgrade.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/Upgrade.md Show resolved Hide resolved
docs/NooBaaNonContainerized/Upgrade.md Outdated Show resolved Hide resolved
docs/NooBaaNonContainerized/Upgrade.md Outdated Show resolved Hide resolved
@shirady
Copy link
Contributor

shirady commented Jan 5, 2025

Adding automatic tests - added for bullet 2, and added to the improvements automatic tests for bullet 1.

@romayalon I didn't see the updated tests for the suffix that was added

@romayalon romayalon force-pushed the romy-online-upgrade-improvements branch from 9550e20 to d8a032a Compare January 5, 2025 09:47
@romayalon romayalon merged commit 5c4b49b into noobaa:master Jan 5, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants