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

Add ability to specify HOMEDIR and a test suite #5

Closed
wants to merge 2 commits into from

Conversation

athornton
Copy link
Member

No description provided.

@athornton athornton force-pushed the tickets/DM-42034 branch 6 times, most recently from 5c993a9 to 7a99921 Compare December 5, 2023 23:45
@athornton athornton requested a review from rra December 5, 2023 23:47
@athornton
Copy link
Member Author

I think I need to be more careful with directory creation: I can't just set the bottom layer to 0700, I need to remember which directories I created and chmod each one of those, which means I can't just do a mkdir -p.

@athornton
Copy link
Member Author

Or, wait, with umask I can guarantee that, can't I?

Copy link
Member

@rra rra left a comment

Choose a reason for hiding this comment

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

This is beyond my complexity threshold for what I think makes sense to do in shell. I don't really want to think hard enough to figure out what all those cuts and revs and whatnot are doing and whether all the quoting is correct, particularly now that there's a test suite in shell. (For example, I'm fairly sure the assignments to r_uid and r_gid need to quote $HOMEDIR in case it contains spaces.)

Can we switch to Python? I know that means a larger container but we use the current Python slim container as a base for basically everything so it's highly likely it's cached. We can just not update that container with new Debian packages and rely on the fact that new Python containers are built regularly and dependabot will switch us to them. We can test performance if needed but I bet it won't be a problem.

Since this is a major version upgrade and there are a bunch of other incompatible changes in Nublado, I think we should drop support for the home directory not being passed in, and I'd also like to standardize the protocol used to communicate with init containers to namespace our environment variables. The latter isn't a big deal but since we're making changes anyway, we may as well follow best practices for environment variables and use NUBLADO_UID, NUBLADO_GID, and NUBLADO_HOME or something like that as the interface.

@athornton
Copy link
Member Author

Well, if we're gonna do it that way, then may as well transplant it over into the monorepo where we'll have our python tooling anyway.

@athornton athornton closed this Dec 6, 2023
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