Skip to content

Conversation

@peppi-lotta
Copy link
Member

No description provided.

@metal3-io-bot
Copy link
Collaborator

[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 sunnatillo for approval. For more information see the Code Review Process.

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

Needs approval from an approver in each of these files:

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

@peppi-lotta peppi-lotta marked this pull request as draft November 6, 2025 08:22
@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 6, 2025
@peppi-lotta peppi-lotta changed the title Add test for quick start guide 🌱 WIP: Add test for quick start guide Nov 6, 2025
Copy link
Member

@nuhakala nuhakala left a comment

Choose a reason for hiding this comment

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

When I tried to deploy this, bmo was not able to connect to ironic and inspection did not even start. there was error microversions not supported by endpoint in bmo log, which I have really no clue what it means. Something off with network.

I tried to enable tls if that would fix it, but it did not. I still got the same error. Also, I suspect that the bmo kustomization is not configured properly for tls, the bmo deployment did not have reference to ironic certs. I think this patch should be applied if tls were to be enabled.

@@ -0,0 +1,3 @@
DEPLOY_KERNEL_URL=http://192.168.222.1:6180/images/ironic-python-agent.kernel
DEPLOY_RAMDISK_URL=http://192.168.222.1:6180/images/ironic-python-agent.initramfs
IRONIC_ENDPOINT=https://192.168.222.2:6385/v1/
Copy link
Member

Choose a reason for hiding this comment

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

https but you have not configured tls for ironic

networkCIDR: "192.168.222.0/24"
interface: "eth0"
ipAddress: "192.168.222.2"
ipAddressManager: "keepalived"
Copy link
Member

Choose a reason for hiding this comment

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

You need to configure the ironic username/password here: apiCredentialsName: ironic-credentials or whatever the secret name is

Comment on lines 11 to 17
secretGenerator:
- name: ironic-auth
behavior: create
files:
- username=ironic-username
- password=ironic-password
type: Opaque
Copy link
Member

Choose a reason for hiding this comment

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

I think you can reuse the ironic-credentials defined in bmo kustomization. If you want create own credentials here, you need to make sure they are in baremetal-operator-system namespace as ironic will be there also.

Comment on lines 4 to 11
# Define and start the baremetal-e2e network
virsh -c qemu:///system net-define net.xml
virsh -c qemu:///system net-start baremetal-e2e

if ! sudo virsh net-list --all | grep baremetal-e2e; then
virsh -c qemu:///system net-define "${REPO_ROOT}/hack/e2e/net.xml"
virsh -c qemu:///system net-start baremetal-e2e
fi
Copy link
Member

@nuhakala nuhakala Nov 6, 2025

Choose a reason for hiding this comment

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

is REPO_ROOT even defined? The if clause is used the prevent creating the network twice, but as your cleanup script deletes the whole network, maybe it is not needed at all. That is, the whole if-clause can be deleted.

virsh -c qemu:///system net-undefine baremetal-e2e

export QUICK_START_BASE=${QUICK_START_BASE:="$(dirname -- "$(readlink -f "${BASH_SOURCE[0]}")")"}
rm -rf "${QUICK_START_BASE}/bmh-vm-01.xml" No newline at end of file
Copy link
Member

@nuhakala nuhakala Nov 6, 2025

Choose a reason for hiding this comment

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

Maybe you want to add cleanup for the veth pair and docker network also here, as we define those explicitly. Also the routing rules could be deleted.

ip link del metalend # delete both ends of veth pair
docker network rm kind
iptables -D FORWARD 1
iptables -D FORWARD 1 # twice because we want to remove the first rule twice

Comment on lines 8 to 9
resources:
- https://github.com/metal3-io/baremetal-operator/config/use-irso?ref=main
Copy link
Member

@nuhakala nuhakala Nov 6, 2025

Choose a reason for hiding this comment

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

I don't know if this manifest is the reason, but when I deployed this on VM, the variables in ironic.env were not defined. Instead, there was ironic named configmap which probably came from bmo repository where this manifest is located.

We might need to override the configmap so that it uses the ironic.env we provide by adding this to the bmo kustomization:

configMapGenerator:
- name: ironic
  behavior: replace
  envs:
  - ironic.env

Signed-off-by: peppi-lotta <[email protected]>
@peppi-lotta peppi-lotta force-pushed the peppi-lotta/add-test-for-quick-start-guide branch from 4b4cc3a to 7ce8153 Compare November 25, 2025 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants