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

AutoYaST: Support to configure the network during the first stage by default #1059

Merged
merged 16 commits into from
May 22, 2020

Conversation

teclator
Copy link
Contributor

@teclator teclator commented May 18, 2020

This PR is related to yast/yast-autoinstallation#606 where the problem and solutions are already described, so, use it as a base context.

Problem (specific to this module)

There are some issues we should fix in order to write the configuration during the first stage.

1. Udev rules are not refreshed after writting

The InterfacesWriter did not refreshed the written udev rules in case of an autoinstallation as we can see here:

https://github.com/yast/yast-network/pull/1059/files#diff-48ec4731f89821c19dc203da64dc00a5L108

But during the first stage, the imported network configuration can be applied before the proposal, and, at that point, a rename of the interfaces is needed. Thus, we should distinguish between being at the end of the installation (already chrooted) or not.

By now, we have added a parameter which determines whether the writer should also reload the configuration or not.

2. Udev rules are written to the inst-sys and need to be copied to the target system

The udev_persistent agent or in general non y2 agents do not permit to change the agent root location

https://yastgithubio.readthedocs.io/en/latest/development-tips/#changing-the-root-location-for-agents

in case that we are at the end of the instalation, the files inst-sys files are copied before written. If the config is written later as it is, at least the udev rules should be copied.

3. Linuxrc and imported configuration is somehow merge even when keep_install_network is failse

The autoinst reader uses the current sysconfig as the base one

f4abc6a#diff-6dc211a0076a98b7941e86c01765b477L51

Applying the changes imported over it. But only interfaces are needed in order to update properly the associated udev rules, and also for removing the ones not imported, specially if it is selected to not keep the linuxrc configured network.

4. The NetworkAutoyast checks the profile directly and writes the config based on the presence of the second_stage

If the idea is to remove the networking section during autosetup, the networking section will not be present when calling it at the end of the installation. Thus, the module need to be updated. But also, it checks whether the second stage is present or not in order to write the config.

The config should not be written again in case it was already configured before the proposal and the check of the second stage does not make sense anymore
/761872905d0b162fc5f46329398763f1116d69d8#diff-24ba5c89e21a968be8706d02f59adb43L314

Solution

  • The refresh of of the udev rules respect now the Lan.write_only variable, thus, during the setup_before_proposal, them should be refreshed, but in case of the end of the installation them should not
  • If the imported network configuration is written at the end of the installation, the udev rules are copied to the target system after being modified.
  • The autoinst reader will only use the current system interfaces apart of the imported configuration as the internal yast_config. In case of merged, it will be already defined by the merged profile structure.
  • Some logic have been removed from NetworkAutoyast, and now will mainly write the imported config.
  • Moved the auto client to its own class under y2network namespace improving the unit test coverage.

Tests

  • Added some uni tests and do some tests manually, in this area, and specially with udev rules handling, integration tests are very important.

@teclator teclator force-pushed the autoyast_poc branch 2 times, most recently from 86b4f21 to b3efed2 Compare May 19, 2020 15:42
@teclator teclator marked this pull request as ready for review May 20, 2020 12:23
@coveralls
Copy link

coveralls commented May 20, 2020

Coverage Status

Coverage increased (+0.1%) to 71.148% when pulling 7e87cf9 on autoyast_poc into b5e46d9 on master.

@teclator teclator changed the title Autoyast poc AutoYaST: Support to configure the network during the first stage by default May 20, 2020
Copy link
Contributor

@imobachgs imobachgs left a comment

Choose a reason for hiding this comment

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

Just a few comments (especially for the future). Thanks a lot for taking care of it. I know it was a lot of work and testing. 😉.

src/lib/network/network_autoyast.rb Outdated Show resolved Hide resolved
src/lib/y2network/clients/auto.rb Show resolved Hide resolved
src/lib/y2network/sysconfig/interfaces_writer.rb Outdated Show resolved Hide resolved
src/modules/Lan.rb Show resolved Hide resolved
@teclator teclator merged commit bd2759d into master May 22, 2020
@teclator teclator deleted the autoyast_poc branch May 22, 2020 08:38
@yast-bot
Copy link
Contributor

✔️ Public Jenkins job #150 successfully finished
✔️ Created OBS submit request #808129

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.

None yet

4 participants