Skip to content

Conversation

@hickey
Copy link
Contributor

@hickey hickey commented Apr 21, 2018

Habitus will now correctly run commands in a busybox or an Alpine container as they use /bin/sh for the shell rather than /bin/bash. In addition updated the stat command to get perms of artifacts to execute correctly under busybox.

@hickey
Copy link
Contributor Author

hickey commented Apr 21, 2018

Also, how does one join your Slack channel? I can not find a link anywhere on habitus.io to allow one to get an invitation to the Slack channel. Might be something you want to put up there if you want to build a community.

@khash
Copy link
Member

khash commented Apr 24, 2018

Hi @hickey

Thank you very much for the PR. I updated the link to our Slack channel in the github repo's README.

As for the PR content, I think it's better to remove the explicit reference to Busybox and instead allow overriding the shell command with default being /bin/bash while the option would allow the user to switch it to something like /bin/sh if they need. This way we'd have a more inclusive approach.

Would you be able to make that minor adjustment please?

@hickey
Copy link
Contributor Author

hickey commented Apr 26, 2018

Well, I started out that way by first starting to write a --shell switch to just override the shell being used. That worked until I hit the stat call that was being made and I had to put some more conditional logic to account for the two different arguments that need to be sent to stat. That is when I changed the switch over to --busybox. I can also foresee where there may be another time or two that there will need to be other conditional logic put in place depending on what features Habitus develops.

I briefly considered using --alpine as it was Alpine images that I was working with when I discovered the issue. But the more that I thought about it and started to realize that the common element that was driving the change was Busybox, it made sense to create the switch as --busybox. This also give a pretty good indication to others as to when the switch is needed.

Maybe you have a suggestion that is escaping me. One thing I would want to avoid (and I would think you would too) is to have 6 different command line switches needing various settings to configure for a specific OS.

@khash
Copy link
Member

khash commented Apr 30, 2018

I completely agree with your point about not having to configure 6 different things for each OS permutation.

How about making things a bit more generic by introducing a param called --os with a valid value of busybox for now and extending later? This way we won't have to keep adding new params for each of the permutations.

@hickey
Copy link
Contributor Author

hickey commented May 1, 2018

Oh, that would work pretty well.

How about I engineer it so that there is a default of 'linux' (I would like a better label, but can not think of one right now) that has the current functionality and then one can specify busybox or alpine to customize for that style of OS. A similar idea is instead of using 'linux' actually specify the family type of debian or redhat (proably default to debian since it is the most widely used base image) so that if at some point Habitus does use a OS specific command like yum or apt then the changes down the road would be easier and less disruptive.

I will wait to hear back before making any changes. No use having to continually revise code :-)

@khash
Copy link
Member

khash commented May 2, 2018

I like your idea of using the OS type (ie debian or suse). Let's go with that. Thanks!

@khash
Copy link
Member

khash commented Jul 12, 2018

Hi @hickey have you made any progress on this one? Would love to have it in the next release if we can.

@hickey
Copy link
Contributor Author

hickey commented Jul 12, 2018

Sorry, I have been retasked on a bunch of things and it got put to the side. I have a good portion of it done. Let me see if I can get it finalized for the early part of next week. Does that work for your schedule?

@khash
Copy link
Member

khash commented Jul 13, 2018

Absolutely! Thank you for the help @hickey

@hickey
Copy link
Contributor Author

hickey commented Aug 6, 2018

Sorry, I forget what I was doing that weekend, but never even thought about getting the changes complete. It occurred to me this weekend that there was work to be done. In any event, here is the final changes for the --os switch.

@khash khash merged commit e509082 into cloud66-oss:master Sep 17, 2018
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