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

Rockons: Invalid environment variable (...) #1588 #2887

Conversation

phillxnet
Copy link
Member

Rock-on front-end install wizard does not collate container info regarding Rock-on defined environment elements. Affects multi-container Rock-ons only. Results in back-end failure to successfully assign user-input environmental variables. Resolved by adding a container id dimension to the environmental matrix created by the install wizard, and updating the back-end Rock-on instantiator to take advantage of this info. Previously a reverse engineering approach was taken: which cannot work for example, where two containers, within the same Rock-on definition, use the same environmental variable name.

Thanks to GitHub user @anatox - the majority of this PR is based on their prior submission to the project - alas un-merged at the time.

Also adds

  • Rock-on install debug log re front-end info received.
  • Fixes, partly, resulting installer wizard summary feedback bug.

Supersedes PR #1688
Fixes #1588


@anatox My apologies for failing to maintain git attribution here: I did cherry pick from your original proposal (older master branch), but in sub-commit changes and this failed to maintain git attribution.

Rock-on front-end install wizard does not collate container info
regarding Rock-on defined environment elements. Affects multi-container
Rock-ons only. Results in back-end failure to successfully assign
user-input environmental variables. Resolved by adding a container id
dimension to the environmental matrix created by the install wizard,
and updating the back-end Rock-on instantiator to take advantage
of this info. Previously a reverse engineering approach was taken:
which cannot work for example, where two containers, within the same
Rock-on definition, use the same environmental variable name.

Thanks to GitHub user @anatox - the majority of this PR is based on
their prior submission to the project - alas unmerged at the time.

Also adds
- Rock-on install debug log re front-end info received.
- Fixes, partly, resulting installer wizard summary feedback bug.
@phillxnet
Copy link
Member Author

Testing:

A reproducer for the linked issue #1688 (reported by @daniel-illi) was the proposed Rock-on presented in draft PR:
"bare os server set ...": rockstor/rockon-registry#379

Prior to the proposed changed we have the issue detailed failure in current testing branch of

Invalid environment variable (DB_ADMIN_PASSWORD).
NoneType: None

see #1588 (comment)

Post proposal the reproducer Rock-on definition successfully installs: all-bit-it with remaining issues to be addressed in dedicated additional issues/PRs. The following is the resulting Web-UI pertaining to the multi-container multi-environment definition install:

Installed-Rock-ons-env-summary-ok

as a result of the following Rock-on install wizard summary:

Rock-ons-install-wizard-env-summary-proposal

with the Rock-on install wizard now passing the akin of:

env_map={'85': {'POSTGRES_PASSWORD': 'ptt2'}, '86': {'DB_ADMIN_PASSWORD': 'ptt2', 'DB_PASSWORD': 'ctt2', 'BAREOS_WEBUI_PASSWORD': 'wtt2'}}

@phillxnet
Copy link
Member Author

phillxnet commented Aug 14, 2024

@FroggyFlox & @Hooverdan96 given the prior delay of 7 years! I'd like to get this merged essentially directly. Assuming there are no glaring issues. This is the first part of a drive to debug our Rock-on installer sufficiently to accommodate the like of the indicated reproducer Rock-on. We also have @Hooverdan96 following #1688 (comment) copied for convenience:

In light of comments/interest on the forum about new Rockons that require multi-container setups, this might become a bit more important for installations like immich.app or photoprism or even ente for photo management as they have multi-container requirements.

from the prior proposed partial fix: upon which this is heavily based.

@FroggyFlox
Copy link
Member

@phillxnet , go for it.

@phillxnet
Copy link
Member Author

An rpm was build from this PR branch and configured to use a locally hosted Rock-on repo with the reproducer Rock-on (BareOS-server-set) draft PR (rockstor/rockon-registry#379) pre-merged.

An install of this Rock-on was then enacted:

Pre-install summary

BareOS-server-set-pre-install-summary

Installed summary

BareOS-server-set-installed-config

The docker container specific environment (user entered), and opt -e (hard-wired env vars) Rock-on configuration elements were confirmed for example via docker inspect bareos-director

e.g.:

docker inspect bareos-director
...
            "Env": [
                "DB_NAME=bareos",
                "DB_HOST=bareos-db",
                "DB_PASSWORD=ctt2",
                "BAREOS_WEBUI_PASSWORD=wtt2",
                "DB_INIT=true",
                "DB_ADMIN_USER=postgres",
                "DB_PORT=5432",
                "DB_ADMIN_PASSWORD=ptt2",
                "DB_USER=bareos",
                "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
            ],

Other issues remain with the reproducer Rock-on, but the install blocker issue of:

Invalid environment variable (DB_ADMIN_PASSWORD).
NoneType: None

is resolved by these changes: moving to merge as a result.

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