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.

Sign up for GitHub

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

Jump to bottom

Metal3 Scalability with Fake Nodes experiment #88

Merged

metal3-io-bot merged 1 commit into main from mquhuy/multiple-ironic-study

Nov 5, 2024

Member

mquhuy commented Jul 17, 2023 •

edited

Loading

This study experiments provisioning multiple workload clusters with CAPI + Metal3 with only fake nodes. It takes into account Fake IPA, FKAS, and Ironic with multiple conductors (with shared endpoints and database).

metal3-io-bot added the size/XXL label

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 3 times, most recently from 105b499 to dde9301 Compare

July 17, 2023 11:57

kashifest assigned mboukhalfa, kashifest, Rozzii, Sunnatillo and smoshiur1237

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 6 times, most recently from 9246c3b to b17e6b4 Compare

August 11, 2023 13:24

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 2 times, most recently from 50bd933 to 2cf05d6 Compare

September 19, 2023 13:34

Contributor

Rozzii commented Sep 20, 2023

@mquhuy please ping me in comment after this passes the static code checks.

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 3 times, most recently from eb18735 to 644cb67 Compare

September 27, 2023 09:19

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 3 times, most recently from ea421e3 to dbc9136 Compare

October 19, 2023 13:04

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 4 times, most recently from 4570c88 to 570b712 Compare

November 3, 2023 07:32

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 2 times, most recently from 61d16bb to 59761fb Compare

November 9, 2023 08:54

Contributor

kashifest commented Oct 30, 2024

wow this is big, will check it but it would take some time

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 3 times, most recently from 002de29 to 697038f Compare

October 30, 2024 12:19

Rozzii reviewed

View reviewed changes

Contributor

Rozzii left a comment

/approve

Thanks for doing the experiment and documenting it !

Member

metal3-io-bot commented Oct 31, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Rozzii

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Rozzii]

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

metal3-io-bot added the approved label

tuminoid reviewed

View reviewed changes

Member

tuminoid left a comment

Plenty of nits, mostly repeating few all over. Some suggestions as well.

Thanks for putting it together!

Support/Multitenancy/Multiple-Ironic-conductors/README.md Outdated

