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

(t) Samba shares broken - testing upgrade too 5.0.6-0 & 5.0.7-0 #2794 #2796

Conversation

phillxnet
Copy link
Member

@phillxnet phillxnet commented Feb 5, 2024

Introduce 'poetry-plugin-dotenv' to enable Poetry to establish environmental variables from a .env file. Add PASSWORD_STORE_DIR & DJANGO_SETTINGS_MODULE variables to inform OS level 'password-store' app of our configuration, and to centralise DJANGO_SETTINGS_MODULE's definition.

Includes:

  • Additional logging to poetry-install.txt to indicate plugins installed.
  • Modify export statement for PASSWORD_STORE_DIR in build.sh, this is also now set via a recent rockstor-build.service file addition.
  • Include new .env file in project.toml for sdist inclusion.
  • Adopt new poetry run mnt-share share-name, with required cd, in
    new root preexec Samba share config creation.
  • Add preexec migration to initrock.
  • Centralise definition of PASSWORD_STORE_DIR environmental variable to .env file.
  • Move definition of DJANGO_SETTINGS_MODULE environmental variable to .env file.
  • Fix a legacy Poetry removal regression in as yet unpublished (via rpm) testing changes.
  • Remove smb & nmb service restarts on prior preexec migration mechanism.

Note: draft status - in development.

….6-0 & 5.0.7-0 rockstor#2794

Introduce 'poetry-plugin-dotenv' to enable Poetry to establish
environmental variables from a .env file. Add PASSWORD_STORE_DIR
variable to inform OS level 'password-store' app of our configuration.

## Includes:
- Additional logging to poetry-install.txt to indicate plugins installed.
- modify export statement for PASSWORD_STORE_DIR in build.sh, this is also
now set via our recent rockstor-build.service file.
@phillxnet
Copy link
Member Author

Dev notes

The following, with our proposed poetry-plugin-dotenv==0.6.10 in place, appears to work:

root preexec = sh -c 'pushd /opt/rockstor && poetry run mnt-share test_share01 && popd'

via for example a linux client test of

smbclient //192.168.2.105/test_share01 -U radmin --password=pass
Try "help" to get a list of possible commands.
smb: \> ls
  .                                   D        0  Tue Feb  6 14:47:40 2024
  ..                                  D        0  Tue Feb  6 14:47:40 2024

                7864320 blocks of size 1024. 5233152 blocks available
smb: \>

Using @FroggyFlox reproducer as detailed in linked issue.
Where as if we remark our our .env contents we get the expected (at cli client):

tree connect failed: NT_STATUS_ACCESS_DENIED

And restoring the .env contents: restores the above cli smb access.

@phillxnet
Copy link
Member Author

phillxnet commented Feb 6, 2024

The default entry of:

root preexec = "/opt/rockstor/.venv/bin/mnt-share test_share01"

does not work, likely as the pwd is used by the plugin to find it's default .env file.

root preexec = sh -c 'pushd /opt/rockstor && /opt/rockstor/.venv/bin/mnt-share test_share01 && popd'

  • we have to invoke a shell -c here to gain the extra capabilities we need to run multiple commands.

Also does not work. So it seems we have to use poetry run as it is currently suspected that invoking mnt-share via it's script entry does not work for us as hoped with the new Poetry plugin.

So given we are required to set up pwd, and so need multiple command, and thus required sh -c we need not incure the addition complexity of pushd / popd. And so the following looks to be the best/simplest option to date:

root preexec = sh -c 'cd /opt/rockstor && poetry run mnt-share test_share01'

…kstor#2794

- Include new `.env` file in project.toml for sdist inclusion.
- Adopt new `poetry run mnt-share share-name`, with required `cd`, in
new `root preexec` Samba share config creation.
…kstor#2794

- fix incorrect plugin url reference in build.sh
- NO SECRETS indicator in .env file.
…kstor#2794

- Fix legacy Poetry removal regression introduced when build.sh
execution was moved to a systemd service from rpm %posttrans.
- Add python-dotenv to .env file compatibility comments.
@phillxnet
Copy link
Member Author

phillxnet commented Feb 7, 2024

Proposed initrock migration

In initrock we do patch-ups of outdated configs, along with DB migrations etc. it would be nice to ensure folks' prior smb.conf preexec configs are migrated prior to relying-on/starting our entire Django function.

pre preexec format update

[test_share01]
    root preexec = "/opt/rockstor/.venv/bin/mnt-share test_share01"
[07/Feb/2024 13:33:37] INFO [scripts.initrock:476] ### BEGIN Establishing SMB config preexec update...
[07/Feb/2024 13:33:37] INFO [scripts.initrock:486] smb.conf preexec format updated
[07/Feb/2024 13:33:37] INFO [scripts.initrock:489] The nmb service is currently active... restart it
[07/Feb/2024 13:33:37] DEBUG [system.osi:235] Running command: /usr/bin/systemctl restart nmb
[07/Feb/2024 13:33:38] INFO [scripts.initrock:489] The smb service is currently active... restart it
[07/Feb/2024 13:33:38] DEBUG [system.osi:235] Running command: /usr/bin/systemctl restart smb
[07/Feb/2024 13:33:38] INFO [scripts.initrock:496] ### DONE Establishing SMB config preexec update...

With a re-run to ensure idempotency.

[07/Feb/2024 13:45:23] INFO [scripts.initrock:476] ### BEGIN Establishing SMB config preexec update...
[07/Feb/2024 13:45:23] INFO [scripts.initrock:495] smb.conf preexec already updated
[07/Feb/2024 13:45:23] INFO [scripts.initrock:496] ### DONE Establishing SMB config preexec update...

post preexec fromat update

[test_share01]
    root preexec = sh -c "cd /opt/rockstor/ && poetry run mnt-share test_share01"

@phillxnet
Copy link
Member Author

phillxnet commented Feb 7, 2024

A quirk has been observed when restarting all services, where we hang on our smb restart:

[07/Feb/2024 14:18:08] INFO [scripts.initrock:476] ### BEGIN Establishing SMB config preexec update...
[07/Feb/2024 14:18:08] INFO [scripts.initrock:486] smb.conf preexec format updated
[07/Feb/2024 14:18:08] INFO [scripts.initrock:489] The nmb service is currently active... restart it
[07/Feb/2024 14:18:08] INFO [scripts.initrock:489] The smb service is currently active... restart it

Moving to not restarting these services as this is not actually required when changing only the preexec entry.

…kstor#2794

- Add smb.conf preexec migration procedure to initrock.
- Modify samba.py smb.conf machine edits to aid above migration target.
@phillxnet
Copy link
Member Author

phillxnet commented Feb 7, 2024

Revised (no smb/nmb restart) log:

[07/Feb/2024 15:20:36] INFO [scripts.initrock:476] ### BEGIN Establishing SMB config preexec update...
[07/Feb/2024 15:20:36] INFO [scripts.initrock:486] smb.conf preexec format updated
[07/Feb/2024 15:20:36] INFO [scripts.initrock:490] ### DONE Establishing SMB config preexec update...

To assist with our migration the following is now our target preexec format:

[test_share01]
    root preexec = sh -c "cd /opt/rockstor/ && poetry run mnt-share test_share01"

@FroggyFlox
Copy link
Member

Moving to not restarting these services as this is not actually required when changing only the preexec entry.

Very good point... does it mean we can probably improve establish_poetry_path() as well so that it doesn't unnecessarily restart smb/nmb?

@phillxnet
Copy link
Member Author

phillxnet commented Feb 7, 2024

@FroggyFlox
Re:

... does it mean we can probably improve establish_poetry_path() as well so that it doesn't unnecessarily restart smb/nmb

OK, maybe. I'll have a quick look. It does change only the exact same line. I've purposefully done a dedicated additional procedure here as we still need that last migration first to account for pre Poetry installs so 4.1.0-0 stable updating to 4.6.x-0 stable. And although we are meant to jump only one stable each time, best we leave both in place was my thinking. As a catch all.

…kstor#2794

- remove smb nmb restarts on prior preexec migrations.
@phillxnet
Copy link
Member Author

phillxnet commented Feb 7, 2024

Note on a confirmation of our regression fix re legacy Poetry removal:

Feb 07 16:11:56 installer systemd[1]: Starting Build Rockstor...
Feb 07 16:11:56 installer build.sh[16741]: /root/.local/bin/poetry
Feb 07 16:11:57 installer build.sh[16740]: Poetry version 1.1.15 found - UNINSTALLING
Feb 07 16:11:57 installer build.sh[16748]: Removing Poetry (1.1.15)
Feb 07 16:11:57 installer build.sh[16740]: Unset VIRTUAL_ENV

We had (in testing branch) a path modification that worked from %posttrans but not from the new home of rockstor-build.service ! As yet unreleased in any rpm version fixed here as trivial modification to build.sh.

…kstor#2794

