Provide an (additional) sample stack, one that is runnable by appsody

[henrynash]

To enable the "incremental" stack development approach, we are adding a second sample stack - one that can run by appsody, and hence can be copied and renamed to be a starter base for a new stack.

This PR also includes minor changes to the build scripts, so that they don't ignore the samples entirely (thanks @ebullient for this code).

Fixes #360

[chilanti]

@henrynash - @kylegc took a look at the PR - it's a really good start. Wondering if it could be enriched with simple steps to demonstrate kill (for example - adding a loop in hello.sh with a sleep in it would allow us to showcase the kill action.
Not sure if there are other aspects of the stack architecture that we could showcase easily (like having a run action different from the change action - that's probably harder to fold into this sample). @neeraj-laad - thoughts?

[kylegc]

For the record, copying my comment on slack here:
My concern with this is that if it becomes the exemplary stack sample, we need to be following all of our best practices. There are a few things, like the WATCH_REGEX and the ON_CHANGE calling the start commands which may not be best practices in my opinion. Of course we can always merge and iterate on it.

[henrynash]

@chilanti I like the idea of keeping the script running in a loop, since this also means that the kill variable can be set to the (much more usual) true state. This also reduces any changes a developer has to make when using this runnable stack as a template to modify.

FYI: The tutorial that uses this is currently in editorial review, but you can see a draft here: https://ibmdev1.rtp.raleigh.ibm.com/tutorials/create-appsody-stack/

[henrynash]

@chilanti So I did try putting in a sleep loop in the test app (hello.sh), i.e.:

while :
do
	echo "Hello from Appsody!"
	sleep 5
done

This actually causes a couple of issues:

1. It usually results in warning when you re-run the app ON_CHANGE (something to do with sleep not being part of bash?), e.g.:

[Container] Hello from Appsody!
[Container] Hello from Appsody!
[Container] Running command:  /bin/bash /project/userapp/hello.sh
[Container] [Warning] Wait Received error starting process of type APPSODY_RUN/DEBUG/TEST_ON_CHANGE while running command: /bin/bash /project/userapp/hello.sh error received was: signal: interrupt
[Container] Hello Again from Appsody!

2. Strangely it seems to also result in ON_CHANGE sometimes not being called at all (not sure I can explain that bit)

So while I will still poke at this, unless there is a strong need to add this right now, maybe we defer that to iterations once merged?

[henrynash]

So I investigated the use of a sleep loop in the hello script further.

Other than the fact that you do get a warning message (as documented above) when re-starting the "app", the other complication is that if you use an editor (e.g. vi) to change the hello script that is being executed to show how a change causes a restart, this doesn't trigger the re-start as expected. This is, I believe, due to the file system semantics of when you are editing a running file, it does a copy/replace and this causes the file modification date to NOT be reflected into the container file system via the volume mount. I validated this by experimentation and watching the file system dates reflected into the container. Using something like touch on the executing file DOES work, however. (This explains the behaviour I mentioned in my earlier comment).

I am open to whether the above makes it more or less useful to put a sleep loop into the hello.sh.

[kylegc]

@kewegner can you comment on the Warning message above? If I'm reading it correctly, it looks like the ON_CHANGE command is getting interrupted. Why is that?

@henrynash WRT vi not triggering changes, this smells like a bug but I'm not able to reproduce it. On my mac with stack nodejs-express, I'm able to successfully use vi to trigger a file change. So either 1) the behavior is different with your os/vi, or 2) your sample stack's watch regex isn't matching the file you are editing.

[henrynash]

@kylegc in terms on not triggering the watcher, the Regex is correct, since if you touch the file it works. I suspect the issue is specifically related to editing shell scripts while they are executing, due to how those editors have work. The modification date in the mounted file system really does not change in these circumstances - so this isn’t the watcher’s fault.

The interrupt i suspect is due to the script termination error code.

These are two reasons why I did not try to to a sleep loop in the shell script.

[kewegner]

RE: the warning. I believe the script terminated.

[kylegc]

Thats very odd. We are able to reproduce it and think there is a bug in the watcher.

[henrynash]

Suggestion: Unless there are strong concerns, maybe we could merge this PR as is (since it doesn't have the shell loop in it - so it doesn't suffer from the problems we have been discussing). I'll then raise a new PR to add the shell loop...and we can test and analyse that until we are happy?

[kylegc]

