🌱 Fix hack/ci-e2e.sh script installations #2699

sirpaleet · 2025-09-25T11:28:51Z

What this PR does / why we need it:

Some people may find the installation process and running the hack e2e-scripts a bit difficult, so making a target for the script and fixing the installed packages and dependencies should be done to make it easier to run them on a VM, and documentation should be added as well.

There is some previous discussion in here: #2642

metal3-io-bot · 2025-09-25T11:28:55Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

metal3-io-bot · 2025-09-25T11:28:57Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign honza for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

sirpaleet · 2025-09-29T06:10:46Z

In case no-one wants to pick this up I can just make this PR about this one target as well, and add some comments to documentation :)

Makefile

sirpaleet · 2025-09-29T07:54:14Z

hack/e2e-prerequisites.sh

@@ -0,0 +1,55 @@
+#!/usr/bin/env bash


Added this to script

sirpaleet · 2025-09-29T07:57:18Z

Additional checks for if installations exist could be added.
The installations could be made to the project's bin, but then in other places the scripts would not know how to find them.

TODO:

test-e2e is used in ci-e2e.sh script so make a new target
expand on clean-e2e to have optional steps on cleanup with flags
update info in docs/testing.md and test/e2e/README.md

Makefile

hack/e2e-prerequisites.sh

sirpaleet · 2025-09-29T11:01:54Z

I noticed that in here: https://github.com/metal3-io/baremetal-operator/blob/main/hack/ci-e2e.sh#L57 there is clear attempt at making the script self-sufficient and it should probably install all needed packages. But alas it does not. I'll expand on just this and remove the new target :D + edit the other scripts to fit this

sirpaleet · 2025-09-29T12:58:38Z

This has now been confined to the scope of installations, and adding the script target. I'll open the PR but I will most likely not have the time to edit all changes if they are major. I'll make an issue on the "TODO" i added before.

EDIT: #2707 has the cleanup issue, and #2708 has the kubectl version bump (which I unfortunately do not have time for)

I noticed that in here: https://github.com/metal3-io/baremetal-operator/blob/main/hack/ci-e2e.sh#L57 there is clear attempt at making the script self-sufficient and it should probably install all needed packages. But alas it does not. I'll expand on just this and remove the new target :D + edit the other scripts to fit this

Additionally based on this finding, changed from a new target for installations to adding the necessary steps to the hack/ci-e2e.sh and making a new ensure_docker.sh for the docker installation.

hack/e2e/ensure_docker.sh

sirpaleet · 2025-09-30T06:37:56Z

hack/e2e/ensure_docker.sh

@@ -0,0 +1,32 @@
+#!/usr/bin/env bash
+# shellcheck disable=SC1091


Added this to fix the shellcheck issue, this code is from the official installation instructions of installing Docker Engine on Ubuntu.

hack/ci-e2e.sh

hack/e2e/ensure_docker.sh

tuminoid · 2025-09-30T07:24:10Z

hack/e2e/ensure_docker.sh

+            sudo apt-get update
+            sudo apt-get install -y docker-ce docker-ce-cli containerd.io docker-buildx-plugin docker-compose-plugin
+            sudo usermod -aG docker "${USER}"
+            newgrp docker


This does nothing

I'm confused as to which part, since it works for me? The new docker group?

tuminoid · 2025-09-30T07:26:37Z

Makefile

+## To note: Run this on your VM and not your local machine. These installations
+## work for linux amd64, at least on Ubuntu 24.04. You may need to log out and
+## log in to apply all changes if the installations are done for the first time.
+.PHONY: run-e2e-script


this is not descriptive name for the target... setup-e2e or such would be nicer

I agree the naming is a bit clunky, it's just that the "setup" would imply installations and anything more vague would imply it is used in actual e2e runs elsewhere no? Or maybe not?

Edit Makefile to add run-e2e-script, fix installations, and add documentation. Signed-off-by: Siiri Kemppainen <[email protected]>

sirpaleet · 2025-09-30T08:29:56Z

Left a couple of comments, and fixed some things. This is as much as I have time today, in case someone has an opinion of the furture of this PR I'm all ears but contributing to this has to end on my part for now :( I'll try to answer if someone has a question though!

sirpaleet · 2025-09-30T12:08:06Z

hack/ci-e2e.sh

-sudo apt-get update
-sudo apt-get install -y libvirt-dev pkg-config
+sudo apt-get install -y libvirt-daemon-system qemu-kvm virt-manager libvirt-dev
+make build


This might be repetition with L82, but this does some necessary steps also, just might slow down the script

metal3-io-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 25, 2025

metal3-io-bot requested review from Rozzii and zaneb September 25, 2025 11:28

metal3-io-bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 25, 2025

tuminoid reviewed Sep 29, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

nuhakala reviewed Sep 29, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

sirpaleet force-pushed the siiri/makefile-e2e-targets branch from dc0a2e2 to 821e988 Compare September 29, 2025 07:53

sirpaleet commented Sep 29, 2025

View reviewed changes

hack/e2e-prerequisites.sh Outdated

@@ -0,0 +1,55 @@

#!/usr/bin/env bash

Copy link

Member Author

sirpaleet Sep 29, 2025

Choose a reason for hiding this comment

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

Added this to script

tuminoid reviewed Sep 29, 2025

View reviewed changes

sirpaleet force-pushed the siiri/makefile-e2e-targets branch from 821e988 to 708d8ae Compare September 29, 2025 12:50

sirpaleet changed the title ~~🌱 Add e2e-prerequisites and ease running e2e locally~~ 🌱 Fix hack/ci-e2e.sh script installations Sep 29, 2025

sirpaleet marked this pull request as ready for review September 29, 2025 12:58

metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 29, 2025

metal3-io-bot requested review from s3rj1k and zhouhao3 September 29, 2025 12:58

sirpaleet force-pushed the siiri/makefile-e2e-targets branch 2 times, most recently from b065e26 to 255f03e Compare September 29, 2025 13:28

sirpaleet commented Sep 29, 2025

View reviewed changes

hack/e2e/ensure_docker.sh Show resolved Hide resolved

sirpaleet force-pushed the siiri/makefile-e2e-targets branch from 255f03e to 6d4df3b Compare September 29, 2025 13:40

sirpaleet mentioned this pull request Sep 29, 2025

Bump ensured kubectl minimum version in e2e #2708

Closed

sirpaleet force-pushed the siiri/makefile-e2e-targets branch from 6d4df3b to a3dd101 Compare September 30, 2025 06:32

sirpaleet commented Sep 30, 2025

View reviewed changes

sirpaleet force-pushed the siiri/makefile-e2e-targets branch 2 times, most recently from fe55660 to 271fdee Compare September 30, 2025 07:02

tuminoid reviewed Sep 30, 2025

View reviewed changes

sirpaleet force-pushed the siiri/makefile-e2e-targets branch 2 times, most recently from 18d7a3e to 5a1774b Compare September 30, 2025 07:43

Fix hack/ci-e2e.sh script installations

4939f30

Edit Makefile to add run-e2e-script, fix installations, and add documentation. Signed-off-by: Siiri Kemppainen <[email protected]>

sirpaleet force-pushed the siiri/makefile-e2e-targets branch from 5a1774b to 4939f30 Compare September 30, 2025 08:27

sirpaleet commented Sep 30, 2025

View reviewed changes

		@@ -0,0 +1,32 @@
		#!/usr/bin/env bash
		# shellcheck disable=SC1091

🌱 Fix hack/ci-e2e.sh script installations #2699

Are you sure you want to change the base?

🌱 Fix hack/ci-e2e.sh script installations #2699

Uh oh!

Conversation

sirpaleet commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

metal3-io-bot commented Sep 25, 2025

Uh oh!

metal3-io-bot commented Sep 25, 2025

Uh oh!

sirpaleet commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sirpaleet Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

sirpaleet commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sirpaleet commented Sep 29, 2025

Uh oh!

sirpaleet commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sirpaleet Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tuminoid Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

sirpaleet Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

tuminoid Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

sirpaleet Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

sirpaleet commented Sep 30, 2025

Uh oh!

sirpaleet Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sirpaleet commented Sep 25, 2025 •

edited

Loading

sirpaleet commented Sep 29, 2025 •

edited

Loading