fix regression devx-2689

JustBerkhout commented 3 years ago

Description

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

What behavior does this PR change, and why? This PR fixes a regression (also described in DEVX-2689):

DevX2676 introduces a sudo command (2x) to enable overwriting root-privileged files (in functions.sh | create_certificates()).
This assumes passwordless sudo is enabled on the docker host system
A fix was introduced approved and merged into 6.2.0-post in PR #396
This fix was overwritten 4 days later in commit dd3b2f65fafc845a0abb0a8cc4defd884a1c6af7

Author Validation

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

BEFORE code change: starting cp-demo with CLEAN=true was failing for me.
AFTER re-applying the changes from the previous commit/PR
I was able to run and re-run cp-demo with CLEAN=true locally (Win10/WSL2/Ubuntu20.04)
I was able to run and re-run cp-demo with CLEAN=true in gitpod

[x] Run cp-demo (on Win10/WSL2/Ubuntu20.04)
[x] jmx-monitoring-stacks (on Win10/WSL2/Ubuntu20.04)
[x] gitpod

Reviewer Tasks

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

[ ] Run cp-demo
[ ] jmx-monitoring-stacks
[ ] gitpod

gitpod-io[bot] commented 3 years ago

vdesabou commented 3 years ago

Looks like it was introduced in https://github.com/confluentinc/cp-demo/pull/399/commits/637f8de6e7eca1e491872da2291bb31c60c34c01

Thanks for spotting this ! LGTM The base branch confluentinc:6.2.x seems incorrect (should be confluentinc:6.2.0-post?) but @ybyzek will confirm

ybyzek commented 3 years ago

@JustBerkhout nice catch. I agree that this should target 6.2.0-post (plus a rebase)

JustBerkhout commented 3 years ago

Yes, I revalidated: ✔️ start CP demo locally (clean = true) ✔️ restart CP demo locally (clean = true) ✔️ start jmx-monitoring-stacks (prometheus/grafana) (clean = true) ✔️ restart jmx-monitoring-stacks (prometheus/grafana) (clean = true) ✔️ start with gitpod (with button in PR) ✖️ restart with gitpod (CLEAN=true ./scripts/start.sh) ✖️ restart with gitpod (base branch 6.2.0-post) (CLEAN=true ./scripts/start.sh) (Note in both these gitpod restart cases the environment seems to start up, but logging in with the well-known superUser credentials fails. Seeing this also occurs in the base branch, I don't hitnk this failing should block this PR403)

JustBerkhout commented 3 years ago

Thanks for your approve @ybyzek

I have a few questions: re reviewing: This change was successfully run on MacOS the first time around #396 , but not yet this time around. Also I'm not sure whether this change has been run on any Linux (outside of gitpod)

I'm in no great posititon to do either. No Mac, and I would have to set up shop in AWS to do a linux verify.

It looks like I cannot invite other reviewers on this PR.

What are the next steps/requirements to get this change merged?

confluentinc / cp-demo