minifabric icon indicating copy to clipboard operation
minifabric copied to clipboard

When running Apprun minifab container remains up if App does not return

Open vipinsun opened this issue 4 years ago • 19 comments
trafficstars

If the App does not return (as in the case of an app that starts a webserver) then the minifab name is taken up by the docker container which starts the app. Once that happens, the only way to run any minifab command is by doing docker stop "minifab container". Maybe the App should be started asynchronously and minifab should exit, just like it does with explorerup or portainerup commands..

vipinsun avatar Oct 13 '21 22:10 vipinsun

This was done on purpose. If one command fails or is still running, this prevents the next command from running. I think you should resolve why the app run is not returning.

litong01 avatar Oct 15 '21 10:10 litong01

The apprun is not returning because it sets up a webserver (similar to what explorer does). (see comment above)

vipinsun avatar Oct 15 '21 15:10 vipinsun

I propose new features: Alternative commands: appup and appdown similar to what happens with explorer. The main difference with explorer or portainer is the fact that the docker tag and build are not known beforehand. these have to be parametrized into the playbook.

As apps migrate up the foodchain this setup will become necessary as an app is often the portal into the blockchain. I have done the following: updated the apprun playbook in my branch to do docker portmapping to the 8081 port so that my webserver is exposed to the world. I have built and deployed my version of minifab that does this.

AppUp and AppDown commands so that we can run multiple docker built apps with different port mappings and tagnames deployed in the same network and channel as the spec.yaml.

I can work on this, if you think it is worthwhile @litong01

vipinsun avatar Oct 16 '21 18:10 vipinsun

@vipinsun @litong01 it might be better if app developper can defines his own playbook as well as application source code.

current apprun supports just single docker container application but it may need some dependant containers, such as database or others.

snippet folder layout is: app/{{appname}}/{{lang}}/

  • src/
  • playbooks/ apply.yaml (apprun.yaml, appdown.yaml) and others including templates.

itaru2622 avatar Nov 21 '21 00:11 itaru2622

@itaru2622 that sounds like a nice addition and should be relatively easy to do by adding a new parameter to mount user's location (assume the access permission is set correctly), then always mount to the place and run things off. Should not be difficult to add.

litong01 avatar Nov 22 '21 16:11 litong01

Sounds good. Do you need my help @litong01 @itaru2622 ?

vipinsun avatar Nov 25 '21 19:11 vipinsun

@vipinsun if you can help adding the feature, that will be nice. Please put up a PR and I will review as soon as possible. Thanks.

litong01 avatar Nov 29 '21 14:11 litong01

@vipinsun @litong01

app/{{appname}}/{{lang}}/

the above is too classified and app/{{appname}}/ may be enough since an app is rarely implemented in multiple languages.

itaru2622 avatar Nov 30 '21 04:11 itaru2622

@itaru2622 The issue is that using that path, minifabric knows what language the app is using, without it, you need another parameter. I would prefer not to adding another parameter. having another parameter is not better than have it in the path. And the worst is that it will prevent multiple language implementation of the same app.

litong01 avatar Dec 01 '21 13:12 litong01

@litong01 I think minifabric doesn't need to know the language of the app. playbook under app folder should have responsibility how to build, deploy, run the app. app may use gradle or maven when java app, otherwise go or nodejs(including typescript).

in this case minifab needs not to know language of app. minifab just needs to kick specific playbook file under app folder.

itaru2622 avatar Dec 01 '21 14:12 itaru2622

hmmm. then you need to specify how to get it started, right? currently we have this script to kick off either go or node, assuming that main.js or main.go available in their directory? If not knowing app's language, you will need to know the entry point.

litong01 avatar Dec 01 '21 14:12 litong01

@litong01 hmm, then, what should it specify the language when the app using different language in main and dependent containers.

I imagine that current ops/apprun/apply.yaml will be moved to new app/{{appname}}/playbook folder. current apprun/apply.yaml knows lanugage and runs app without any hint from spec.yaml nor command line option when it kicked.

itaru2622 avatar Dec 01 '21 15:12 itaru2622

@itaru2622 I do not really feel very comfortable asking user to author a yaml file in addition to their own application. what we can do to remove this will be assume user will provide a simple startapp.sh file which invokes their own app. That way, we provide a sample startapp.sh file, if user does not provide, then we use the default one which invokes the main.go or main.js (auto detection?) whatever we are doing currently for go and node. That maybe a better approach. If you agree with that, I think I can go with that approach to remove the language part.

litong01 avatar Dec 01 '21 16:12 litong01

@litong01 hmm, how about below approch?

app/{{appname}}/playbook/startapp.yaml kicks app/{{appname}}/startapp.sh in this case, lite users just need to edit startap.sh (startapp.yaml is just copied from minifab sample app) and heavy user can access allpeer etc easily by startapp.yaml

I think merit of minifab cares startapp is, configuring app easily according to spec.yaml and commandline options. your approch (user can edit just startapp.sh) misses it.

itaru2622 avatar Dec 01 '21 23:12 itaru2622

@itaru2622 I think that is what I am saying, user can do whatever they want to do with startapp.sh, we mount a directory /vars/app directory and call startapp.sh. that is all. a user can write their own startapp.sh, and user should have all the flexibility.

litong01 avatar Dec 02 '21 18:12 litong01

OK have we settled on the functionality and design? There will be an additional up or down parameter to be passed to the shell or maybe two separate shells startapp.sh and stopapp.sh - @litong01 @itaru2622 If so I can take a crack at it before the end of the year.

vipinsun avatar Dec 03 '21 14:12 vipinsun

@vipinsun the reason for stopapp.sh is to kill the application? are you saying that some of the apps are more like a server programs and wont finish on its own? If that is the case, would it be ok to add logic in minifab runapp command to detect if there is one running, then call stopapp.sh. if there isn't one running, call startapp.sh, I am trying hard to avoid adding more parameters and sort of keep the command stable.

litong01 avatar Dec 03 '21 14:12 litong01

Thinking about more of this, I would really prefer just one script for user to be able to write or overwrite. The script can be really simple or complex which is totally up to the user. For example, if a user wants to have start or stop capability, the user can have a small logic in that one script to set up a flag to indicate if it is starting or stopping. Most apps probably do not really need this kind of capability.

litong01 avatar Dec 03 '21 19:12 litong01

I assume that we are still not all the way with the functionality definition for apps @litong01 @itaru2622 ? I have revisited these comments from couple of years ago. Will it make sense to restate functionality?

  1. minifab behaves exactly the same without these parameters(or in absence of scripts). That is starts the app and waits until app is finished (default behavior)
  2. minifab needs to exit if app is webserver (since it app runs infinitely) with special parameter or scripts (startapp and stopapp)
  3. minifab needs to restart app if app is running, also special parameter with apprun or embedded in script
  4. minifab startapp.sh or stopapp.sh are optional scripts, either generated or otherwise, if they are found they will be executed accordingly
  5. Portmapping and envsettings for the app can also be introduced in these. Since I had to manually change the main script to achieve this result. Will post some samples before agreeing and creating PR.

vipinsun avatar Apr 25 '23 15:04 vipinsun