-
Notifications
You must be signed in to change notification settings - Fork 44
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
DLPX-89763 DLPX-86523 delphix-platform changes #477
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
#!/bin/bash -eux | ||
# | ||
# Copyright 2024 Delphix | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
# | ||
|
||
case $1 in | ||
upgrade) | ||
# 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. | ||
fs_tab=/etc/fstab | ||
auto_master=/etc/auto.master | ||
|
||
if grep -q "\/export\/home" "$fs_tab"; then | ||
sed -i 's|/export/home|/home|g' "$fs_tab" | ||
mount /home | ||
fi | ||
|
||
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
+33
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this here? |
||
|
||
passwd_file=/etc/passwd | ||
if grep -q "\/export\/home" "$passwd_file"; then | ||
sed -i 's/\/export\/home/\/home/g' /etc/passwd | ||
fi | ||
|
||
;; | ||
esac | ||
|
||
exit 0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
# it below; otherwise that task will fail. | ||
# | ||
- file: | ||
path: /export/home | ||
path: /home | ||
justsanjeev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
state: directory | ||
mode: 0755 | ||
|
||
|
@@ -35,7 +35,7 @@ | |
shell: /bin/bash | ||
create_home: yes | ||
comment: Delphix User | ||
home: /export/home/delphix | ||
home: /home/delphix | ||
|
||
# | ||
# In order for this locale to be used (e.g. by virtualization) we need | ||
|
@@ -689,7 +689,7 @@ | |
|
||
- name: Source bash completion | ||
blockinfile: | ||
dest: "/export/home/delphix/.bashrc" | ||
dest: "/home/delphix/.bashrc" | ||
block: | | ||
. /etc/bash_completion.d/systemctl | ||
. /etc/bash_completion.d/zfs | ||
|
@@ -738,3 +738,53 @@ | |
path: /etc/environment | ||
state: absent | ||
regexp: '^\s*PATH\s*=' | ||
|
||
# | ||
# Soft link creation in case it doesn't exist | ||
# | ||
- name: Check export | ||
ansible.builtin.stat: | ||
path: /export | ||
register: export_status | ||
|
||
- name: Check export home | ||
ansible.builtin.stat: | ||
path: /export/home | ||
when: export_status.stat.exists and export_status.stat.isdir | ||
register: export_home_status | ||
|
||
# | ||
# Before deleting the /export/home directory if the | ||
# home data set is mounted on /export/home if its | ||
# mounted remove if first and then go ahead. | ||
# | ||
- name: Check if the path is mounted | ||
ansible.builtin.shell: | | ||
mount | grep /export/home | ||
register: mount_status | ||
ignore_errors: yes | ||
|
||
- name: Unmount the path if it is mounted | ||
ansible.builtin.mount: | ||
path: /export/home | ||
state: unmounted | ||
when: mount_status.rc == 0 | ||
Comment on lines
+767
to
+771
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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:
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. |
||
|
||
- name: Delete home directory | ||
ansible.builtin.file: | ||
path: /export/home | ||
state: absent | ||
when: not export_status.stat.exists or export_home_status.stat.exists and export_home_status.stat.isdir | ||
|
||
- name: Create export directory | ||
ansible.builtin.file: | ||
path: /export | ||
state: directory | ||
mode: 0755 | ||
when: not export_status.stat.exists | ||
|
||
- name: Create the soft link | ||
ansible.builtin.file: | ||
src: /home | ||
dest: /export/home | ||
state: link |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.