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

docker: allow IPFS_PROFILE to choose the profile for ipfs init #5473

Merged
merged 2 commits into from
Nov 26, 2018

Conversation

akaihola
Copy link

@akaihola akaihola commented Sep 15, 2018

Added support for an IPFS_PROFILE environment variable in the bin/container_daemon. The variable, if present, is used with ipfs init to add a --profile=$IPFS_PROFILE argument which will apply to chosen profile to the newly created configuration.

This is useful for automatic IPFS deployments in hosted container environments, e.g. Kubernetes. In such situations, it's essential to use --profile=server.

Pull request helm/charts#7746 requires this change in order to add the profile configurable parameter to the IPFS Helm chart.

@akaihola
Copy link
Author

It seems GitCop rejected my commit message as invalid, but I can't find the guidelines for commit messages. My message is 64 characters long, so that shouldn't be the issue. Advice, anyone?

@rob-deutsch
Copy link
Contributor

@akaihola guidance is here: https://github.com/ipfs/go-ipfs/blob/master/contribute.md

The commit message is missing the trailers.

License: MIT
Signed-off-by: Antti Kaihola <antti+ipfs@kaihola.fi>
@akaihola
Copy link
Author

akaihola commented Sep 19, 2018

@rob-deutsch, thanks for your explanation and sorry for missing those instructions.

I now corrected the commit message and force pushed the branch.

Stebalien
Stebalien previously approved these changes Sep 24, 2018
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

This may not be necessary, but I can never remember the precise rules.

ipfs init
case $IPFS_PROFILE in
"") INIT_ARGS= ;;
*) INIT_ARGS=--profile=$IPFS_PROFILE ;;
Copy link
Member

Choose a reason for hiding this comment

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

Let's quote this. That is, INIT_ARGS="...".

bin/container_daemon Show resolved Hide resolved
License: MIT
Signed-off-by: Antti Kaihola <antti+ipfs@kaihola.fi>
@funkyfuture
Copy link
Contributor

@Stebalien afaict, your remaining concern has been resolved (it is marked as outdated in the view), and you might approve this now.

@Stebalien
Copy link
Member

@whyrusleeping @lgierth any objections?

@whyrusleeping
Copy link
Member

Just for the docker stuff? Seems fine to me

Copy link
Member

@Kubuxu Kubuxu left a comment

Choose a reason for hiding this comment

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

SGMT.
It will do until we need to pass more things as INIT_ARGS and then we can use bash's array syntax.

@victorb
Copy link
Member

victorb commented Nov 21, 2018

Any chance of merging this? Makes infra a lot simpler (no need to have a separate Dockerfile to just override the profile), seems straightforward and been approved.

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.

8 participants