- Add DJANGO_SETTINGS_MODULE environment variable to .env file.
- Resource .env file in all relevant rockstor*.service files
via `EnvironmentFile=` directive.
- Normalise on `/usr/local/bin/poetry run` script invocation
in all relevant rockstor*.service files.
- Modify developer instructions (build.sh) to account for new
poetry-plugin-dotenv.
…kstor#2794

- Update build.sh `pipx inject poetry`: poetry-plugin-dotenv==0.6.11
@phillxnet
Copy link
Member Author

phillxnet commented Feb 8, 2024

Test of branch to date

Web-UI admin user radmin: stable updates install 4.6.1-0 (derived from 15.3 installer and updated via howto to 15.4 base OS)

As per reproducer in linked issue:

  • samba config: WORKGROUP
  • share: test_share01 (Access Control - Edit - owner:group) radmin.users
  • Samba export: test_share01 with all defaults
####BEGIN: Rockstor SAMBA CONFIG####
[test_share01]
    root preexec = "/opt/rockstor/.venv/bin/mnt-share test_share01"

Access from 15.5 client OS:

smbclient //192.168.2.105/test_share01 -U radmin --password=pass
Try "help" to get a list of possible commands.
smb: \> mkdir pr2796
smb: \> ls
  .                                   D        0  Thu Feb  8 11:02:28 2024
  ..                                  D        0  Thu Feb  8 10:50:26 2024
  pr2796                              D        0  Thu Feb  8 11:02:28 2024

                7864320 blocks of size 1024. 5233152 blocks available
smb: \> q

Web-UI initiated update to rpm build using this pull requests branch to date: 5.0.7-2796

Rockstor is being updated ... dialog times out

New Web-UI login presented and Web-UI login works as expected with working Web-UI.
During this rpm upgrade we have:

  • updated DB format (rpm %posttrans) and binary (via postgres systemd scripts), from 10 to 13.
  • invoked new lowest level rockstor-build.service (invokes build.sh).
    -- uninstalled legacy Poetry 1.1.15 (build.sh)
    -- installed current Poetry 1.7.1 via OS provided pipx under OS Py3.11 (via build.sh)
    -- injected into Poetry's pipx managed venv poetry-plugin-dotenv (via build.sh)
    -- rebuild our .venv via the new Poetry + plugins - .venv dir is removed on each rpm upgrade. (via build.sh)
    -- re-established jslibs - also wiped on each rpm upgrade (via build.sh)
    -- introduced and pre-configured our new password-store secrets managmenet. (via build.sh)
    -- switched to .env for central management of NON secret environmental variables. (via build.sh)
  • updated Python from 2.7 to 3.11
  • updated Django from 1.11 to 4.2 LTS
  • updated all Python dependencies.
  • maintained existing SMB export functionality via smb.conf migration of preexec entries:
####BEGIN: Rockstor SAMBA CONFIG####
[test_share01]
    root preexec = sh -c "cd /opt/rockstor/ && poetry run mnt-share test_share01"

with exiting write access maintained:

smbclient //192.168.2.105/test_share01 -U radmin --password=pass
Try "help" to get a list of possible commands.
smb: \> mkdir pr2796-test
smb: \> ls
  .                                   D        0  Thu Feb  8 11:35:08 2024
  ..                                  D        0  Thu Feb  8 10:50:26 2024
  pr2796                              D        0  Thu Feb  8 11:02:28 2024
  pr2796-test                         D        0  Thu Feb  8 11:35:08 2024

                7864320 blocks of size 1024. 5233152 blocks available
smb: \> q

A subsequent reboot to ensure SMB (this issue/pr focus) is maintained:

smbclient //192.168.2.105/test_share01 -U radmin --password=pass
Try "help" to get a list of possible commands.
smb: \> mkdir pr2796-test-reboot
smb: \> ls
  .                                   D        0  Thu Feb  8 11:40:30 2024
  ..                                  D        0  Thu Feb  8 10:50:26 2024
  pr2796                              D        0  Thu Feb  8 11:02:28 2024
  pr2796-test                         D        0  Thu Feb  8 11:35:08 2024
  pr2796-test-reboot                  D        0  Thu Feb  8 11:40:30 2024

                7864320 blocks of size 1024. 5233152 blocks available
smb: \> q

@phillxnet
Copy link
Member Author

Given the prior comment's upgrade test, I'll move to squash and re-present as non draft pull request.

@phillxnet phillxnet closed this Feb 8, 2024
@phillxnet phillxnet deleted the 2794-(t)-Samba-shares-not-accessible-after-testing-channel-upgrade-too-5.0.6-0-&-5.0.7-0 branch February 8, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants