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

Closed

Conversation

fcrozat
Copy link
Contributor

@fcrozat fcrozat commented Jan 23, 2025

Handle if /etc/resolv.conf doesn't exist on system to migrate or if it is a symlink

This is required to migrate SLE 15 system and should be safe on SLE12 ones (untested)

Handle if /etc/resolv.conf doesn't exist on system to migrate or if it
is a symlink
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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

why should bind mounting a symlink be a problem ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the various issues the patch is trying to resolve are due to /etc/resolv.conf symlink to /run/netconfig/resolv.conf being broken on a SLES 16 system (ie the migration live image), since netconfig is not present anymore, nor wicked.

  • test on /etc/resolv.conf existence fails (os.path.exists will follow the link)
  • bind mount on /etc/resolv.conf symlink will fail too (symlink target doesn't exist)

I'm not super happy with touching the migration system either, before we start the migration process.

I'll test another approach, by creating a symlink /run/netconfig/resolv.conf in the sle16 migration image and reports if it works.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@fcrozat as we control the migration environment I would also be in favor to "fix" the migration live image and the container to provide a proper environment rather than adding code to do this.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@fcrozat something different. I see you work from forks and for whatever reason I cannot make the github actions to run from your contribution. Normally I have a button to "approve and run" the actions but I believe it's because the SUSE org is a private one this doesn't work. It would be better to add you to the project and you can work from branches which will then also run the actions and tests. I believe I don't have permissions to add members but maybe @rjschwei or @aosthof can do this ? Thanks much

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to bother you with my initial tests but I'm fine following your regular workflow.

@fcrozat
Copy link
Contributor Author

fcrozat commented Feb 4, 2025

Replaced by #311 (and another upcoming PR)

@fcrozat fcrozat closed this Feb 4, 2025
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.

3 participants