DLPX-89763 DLPX-86523 delphix-platform changes #477
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
from
ec9dada to
317e58e
Compare
prakashsurya
reviewed
May 8, 2024
...n/var/lib/delphix-platform/ansible/10-delphix-platform/roles/delphix-platform/tasks/main.yml
Show resolved
Hide resolved
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
from
317e58e to
68118d0
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
from
68118d0 to
e56b7de
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
4 times, most recently
from
a4f876c to
d356ef1
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
5 times, most recently
from
38fba97 to
28cab6b
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
2 times, most recently
from
b1d61e7 to
0c718f3
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
2 times, most recently
from
b984a71 to
f389cbe
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
2 times, most recently
from
64091f7 to
5022542
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
from
5022542 to
744dc46
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
3 times, most recently
from
d39c466 to
52eee14
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
3 times, most recently
from
b5743b5 to
3a9629e
Compare
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
5 times, most recently
from
4e5c764 to
d0ba9f9
Compare
…rt/home DLPX-89763 DLPX-86523 delphix-platform changes PR URL: https://www.github.com/delphix/delphix-platform/pull/477
justsanjeev
force-pushed
the
dlpx/pr/justsanjeev/ef30769e-a58a-4475-af32-174db6d07951
branch
from
d0ba9f9 to
43d194d
Compare
sebroy
requested changes
Mar 26, 2025
Comment on lines
+33
to
+38
| if [[ -e $auto_master ]]; then | ||
| if grep -q "\/home\s+auto_home\s+-nobrowse" "$auto_master"; then | ||
| sed -i 's|/home auto_home -nobrowse|#/home auto_home -nobrowse|g' "$auto_master" | ||
| systemctl restart autofs | ||
| fi | ||
| fi |
Comment on lines
+20
to
+24
| # Checking the fstab file if the /export/home entry | ||
| # is present in the /etc/fstab, In case of container | ||
| # upgrade the file is already changed by the | ||
| # container-upgrade script and we dont need to do | ||
| # it again. |
There was a problem hiding this comment.
I'm confused with the wording in this comment. There is also no script called "container-upgrade". If this is referring to the code from the "upgrade-container" script, that script sets up the container used for upgrade verification. I don't think it's relevant here. I might change this to something like:
Home directories were previously mounted under /export/home, and this was changed to /home. This is the upgrade logic that updates the /etc/fstab file to reflect that change. Home directories will be mounted in both /export/home and /home until the system is rebooted to ensure that running processes referencing the old /export/home paths continue to function while also enabling new logins under /home to work.
Comment on lines
+767
to
+771
| - name: Unmount the path if it is mounted | ||
| ansible.builtin.mount: | ||
| path: /export/home | ||
| state: unmounted | ||
| when: mount_status.rc == 0 |
There was a problem hiding this comment.
This is going to fail if there are any running processes with references to any files under /export/home. This feels very fragile. What I would recommend doing instead is:
- Keep /export/home mounted. Since it's been changed in /etc/fstab, it simply won't get mounted at the next reboot.
- At boot, since /export/home won't be mounted at that point, create the symbolic link then. I think this could be as simple as creating the symbolic link (if it doesn't already exist) in the delphix-platform service's start method.
This would remove the need for all of this new ansible logic, and harden the system against random upgrade failures due to unexpected processes holding the mount hostage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
CIS is looking or a single home directory filesystem mounted at the
/homelocation, currently we have the home dataset is mounted on/export/homeDue to that we see the below issues in the CIS Report
Solution
/home./export/hometo/homeso that the exiting tests and dependencies have minimal impact.
**Kindly refer testing section from Review** : https://github.com/delphix/appliance-build/pull/756Testing Done