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

Increase reliability of host network setup #310

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 20 additions & 14 deletions suse_migration_services/units/setup_host_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ def main():
resolv_conf = os.sep.join(
[root_path, 'etc', 'resolv.conf']
)
if not os.path.exists(resolv_conf):
raise DistMigrationNameResolverException(
'Could not find {0} on migration host'.format(resolv_conf)
)
sysconfig_network_providers = os.sep.join(
[root_path, 'etc', 'sysconfig', 'network', 'providers']
)
Expand All @@ -64,15 +60,16 @@ def main():
)
try:
log.info('Running setup host network service')
Command.run(
[
'mount', '--bind', sysconfig_network_providers,
'/etc/sysconfig/network/providers'
]
)
system_mount.add_entry(
sysconfig_network_providers, '/etc/sysconfig/network/providers'
)
if os.path.exists(sysconfig_network_providers):
Command.run(
[
'mount', '--bind', sysconfig_network_providers,
'/etc/sysconfig/network/providers'
]
)
system_mount.add_entry(
sysconfig_network_providers, '/etc/sysconfig/network/providers'
)
for network_setup in glob.glob(sysconfig_network_setup):
if os.path.isfile(network_setup):
shutil.copy(
Expand All @@ -87,7 +84,14 @@ def main():
resolv_conf, '/etc/resolv.conf'
)
else:
log.info('Empty {0}, bind mounting /etc/resolv.conf to {0}'.format(resolv_conf))
# ensure resolv.conf won't be a symlink on migrated system, create an empty file instead to allow bind mount
if os.path.islink(resolv_conf):
log.info('{0} is a symlink, renaming it to {0}.pre-migration, bind mounting /etc/resolv.conf to {0}'.format(resolv_conf))
os.replace(resolv_conf,resolv_conf+'.pre-migration')
open(resolv_conf,'w').close()
Copy link
Member

Choose a reason for hiding this comment

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

But if resolv.conf is a symlink then the user could have pointed it to any location that has resolution information. If we stick an empty file in it's place there is a good chance that network access will not work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By default on SLES15, /etc/resolv.conf is a symlink to /run/netconfig/resolv.conf (in tmpfs), which is not available on SLES16. This is why we need to move the symlink away (we can't bind mount on top of a symlink).

On SLES16, /etc/resolv.conf is no longer a symlink but a file, which is managed by NetworkManager. If the file is empty, it will replaced by NetworkManager by the information from DHCP (or NM configuration).

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we at least check if the symlink points to the default location? If it doesn't we should probably not write an empty file and instead copy the content from the pointed to file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I'll add some checks to see if the symlink is resolvable (ie not pointing to /run). If it is, it should be left intact, if not, we could do the renaming.

WDYT ?

Copy link
Member

Choose a reason for hiding this comment

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

I think if the symlink points to the default location we can be reasonably certain that the user has not added any custom configuration and that a dhcp based setup will work. If it it points somewhere else we should read the content of the target and dump that in the new file being created.

else:
log.info('Empty {0}, bind mounting /etc/resolv.conf to {0}'.format(resolv_conf))

Command.run(
[
'mount', '--bind', '/etc/resolv.conf',
Expand Down Expand Up @@ -154,6 +158,8 @@ def log_network_details():


def has_host_resolv_setup(resolv_conf_path):
if not os.path.exists(resolv_conf_path):
return False
with open(resolv_conf_path, 'r') as resolv:
for line in resolv:
# check there is useful information in the remaining lines
Expand Down