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

chore(deis-builder-rc.yaml,Dockerfile): remove entrypoint and specify it in the manifest #36

Closed
wants to merge 1 commit into from

Conversation

arschles
Copy link
Member

No description provided.

@technosophos
Copy link
Member

What's your thinking about how we should do this across projects? Is this solving a special case, or are you thinking this is the way we should do things?

@bacongobbler
Copy link
Member

I think we should leave this in the Dockerfile, IMO. That way we can still use this docker image outside of kubernetes without having to know the boot incantation.

I think this also break things as /bin/boot is not called in entrypoint.sh from what I can see.

@arschles
Copy link
Member Author

Sorry about the upcoming wall of text:

@technosophos I made this change so I could use the mc client in the image locally, for debugging purposes. I had intended to back it out but realized that, since this image has more than 1 artifact in it (even though that may not be a best practice), I believe it's overall more flexibly to not have an ENTRYPOINT.

I won't say that we should never use ENTRYPOINT, but in this case I think the flexibility of removing it is worthwhile.

Relating to what @bacongobbler said, I could certainly add the CMD back in, and I'm now thinking that is probably a happy medium - we could remove the command: from the manifest while keeping the simplicity and retaining the aforementioned flexibility.

Also, the Makefile builds a builder binary in this project instead of a boot. I know the prototype repo builds boot so I'm considering it standard. I've created #37 to make the change later.

Thoughts on any of the above?

@bacongobbler
Copy link
Member

+1 on migrating the ENTRYPOINT to the boot script. That'll give you the ability to use the mc client while still retaining backwards compat.

@technosophos
Copy link
Member

Sounds good. The binary named builder is an artifact of the 1.x tree and should be changed.

@arschles
Copy link
Member Author

tracking the builder=>boot change in #37

@smothiki
Copy link
Contributor

smothiki commented Jan 6, 2016

Im good with this . I'll give LGTM once the rebase is done

@arschles
Copy link
Member Author

closing, as #72 made these changes anyway

@arschles arschles closed this Jan 15, 2016
@arschles arschles deleted the clean-dockerfile branch January 15, 2016 20:55
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.

4 participants