I'm okay with that. @neeraj-laad please review and merge if you approve.

[neeraj-laad]

I think we should not build additional samples index. We will allow users to get this sample and then build/test locally themselves using standard process.

[henrynash]

@neeraj-laad @ebullient So have looked at this some more. While I applaud the goal of minimising indexes, it also doesn't feel right that we would publish a runnable sample stack that we would not test as part of our travis builds. Further, without some changes to the travis side of things, the inclusion of a runnable sample in the samples directory breaks it (since some parts recognise it as a stack, but other parts don't build anything in the samples dir). While it would be possible, I suspect, to fix it up and make it build but avoided the index, I worry we will introduce more complications. We also, of course, want to use this stack with the new appsody stack create command. All this pushes me to say that sticking with the common index structure might actually be the best way to go (and swallow having to have the extra index), rather than making this special. Open to other ideas, however.

[neeraj-laad]

@henrynash Thanks for looking into this further. I am still not sure I am understanding the value of building this as an index.

A repo index allows an application developer to be able to init using that stack and get a template that they can work with. I don't think this is the intended purpose for this stack.

I'd prefer for stack creators to be able to say appsody stack create mystack --copy <sample-bash> and get a copy of this stack that they can start working with. They can then run appsody stack package and see their stack in action. and iterate over the process of making changes and testing it locally.

So, in my opinion the only thing we need is to ensure that the (to be released) users can specify they want to start with this stack when they run the appsody stack create command.

[kylegc]

While I don't particularly like having an index of "samples", I see the value in making this a full blown stack. That way we can treat it in the build process and the CLI as a normal stack and not have additional logic all over for samples. It actually makes the appsody stack create command logic simpler too for this same reason.

Proposal: What if we publish this to the "incubator" (a.k.a. appsodyhub) index with a better name so it fits in? Possible names: "slim", "base", "minimal"

[henrynash]

@kylegc I'd be happy with that (and @ebullient and I did briefly discuss that when we started this). base or minimal work for me.

@neeraj-laad Agree with the goal! However, if we don't create a new index (and I'm fine with that in principle), unless we change something in the ci system, the inclusion of a runnable sample stack breaks our current ci build/test scripts, when you build everything (because they assume that since they find what looks like a real stack, it should be in an index, and gets all confused. Early versions of my PR failed the travis build/test checks because of this). My point was therefore that we would need to modify the ci scripts to build and test a stack that wasn't in an index....which would need some special casing that we don't have right now.

[neeraj-laad]

@henrynash I completely agree with your comment regarding the ability to maintain a stack that is not in any index and is not tested/built like others.

[henrynash]

@neeraj-laad I have updated the PR with the runnable sample being placed in incubator.

[neeraj-laad]

I am also ok with adding this to an existing index, but I think we should base the decision on how we believe people will use it:

Do we think developers will want to use this as an appsody stack "as-is" and work with it? (similar to how they would use nodejs or `similar stack? Can we think of scenarios where this might be useful?

If the answer is yes, we should add it to one of our existing repos that are indexed - incubator will be a good one. and PErhaps the name should indicate what capability they get from the stack. For me maybe bash or unix shell is more useful than minimalistic.

If the answer is no, and we think this is something that stack creators will use as a starting point then it should not be in one of our standard indexes. But, I also agree we need to have this tested in CI to make it useful. Perhaps we can build and test this as a sample in our CI but not publish the samples index during the release.

[henrynash]

@kylegc @neeraj-laad

So I do not believe people will use this stack as is, expect perhaps the initial step or running it to makes sure they local configuration is good. The stack is intended to really to be copied and then modified. While I agree it is a perfectly reasonable approach to have this built and tested by our scripts but not included in an index (provided we do NOT also have to do something special for appsody stack create)...is the extra work in modifying the build scripts worth it (and maintaining the special case going forward)? I'm not sure I see the down side of it being in one of the standard indexes.

(As an aside, from a IA Customer Dev Advocacy point of view, we are running low on time....we have a whole slew of Kabanero content due to be published on Wednesday...including docs that use this runnable sample.....so it would be ideal if we can resolve this issue quickly, so I can update those docs with whatever is decided :-). Here's the main tutorial that references the runnable stack: https://github.ibm.com/IBMCode/Code-Tutorials/blob/master/create-appsody-stack/index.md)

[neeraj-laad]

LGTM