Skip to content

Conversation

@jeqo
Copy link
Contributor

@jeqo jeqo commented Nov 2, 2021

Description

What behavior does this PR change, and why?

While enabling gitpod on jmx-monitoring-stacks repo (confluentinc/jmx-monitoring-stacks#54), it made sense to extend cp-demo instead of copy/paste'ing most of the gitpod configuration in another project.

This PR adds a new variable MONITORING_STACK to choose which stack to start, if any, when starting cp-demo on gitpod.

Author Validation

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

Reviewer Tasks

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

@gitpod-io
Copy link

gitpod-io bot commented Nov 2, 2021

@vdesabou
Copy link
Member

vdesabou commented Nov 3, 2021

Am I the only one getting for https://gitpod.io/#https://github.com/confluentinc/cp-demo/pull/401 ?

CleanShot 2021-11-03 at 11 22 37@2x

@jeqo
Copy link
Contributor Author

jeqo commented Nov 3, 2021

@vdesabou it works for me: https://gray-bee-8m5ifxwq.ws-us17.gitpod.io/
Screenshot 2021-11-03 at 05 24 45

@jeqo jeqo marked this pull request as ready for review November 3, 2021 15:17
Copy link
Contributor

@chuck-confluent chuck-confluent left a comment

Choose a reason for hiding this comment

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

@jeqo Hey, I'm taking over maintenance of CP demo and I'd like to get this PR merged. Just a couple of minor comments, but mostly looks good!

.gitpod.yml Outdated
init: ./scripts/start.sh && ./scripts/stop.sh && gp sync-done prebuild
command: curl -L --http1.1 https://cnfl.io/ccloud-cli | sudo sh -s -- -b /usr/local/bin; exit
init: |
cd .. && git clone https://github.com/confluentinc/jmx-monitoring-stacks && cd cp-demo/. && \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the jmx repo should only be cloned if the user specifies the MONITORING_STACK variable. I think this should move to the "cp-demo" task

.gitpod.yml Outdated
command: curl -L --http1.1 https://cnfl.io/ccloud-cli | sudo sh -s -- -b /usr/local/bin; exit
init: |
cd .. && git clone https://github.com/confluentinc/jmx-monitoring-stacks && cd cp-demo/. && \
./scripts/start.sh && ./scripts/stop.sh && \
Copy link
Contributor

Choose a reason for hiding this comment

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

Something I'm planning to do anyway is to replace ./scripts/start.sh && ./scripts/stop.sh with simply CLEAN=true ./scripts/start.sh. This makes a better experience if there is no prebuild enabled. There's no reason to require stop.sh. I think it's fine to add it to this PR because I don't want you to have to rebase and fix the conflict.

.gitpod.yml Outdated
echo "🚀 Starting up cp-demo (you can disable autostart by exporting DISABLE_AUTOSTART environment variable, see https://www.gitpod.io/docs/environment-variables)";
if [ -z "$MONITORING_STACK" ]; then
echo "cp-demo is not including any JMX monitoring stack (you can enable them by exporting MONITORING_STACK environment variable, see https://github.com/confluentinc/jmx-monitoring-stacks)."
./scripts/start.sh;
Copy link
Contributor

Choose a reason for hiding this comment

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

change to CLEAN=true ./scripts/start.sh

.gitpod.yml Outdated
else
echo "cp-demo includes JMX monitoring stack $MONITORING_STACK"
cd ../jmx-monitoring-stacks
./$MONITORING_STACK/start.sh;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest the JMX repo also set CLEAN=true when starting CP Demo in its own start scripts. My thinking is it's possible for a prebuild to create certificates and junk that should probably be regenerated.

@jeqo jeqo force-pushed the gitpod-monitoring branch from a312c72 to b75bc4d Compare April 11, 2022 19:12
@jeqo jeqo changed the base branch from 6.2.1-post to 7.0.1-post April 11, 2022 19:50
@chuck-confluent
Copy link
Contributor

chuck-confluent commented Apr 11, 2022

I'm working on some other gitpod prebuild optimizations in #419 that would need to go through before adding this. Hold tight, and thank you for contributing this!

@chuck-confluent
Copy link
Contributor

Sorry for the delay on this. I've since discovered that CP demo is pushing the resource limitations of gitpod quite a bit, so i don't think it's feasible to run more components. I'll have to close this PR, unfortunately.

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.

3 participants