Skip to content

Conversation

jeffmahoney
Copy link
Contributor

On some newer OS releases, standard config files are installed in /usr/etc with site changes to copies in /etc.

That results in the following startup failure:
Traceback (most recent call last):
File "/usr/bin/samba-container", line 8, in
sys.exit(main())
^^^^^^
File "sambacc/commands/main.py", line 231, in main
cfunc(CommandContext(cli))
File "sambacc/commands/run.py", line 75, in run_container
init_container(ctx)
File "sambacc/commands/initialize.py", line 116, in init_container
cmds[step_name].cmd_func(ctx)
File "sambacc/commands/initialize.py", line 38, in _import_nsswitch
nss.read()
File "sambacc/textfile.py", line 28, in read
with open(self.path) as f:
^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/etc/nsswitch.conf'

This changes the initialization so that we fall back to reading from /usr/etc/nsswitch.conf if reading from /etc/nsswitch.conf fails but still write out any changes to /etc/nsswitch.conf.


def write(self) -> None:
tpath = self._tmp_path(self.path)
def write(self, path: typing.Optional[str] = None) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Most of this patch looks OK to me, but this part I find a bit confusing. The path is provided to the object at initialization time, and then again (optionally) here - that seems redundant to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On systems where this matters, /usr is typically read-only. Any changes to the config files in /usr/etc are written to /etc and prioritized over the ones in /usr/etc. This change allows us to write the contents of the read-in (and modified) file back out to a different location. When the path is unspecified, the existing behavior is preserved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, I see that. I am not thrilled by that aspect of the api though. However, I can see how other changes, like copying the content from one NameServiceSwitchLoader to another is probably more of a hassle. Maybe change the name of the new argument to alternate_path will be a little more self-documenting and ease my concerns.

There's also a formatting failure in the test suite. You should be able to simply run tox exec -e formatting -- black . to have the pinned version of black fix it for you.

@phlogistonjohn
Copy link
Collaborator

I'd like to see this move forward. Do you think you'll have time soon to fix the tests? (and maybe rename that argument :-) ) Thanks!

@phlogistonjohn
Copy link
Collaborator

@jeffmahoney ping ^

On some newer OS releases, standard config files are installed in
/usr/etc with site changes to copies in /etc.

That results in the following startup failure:
Traceback (most recent call last):
  File "/usr/bin/samba-container", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "sambacc/commands/main.py", line 231, in main
    cfunc(CommandContext(cli))
  File "sambacc/commands/run.py", line 75, in run_container
    init_container(ctx)
  File "sambacc/commands/initialize.py", line 116, in init_container
    cmds[step_name].cmd_func(ctx)
  File "sambacc/commands/initialize.py", line 38, in _import_nsswitch
    nss.read()
  File "sambacc/textfile.py", line 28, in read
    with open(self.path) as f:
         ^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/etc/nsswitch.conf'

This changes the initialization so that we fall back to reading from
/usr/etc/nsswitch.conf if reading from /etc/nsswitch.conf fails but still
write out any changes to /etc/nsswitch.conf.

Signed-off-by: Jeff Mahoney <[email protected]>
@jeffmahoney
Copy link
Contributor Author

Ack. Changes made and tox run. Rebased on current HEAD to get rid of that merge commit.

Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

lgtm, thanks

Copy link
Collaborator

@spuiuk spuiuk left a comment

Choose a reason for hiding this comment

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

ACK

@mergify mergify bot merged commit d2650a5 into samba-in-kubernetes:master Mar 18, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants