Skip to content

Conversation

@Phil-Friderici
Copy link
Contributor

No description provided.

@Phil-Friderici
Copy link
Contributor Author

If the file 10-unsupported-modules.conf doesn't exists on SLES 15.4 it can be created. I just need to know which mode and owner it should have.

Copy link
Contributor

@anders-larsson anders-larsson left a comment

Choose a reason for hiding this comment

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

I think it would be better to create a file in /etc/modprobe.d since it would never be replaced by OS updates. It seems like it should be root:root and mode 0644.

Do you think we should apply this to all SLES11.4 and all newer releases or should we match this specific release?

@Phil-Friderici
Copy link
Contributor Author

I think it would be better to create a file in /etc/modprobe.d since it would never be replaced by OS updates. It seems like it should be root:root and mode 0644.

Should we continue to use the old path (/etc/modprobe.d/10-unsupported-modules.conf)? I guess than we have to modify modprobes configuration too. Or do you meant to just create /lib/modprobe.d/10-unsupported-modules.conf as new file and then use file_line() to add what's needed ?

Do you think we should apply this to all SLES11.4 and all newer releases or should we match this specific release?

Yes, guess it will become the new default for some more versions/minors.

@anders-larsson
Copy link
Contributor

Should we continue to use the old path (/etc/modprobe.d/10-unsupported-modules.conf)? I guess than we have to modify modprobes configuration too.

In SLES15.4 they moved all default files from /etc/modprobe.d to /lib/modprobe.d. However /etc/modprobe.d/ exists and system specific configuration should be put in this directory instead of /lib/modprobe.d which is directly managed by the package manager (files might be replaced when packages are updated).

This is similar to how it works with systemd where system units managed by packages are put in {/usr,}/lib/systemd/system while units not managed by a package should be put in /etc/systemd/system.

Or do you meant to just create /lib/modprobe.d/10-unsupported-modules.conf as new file and then use file_line() to add what's needed ?

Maybe we should just skip file_line altogether and manage the file using file {}. The file contains a lot of commented out lines, only thing actually configured is allow_unsupported_modules 1.

@Phil-Friderici
Copy link
Contributor Author

Phil-Friderici commented Oct 17, 2022

Ok, that makes sense to keep using /etc/modprobe.d/ on Susies. If we manage a afs-module specific configuration file, shouldn't we give it a better name than /etc/modprobe.d/10-unsupported-modules.conf ?
What about /etc/modprobe.d/99-afs-modules.conf with an included header that points the interessted reader to Puppet.
The only drawback would be that we should remove the old afs specific lines from /etc/modprobe.d/10-unsupported-modules.conf.
Maybe we better stick to the old name instead ;)

@Phil-Friderici
Copy link
Contributor Author

I think this should do the trick. Includes unit tests to show the wanted behaviour.

@anders-larsson
Copy link
Contributor

Yeah. It's definitely a bit bad for AFS to take exclusive control. Best case would be to have some other module handle this. That's why I mentioned using puppet-kmod in the issue (different system). Let's have that as a long-term improvement though instead of messing around with it right now.

Looks OK. I don't think we should be required to test this change manually. Spec tests passing should be enough?

@Phil-Friderici
Copy link
Contributor Author

I had added a comment about kmod so we get inspired everytime we read the code ;)

@Phil-Friderici Phil-Friderici merged commit ea46fd9 into Ericsson:master Oct 18, 2022
@Phil-Friderici Phil-Friderici deleted the sles15.4 branch October 18, 2022 08:31
@Phil-Friderici
Copy link
Contributor Author

Released as v2.3.3

@anders-larsson
Copy link
Contributor

anders-larsson commented Oct 18, 2022

Thanks!

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.

2 participants