+              This directory stores the code and configuration for the __Metal3 Scalability with
+              Fake Nodes and Multiple Ironics__. Its purpose is to have a way of testing
+              [Metal3](https://metal3.io) provisioning on multiple nodes, without needing

Member

tuminoid Oct 31, 2024

Suggested change

      
            [Metal3](https://metal3.io) provisioning on multiple nodes, without needing
          
            [Metal3](https://metal3.io/) provisioning on multiple nodes, without needing

Support/Multitenancy/Multiple-Ironic-conductors/README.md Outdated

+              This directory stores the code and configuration for the __Metal3 Scalability with
+              Fake Nodes and Multiple Ironics__. Its purpose is to have a way of testing
+              [Metal3](https://metal3.io) provisioning on multiple nodes, without needing
+              too much hardware resource.

Member

tuminoid Oct 31, 2024

Suggested change

      
            too much hardware resource.
          
            too much hardware resources.

Support/Multitenancy/Multiple-Ironic-conductors/README.md Outdated


		## Quick Start

		For a quick start, here are the steps you need:

Member

tuminoid Oct 31, 2024

Suggested change

      
            For a quick start, here are the steps you need:
          
            For a quick start:

Support/Multitenancy/Multiple-Ironic-conductors/README.md Outdated

+              For a quick start, here are the steps you need:
+              - Install some additional tools, add libvirt network
+              (named `baremetal`) by using `./vm-setup.sh`.

Member

tuminoid Oct 31, 2024

At this point, I'm already confused about the usage of vm-setup.sh as its mentioned two times, doing different things, with no arguments.

Support/Multitenancy/Multiple-Ironic-conductors/README.md Outdated

+              For easy comparison, here is the spec of the machine we used to provision 1000
+              single-node clusters:
+              - CPUs: 20c

Member

tuminoid Oct 31, 2024

Suggested change

      
            - CPUs: 20c
          
            - CPUs: 20 cores

Support/Multitenancy/Multiple-Ironic-conductors/start_containers.sh Outdated

+              API_URL="https://192.168.222.100:6385"
+              CALLBACK_URL="https://192.168.222.100:6385/v1/continue_inspection"
+              rm -rf "$SUSHY_CONF_DIR"

Member

tuminoid Oct 31, 2024

Please fix all the brace issues in this file.

Support/Multitenancy/Multiple-Ironic-conductors/start_image_server.sh Outdated

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

		set -x

Member

tuminoid Oct 31, 2024

Suggested change

      
            set -x
          
            set -eux

Support/Multitenancy/Multiple-Ironic-conductors/vm-setup.sh Outdated

		@@ -0,0 +1,58 @@
		#!/bin/bash

Member

tuminoid Oct 31, 2024

Suggested change

      
            #!/bin/bash
          
            #!/usr/bin/env bash

Support/Multitenancy/Multiple-Ironic-conductors/vm-setup.sh Outdated


		sudo systemctl enable --now libvirtd

		if [[ ! -f $(which minikube) ]]; then

Member

tuminoid Oct 31, 2024

Suggested change

      
            if [[ ! -f $(which minikube) ]]; then
          
            if ! command -v minikube &>/dev/null; then

Support/Multitenancy/Multiple-Ironic-conductors/vm-setup.sh Outdated

+                rm minikube-linux-amd64
+              fi
+              if [[ ! -f $(which kubectl) ]]; then

Member

tuminoid Oct 31, 2024

Same for all tools

mquhuy force-pushed the mquhuy/multiple-ironic-study branch from 697038f to 7693d3d Compare

October 31, 2024 07:59

Member Author

mquhuy commented Oct 31, 2024

Plenty of nits, mostly repeating few all over. Some suggestions as well.

Thanks for putting it together!

Thank you for the review! I've fixed the issues now.

mquhuy force-pushed the mquhuy/multiple-ironic-study branch 2 times, most recently from e2c69d9 to 6056aac Compare

October 31, 2024 08:02

mquhuy requested a review from tuminoid

October 31, 2024 08:07

mquhuy mentioned this pull request

📖 Add blog post for Fake Node Scalability on Metal3 metal3-io/metal3-io.github.io#500

Merged

lentzi90 reviewed

View reviewed changes

Member

lentzi90 left a comment

Nice job on putting all of this together!
I added some small nits below. Not sure if it is worth fixing.
One thing I'm curious about is the use of helm. Why create a helm chart when almost everything is hard coded in the templates? 🤔
Even the configmap content and container images are static in there, so why not just use a plain manifest and get rid of the extra dependency? 😅

Anyway, I don't want to block this. It looks good enough to merge
/lgtm

Support/Multitenancy/Scalability-with-Fake-Nodes/bmo-config/kustomization.yaml Outdated Show resolved Hide resolved

Support/Multitenancy/Scalability-with-Fake-Nodes/ironic/Chart.yaml Outdated Show resolved Hide resolved

.yamllint.yaml Show resolved Hide resolved

metal3-io-bot assigned lentzi90

metal3-io-bot added the lgtm label

Member

lentzi90 commented Nov 5, 2024

/hold
if you want to change anything. Otherwise remove

metal3-io-bot added the do-not-merge/hold label

Member Author

mquhuy commented Nov 5, 2024

Nice job on putting all of this together! I added some small nits below. Not sure if it is worth fixing. One thing I'm curious about is the use of helm. Why create a helm chart when almost everything is hard coded in the templates? 🤔 Even the configmap content and container images are static in there, so why not just use a plain manifest and get rid of the extra dependency? 😅

Anyway, I don't want to block this. It looks good enough to merge /lgtm

Thanks Lennart. What I want to achieve here is to have a custom number of ironic conductors, each with its own configmap, and provisioning_ip. Go template allows us to do so, but I haven't found a way to achieve the same result in kustomize. Perhaps ironic-standalone-operator will allow that.

Member

lentzi90 commented Nov 5, 2024

Thanks Lennart. What I want to achieve here is to have a custom number of ironic conductors, each with its own configmap, and provisioning_ip. Go template allows us to do so, but I haven't found a way to achieve the same result in kustomize. Perhaps ironic-standalone-operator will allow that.

Fair enough, if it works it works. 🙂
I would perhaps have chosen to do it with just bash or python that was used elsewhere, to keep the number of dependencies down.


          Add scalability setup with fake nodes using Fake IPA and FKAS

4578a4a

Signed-off-by: Huy Mai <[email protected]>

mquhuy force-pushed the mquhuy/multiple-ironic-study branch from 6056aac to 4578a4a Compare

November 5, 2024 10:15

metal3-io-bot removed the lgtm label

Member Author

mquhuy commented Nov 5, 2024

Fair enough, if it works it works. 🙂 I would perhaps have chosen to do it with just bash or python that was used elsewhere, to keep the number of dependencies down.

Thanks. Change coming

lentzi90 reviewed

View reviewed changes

Member

lentzi90 left a comment

/lgtm
/hold cancel

metal3-io-bot added lgtm and removed do-not-merge/hold labels

metal3-io-bot merged commit 23e88a0 into main

4 checks passed

metal3-io-bot deleted the mquhuy/multiple-ironic-study branch

November 5, 2024 11:03

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

lentzi90 lentzi90 left review comments

Rozzii Rozzii left review comments

adilGhaffarDev Awaiting requested review from adilGhaffarDev

kashifest Awaiting requested review from kashifest

tuminoid Awaiting requested review from tuminoid

Labels

approved lgtm size/XXL