Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add jepsen-mysql tests #465

Closed
wants to merge 1 commit into from
Closed

add jepsen-mysql tests #465

wants to merge 1 commit into from

Conversation

vladbogo
Copy link
Collaborator

@vladbogo vladbogo commented Jun 7, 2024

@vladbogo vladbogo force-pushed the jepsen branch 8 times, most recently from b13d10d to 2fa007b Compare June 9, 2024 17:39
@vladbogo
Copy link
Collaborator Author

@fauust can you pls help with the following questions:

  • where is the buildbot user created? Wouldn't the buildbot-worker.Dockerfile be the last one?
  • how to handle this user creation that currently fails since buildbot doesn't have permissions in /home/buildbot

@fauust
Copy link
Collaborator

fauust commented Jun 10, 2024

where is the buildbot user created? Wouldn't the buildbot-worker.Dockerfile be the last one?

The buildbot user is created by buildbot-worker.Dockerfile. So probably you should try to include this before the jepsen docker file.

how to handle this user creation that currently fails since buildbot doesn't have permissions in /home/buildbot

In the CI:

-           - dockerfile: debian.Dockerfile jepsen-mysql.Dockerfile
+           - dockerfile: debian.Dockerfile buildbot-worker.Dockerfile jepsen-mysql.Dockerfile

But apart from that, the PR should describe a bit better what those tests are or at least who requested them (jira task?). Who is going to look at them, who is going to be able to fix them? I think that before implementing new tests, those questions should be discussed. I just went back from time off so maybe that was already discussed?

Also, does it make sens to have a different naming/CI file for those "ecosystem" images?

@vladbogo
Copy link
Collaborator Author

where is the buildbot user created? Wouldn't the buildbot-worker.Dockerfile be the last one?

The buildbot user is created by buildbot-worker.Dockerfile. So probably you should try to include this before the jepsen docker file.

yes, but somehow the user still exists and I don't understand from where since if I try to create it in jepsen it fails with "user already exists" as you can see here https://github.com/MariaDB/buildbot/actions/runs/9438346213/job/25995220974#step:10:3975, so why does it already exist?

how to handle this user creation that currently fails since buildbot doesn't have permissions in /home/buildbot

In the CI:

-           - dockerfile: debian.Dockerfile jepsen-mysql.Dockerfile
+           - dockerfile: debian.Dockerfile buildbot-worker.Dockerfile jepsen-mysql.Dockerfile

moving jepsen after buildbot-worker.Dockerfile I think it's not clean since than we will need to re-define the CMD. Also, wouldn't the above solution add buildbot-worker.Dockerfile twice? So I would better suggest changing the permissions in jepsen dockerfile, but I still don't know where the user (or the path) comes from

But apart from that, the PR should describe a bit better what those tests are or at least who requested them (jira task?). Who is going to look at them, who is going to be able to fix them? I think that before implementing new tests, those questions should be discussed. I just went back from time off so maybe that was already discussed?

Also, does it make sens to have a different naming/CI file for those "ecosystem" images?

This was requested on Slack and this PR is just an adaptation of the work done by Vladislav Lesin. There is no Jira ticket as far as I am aware.

@fauust
Copy link
Collaborator

fauust commented Jun 11, 2024

@vladbogo here is the patch, but as discussed let's discuss with @grooverdan how to implement this.

diff --git a/ci_build_images/jepsen-mysql.Dockerfile b/ci_build_images/jepsen-mysql.Dockerfile
index d858f42..c0931e5 100644
--- a/ci_build_images/jepsen-mysql.Dockerfile
+++ b/ci_build_images/jepsen-mysql.Dockerfile
@@ -31,9 +31,6 @@ RUN apt-get update && apt-get -y install --no-install-recommends \
     curl \
     && apt-get clean
 
-# Create a buildbot user
-RUN adduser --disabled-password -gecos "" buildbot
-
 USER buildbot
 
 WORKDIR /home/buildbot
@@ -47,6 +44,6 @@ WORKDIR /home/buildbot/jepsen-mysql
 
 # Create necessary directories and run leiningen to download dependencies
 RUN mkdir -p /home/buildbot/mariadb-bin/tmp \
-    && ../lein run test --help
+    && ~/lein run test --help
 
-WORKDIR /home/buildbot
\ No newline at end of file
+WORKDIR /home/buildbot
diff --git a/ci_build_images/qpress.Dockerfile b/ci_build_images/qpress.Dockerfile
index 5ed1784..d2a90e1 100644
--- a/ci_build_images/qpress.Dockerfile
+++ b/ci_build_images/qpress.Dockerfile
@@ -2,6 +2,8 @@
 # qpress.Dockerfile
 # those steps are common to all images
 
+USER root
+
 # install qpress (MDEV-29043)
 COPY qpress/* /tmp/qpress/
 WORKDIR /tmp/qpress

Also, the lein binary should probably go into /home/buildbot/bin which is more 'standard'.

@fauust
Copy link
Collaborator

fauust commented Jun 19, 2024

superseeded by #468

@fauust fauust closed this Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants