Devx 2672: Enable cp-demo to be run in gitpod.io

vdesabou commented 3 years ago

Description

https://confluentinc.atlassian.net/browse/DEVX-2672

What behavior does this PR change, and why?

Allow to run cp-demo using Gitpod.io

Pre-merge Checklist

[x] https://github.com/confluentinc/examples/pull/1036 is reviewed and merged
[ ] Validate that new branch automatically triggers creation of new gitpod config (e.g. did 6.2.1-post work?)
[x] Discuss support for issues -- who will support issues/questions that arise from Gitpod environments, i.e., triage to see if it's related to Gitpod or not?
[x] Confirm there are no usage metrics available in Gitpod (see https://confluentinc.atlassian.net/browse/DEVX-2672 for discussion, but looks like the answer is this is not available today)
[x] Validate docs build passes with the changes made to the rst files
[x] Validate smoke tests at https://github.com/confluentinc/cp-demo/tree/6.2.0-post/scripts/validate
[x] Additional validation called out in jira, which @vdesabou has already checkmarked below

Author Validation

Describe the validation already done, or needs to be done, by the PR submitter.

[x] Run all steps from https://docs.confluent.io/platform/current/tutorials/cp-demo/docs/on-prem.html#module-1-on-prem-tutorial
[x] Run all steps from https://docs.confluent.io/platform/current/tutorials/cp-demo/docs/hybrid-cloud.html
[x] jmx-monitoring-stacks

Reviewer Tasks

Describe the tasks/validation that the PR submitter is requesting to be done by the reviewer.

chuck-confluent commented 3 years ago

@ybyzek Note that to enabled prebuilds, whoever has appropriate repo permissions would have to install the gitpod app in GitHub and restrict its access to the cp-demo repo (and any other repo where we want prebuilds).

ybyzek commented 3 years ago

@ybyzek Note that to enabled prebuilds, whoever has appropriate repo permissions would have to install the gitpod app in GitHub and restrict its access to the cp-demo repo (and any other repo where we want prebuilds).

I did this last week and, @vdesabou can correct me if needed, but I think there is nothing outstanding to enable prebuilds.

chuck-confluent commented 3 years ago

@mikebin @rspurgeon it looks like merge is blocked by your review. Thoughts on this? It would be great to get this out.

Fun fact, you can actually run this PR branch IN GITPOD with this link: https://gitpod.io/#https://github.com/vdesabou/cp-demo/blob/devx-2672 :-)

chuck-confluent commented 3 years ago

@vdesabou looks like it's just a couple of minor tweaks and then we can get this out there! I appreciate your work on this and I look forward to seeing it live after it gets merged.

ybyzek commented 3 years ago

@mikebin @rspurgeon it looks like merge is blocked by your review. Thoughts on this? It would be great to get this out.

Fun fact, you can actually run this PR branch IN GITPOD with this link: https://gitpod.io/#https://github.com/vdesabou/cp-demo/blob/devx-2672 :-)

I have updated the Description of this PR with a Pre-merge Checklist section that needs to be completed before merge. Normally we wouldn't have such a checklist, but this is a new workflow so just want to make sure we do the preparation upfront.

rspurgeon commented 3 years ago

@chuck-confluent @vdesabou I tried running this PR in gitpod without success. I'm confused by this line:

init: |
      ./scripts/start.sh && ./scripts/stop.sh

In the terminal I can see that the resources are immediately destroyed after they are created, so i'm confused how one would use the demo. I tried following the instructions in our docs (per the note at the end of the gitpod init script), but nothing works because the services have been destroyed.

rspurgeon commented 3 years ago

FWIW I also verified that these changes do not break a local execute of the demo on Ubuntu

chuck-confluent commented 3 years ago

@rspurgeon There's a couple gitpod specific things to consider and it might be better to have a short conversation rather than go back in forth in comments. I'll respond to some things, but if that's not helpful then a chat or maybe a "PR huddle" might go a long way.

I tried running this PR in gitpod without success.

What is the definition of success? The demo starts and stops successfully as far as I can tell. I can subsequently start again successfully.

In the terminal I can see that the resources are immediately destroyed after they are created, so i'm confused how one would use the demo.

This is a gitpod thing. The init command will only execute on prebuilds. I find this image from the Gitpod docs to be helpful to understand how start tasks work. After the first prebuild (which I think happens automatically on merge and any subsequent push if prebuilds are set up, which Yeva confirmed they are), users will enter the workspace and only the command command will run (echo in this case). The user would then run ./scripts/start.sh like in the directions. We could probably change the command command to go ahead and start the demo for the user automatically to save them a step if we want.

how does a user connect to C3 if it's running in Gitpod?

Gitpod shows the active ports in the "remote explorer" and you can open web services on those ports in the browser. Here is a screen grab, and if you hover over the port, there will be an icon to open in a browser tab.

When I click the icon, it takes me to a new browser tab (notice the URL)

I have cp demo running in gitpod right now. Here's Kibana running in my browser from the Gitpod workspace, for example:

rspurgeon commented 3 years ago

This is a gitpod thing. The init command will only execute on prebuilds. I find this image from the Gitpod docs to be helpful to understand how start tasks work. After the first prebuild (which I think happens automatically on merge and any subsequent push if prebuilds are set up, which Yeva confirmed they are), users will enter the workspace and only the command command will run (echo in this case). The user would then run ./scripts/start.sh like in the directions. We could probably change the command command to go ahead and start the demo for the user automatically to save them a step if we want.

In this case I don't see the value in running start && stop. If start fails, will it leave the workspace in a semi started / broken state for subsequent users or have we verified the prebuild stage fails in a graceful way? If the goal is to cache docker images, we could use (cd scripts; source env.sh; docker-compose pull) in the prebuild init stage instead.

Then if the goal is for the user to run the start script themselves, we should point them directly to the On-Prem instructions instead of the landing page for cp-demo (which could could be confusing if you land there from GitPod): https://docs.confluent.io/platform/current/tutorials/cp-demo/docs/on-prem.html#cp-demo-on-prem-tutorial. I think the instructions from here could also be confusing because the user doesn't know if they need to install pre-reqs, do the git clone and checkout, etc... Maybe the command script shoudl have instructions added that are more specific to the GitPod experience and then point the user to a specific anchor in the cp-demo instructions where they should pickup

chuck-confluent commented 3 years ago

How the init step works is it must succeed for the prebuild to be successful and the snapshot to be created. Users will never start from a failed state in the way you described. They will start from a previous successful prebuild snapshot if it exists, or in the worst case, it will run from scratch (not ideal, so we might want to ad a check whether the prebuild passes). The start/stop scripts are actually an appropriate use of init because it actually tests whether the demo works end to end. A docker pull would not catch new code pushes that break the demo. We don't want a prebuild to succeed if it isn't actually able to complete the demo. On the other hand, new code pushes are already tested in other CI systems, so that may be unnecessary. I could go either way.

I actually just manually triggered a successful prebuild (doc) on Vincent's branch, so this link should now be a little more indicative of the user experience (prebuild saved 14 minutes for the user!).

Then if the goal is for the user to run the start script themselves, we should point them directly to the On-Prem instructions instead of the landing page for cp-demo

I agree, and I'll take back my review green light and suggest also putting ./scripts/start.sh in the command start task to be run for users. I think also there should be something in the gitpod paragraph about opening browser tabs from ports in the "Remote Explorer".

rspurgeon commented 3 years ago

actually just manually triggered a successful prebuild (doc) on Vincent's branch, so this link should now be a little more indicative of the user experience (prebuild saved 14 minutes for the user!).

I think it's a little confusing for the user to see the entire output of the test run (init stage) when they go to a prebuild link. Can we either supress that w/ GitPod somehow or can we redirect output of the init stage to a file so the user gets a clean looking terminal when they initiate a workspace from the prebuilt snapshot?

vdesabou commented 3 years ago

Sorry all, I did not get notifications and completely missed your comments

Regarding @chuck-confluent comments:

add ./scripts/start.sh to the command portion of the start task so that cp demo starts on launch for users.

Yes I thought about it but a common use case for GTS is that we make a few modifications before starting cp-demo. Therefore I propose to make it optional by doing the following:

      if [ -z "$DISABLE_AUTOSTART" ]; then echo "🚀 Starting up cp-demo (you can disable autostart by exporting DISABLE_AUTOSTART environment variable, see https://www.gitpod.io/docs/environment-variables";./scripts/start.sh; echo "🚀 You can now follow steps in https://docs.confluent.io/platform/current/tutorials/cp-demo/docs/on-prem.html#guided-tutorial"; else echo "ℹ️ DISABLE_AUTOSTART environment variable is set, use ./scripts/start.sh to start cp-demo";fi

This way if DISABLE_AUTOSTART is set (see docs), then we don't start automatically and we print:

ℹ️ DISABLE_AUTOSTART environment variable is set, use ./scripts/start.sh to start cp-demo

Otherwise we have:


🚀 Starting up cp-demo (you can disable autostart by exporting DISABLE_AUTOSTART environment variable, see https://www.gitpod.io/docs/environment-variables)

./scripts/start.sh ****** this is executed

🚀 You can now follow steps in https://docs.confluent.io/platform/current/tutorials/cp-demo/docs/on-prem.html#guided-tutorial
``

> Point users to the on prem instructions guided tutorial rather than cp demo overview

Agreed, see above

> In the gitpod paragraph, add instructions about how to open browser tabs from open ports, preferably with screenshots

ok

gitpod-io[bot] commented 3 years ago

vdesabou commented 3 years ago

In term of documentation, I've added this:

CleanShot 2021-10-06 at 15 44 26

vdesabou commented 3 years ago

actually just manually triggered a successful prebuild (doc) on Vincent's branch, so this link should now be a little more indicative of the user experience (prebuild saved 14 minutes for the user!).

I think it's a little confusing for the user to see the entire output of the test run (init stage) when they go to a prebuild link. Can we either supress that w/ GitPod somehow or can we redirect output of the init stage to a file so the user gets a clean looking terminal when they initiate a workspace from the prebuilt snapshot?

This is now handled by 648b919650b4a940213cb8e973dfc611984ba760

As a result, we have:

CleanShot 2021-10-06 at 17 04 51@2x

vdesabou commented 3 years ago

@ybyzek @rspurgeon @chuck-confluent Sorry for the delay, I think I have now addressed all your comments. Let me know if I missed anything !

chuck-confluent commented 3 years ago

I'm sorry, this is too cool:

I can open C3 as a preview inside the IDE and browse code while I browse topics in C3 or dashboards in kibana. This is SO exciting!

vdesabou commented 3 years ago

Is there any action required on my side ?

vdesabou commented 3 years ago

@vdesabou can you please go through the checklist in the description of this PR before it gets merged?

Sure :D

Validate that new branch automatically triggers creation of new gitpod config (e.g. did 6.2.1-post work?)

I don't think I can validate that until the merge is done, as Gitpod.io needs to have the .gitpod.yml file to be present in the branch. But I'm pretty confident that it works since the branch for this PR is working fine, see the comment https://github.com/confluentinc/cp-demo/pull/390#issuecomment-936193226 The link in there is working fine https://gitpod.io/#https://github.com/confluentinc/cp-demo/pull/390 Which means that gitpod.io is able to prebuild cp-demo branches having .gitpod.ymlfile present

Validate smoke tests at https://github.com/confluentinc/cp-demo/tree/6.2.0-post/scripts/validate

I've run all 4 scripts and they were all OK

Discuss support for issues -- who will support issues/questions that arise from Gitpod environments, i.e., triage to see if it's related to Gitpod or not?

I'll be happy to help supporting this

Confirm there are no usage metrics available in Gitpod (see https://confluentinc.atlassian.net/browse/DEVX-2672 for discussion, but looks like the answer is this is not available today)

This is not available, see https://community.gitpod.io/t/usage-monitoring-and-statistics/715

Validate docs build passes with the changes made to the rst files

I've followed https://github.com/confluentinc/cp-demo/blob/6.2.0-post/docs/README.md and build locally. I was able to see my changes are not breaking existing doc. See also the screenshots I posted.

confluentinc / cp-demo