Ironic Standalone Operator #461

dtantsur · 2024-07-25T15:29:46Z

This design proposal discusses ironic-standalone-operator: provides a
motivation for its creation, describes its goals and outlines the
current design and features, as well as the plans for the nearest
future.

Signed-off-by: Dmitry Tantsur [email protected]

metal3-io-bot · 2024-07-25T15:29:49Z

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

This design proposal discusses ironic-standalone-operator: provides a motivation for its creation, describes its goals and outlines the current design and features, as well as the plans for the nearest future. Signed-off-by: Dmitry Tantsur <[email protected]>

elfosardo · 2024-08-05T08:32:27Z

design/ironic-standalone-operator.md

+
+## Motivation
+
+Ironic is not trivial to install and operate. Also we provide


"Even if we provide" ?

Also-vs-Although 🤦🏽 Will fix if I need to update.

MahnoorAsghar · 2024-08-16T10:03:56Z

design/ironic-standalone-operator.md

+- `credentialsRef` - a reference to a secret with credentials (generated if
+ missing)
+- `databaseRef` - a reference to an `IronicDatabase` object (if needed)
+- `distributed` - a boolean flag that enables the HA architecture


Why not just call this field highlyAvailable or highAvailability

That's a good question. I guess distributed is more precise technically, while highAvailability may be more user-friendly. I'd like to hear more opinions on the topic.

I would vote for highAvailability or haEnabled

MahnoorAsghar · 2024-08-16T10:06:18Z

design/ironic-standalone-operator.md

+
+- `apiPort`, `imageServerPort`, `imageServerTLSPort` allow overriding listening
+ ports for the services
+- `bindInterface` - a boolean flag that makes Ironic listen on only the


It may sound too simplistic, but can we call this provisioningInterfaceOnly? )bindInterface can sound like another kind of interface.)

I concur but see below about the word "provisioning".

MahnoorAsghar · 2024-08-16T10:06:53Z

design/ironic-standalone-operator.md

+- `dhcp` - another nested structure with DHCP parameters (see below)
+- `externalIP` - IP through which nodes deployed over virtual media access
+ Ironic and HTTPD
+- `interface`, `ipAddress`, `macAddresses` - various ways to specify the


Similarly, how does provisioningInterface, provisioningIPAddress and provisioningMACAddress sound?

There is a potential for confusion here. We call "provisioning network" a dedicated network for the PXE and other provisioning traffic. Ironic can work without it, and the variables work regardless of whether you have a "provisioning network".

Maybe I'm overthinking it? Would like more opinions here as well.

If we do decide to change this, I'd rather rename the whole "networking" sub-field into something like "provisioningNetwork" to avoid duplicate prefixes.

But the rest of the sub-fields seem unrelated to the provisioning network (like dhcp and external ip)?
How would the interface, ipAddress and macAddresses fields work if there is no provisioning interface?
(I am not too familiar with ironic's networking, so just trying to understand the expected behaviour)

DHCP is very-very much related to the provisioning network: it's DHCP on the provisioning network. ExternalIP is not directly related indeed, maybe we'd need to move it out then.

How would the interface, ipAddress and macAddresses fields work if there is no provisioning interface?

There must be a provisioning interface, but it can be on some existing network rather than an isolated provisioning network. Confusing, I know. This is why I'm pondering our usage of "provisioning" here.

MahnoorAsghar · 2024-08-16T10:55:09Z

design/ironic-standalone-operator.md

+
+After each full reconciliation, the operator will store the version of Ironic
+it has just installed in the `status`. In the future, this will allow to apply
+any logic on upgrade. However, we will try to keep ironic-image self-upgrading


'However, we will try to keep ironic-image self-upgrading for the sake of users that do not use ironic-standalone-operator.' What do you mean?

I mean, if you just use ironic-image, and you upgrade it by replacing the container, we will try to support this case still. Any ideas on how to phrase it better?

You mean that for example, if someone was running ironic-standalone-operator v1.0 (with ironic-image v1.0), and just replaced their ironic-image with v1.1, ironic-standalone-operator will continue to work?
If that's what you mean, then 'for the sake of users that do not use ironic-standalone-operator.' can be confusing..

Ah, I see the source of confusion. What I wanted to say was: users of ironic-image without the operator will be able to do it still. I'll work on a different way to phrase it.

hardys · 2024-08-21T17:39:49Z

design/ironic-standalone-operator.md

+ avoid running it 3+ times in parallel. Maybe it will take a form of a *Job*.
+- BMO will need to be updated to handle more cases of unexpected provision
+ state. E.g. what needs to be done when the BMH is *inspecting* but the node
+ is found in a completely wrong state like `active` or `clean wait`.


There is an additional complexity to consider if we're going to support mariadb combined with persistent volumes, when the mariadb version changes you either have to ensure clean shutdown of the container or enable some kind of backup/restore workflow due to the mariadb checks around versioning for the redo log - see here for example.

This can likely be resolved with with pod lifecycle hooks, or perhaps some kind of job pre/post upgrade but it's additional complexity which I don't think we currently consider in the community deployment examples.

Very good point, thank you!

Rozzii · 2024-09-25T08:09:52Z

design/ironic-standalone-operator.md

+
+Q: Why using *DaemonSets* if *StatefulSets* provide us an easier way to address
+separate Ironic instances? A: While we're relying on host networking, making
+several Ironic instances co-exist on the same Kubernetes node is too complex.


From Kubernetes HA perspective it is also more reliable to have the ironic pods present on separate control plane nodes. In other words clustering all Ironic pods on e.g. 1 controlplane node wouldn't be HA anyways as that node could be lost.

Rozzii

/approve

Rozzii

/lgtm
From my perspective it is fine given that you will address the comments.

Rozzii · 2024-09-25T08:28:58Z

/hold
Just holding to avoid someone putting approve on it while my lgtm is active and merging it

kashifest

Thanks a lot @dtantsur few questions/nits inline

kashifest · 2024-09-25T06:27:48Z

design/ironic-standalone-operator.md

+#### HA architecture
+
+The *non-HA* architecture is the architecture that Metal3 uses now. All Ironic
+components are run in a *deployment*.


Suggested change

components are run in a *deployment*.

components are run in a single *deployment*.

kashifest · 2024-09-25T06:29:26Z

design/ironic-standalone-operator.md

+
+When the HA architecture is enabled via a flag on the `Ironic` resource, all
+Ironic components (except for MariaDB and dnsmasq) will be installed in a
+*DaemonSet* instead of a *Deployment*.


Can we explain the reasoning here ?

kashifest · 2024-09-25T06:55:16Z

design/ironic-standalone-operator.md

+
+## Design Details
+
+This proposal adds a new project under the Metal3 umbrella:


is it a project or a repository?

kashifest · 2024-09-25T13:05:28Z

design/ironic-standalone-operator.md

+its private key certificate and sign the certificate with this CA. The CA will
+be trusted **only** for the RPC purpose, removing the possibility of abuse.
+To reduce the number of code paths, this process will happen unconditionally,
+even when TLS for Ironic itself is not enabled.


Just for my understanding what prevents us from enabling TLS unconditionally for ironic itself?

kashifest · 2024-09-25T13:09:10Z

design/ironic-standalone-operator.md

+- `credentialsRef` - a reference to a secret with credentials (generated if
+ missing)
+- `databaseRef` - a reference to an `IronicDatabase` object (if needed)
+- `distributed` - a boolean flag that enables the HA architecture


I would vote for highAvailability or haEnabled

kashifest · 2024-09-25T13:15:20Z

I can approve it already and hold can be taken back when the reviews are addressed
/approve

metal3-io-bot · 2024-09-25T13:15:26Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kashifest, 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~~ [kashifest]
~~design/OWNERS~~ [Rozzii,kashifest]

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 do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 25, 2024

metal3-io-bot requested review from kashifest and Rozzii July 25, 2024 15:29

metal3-io-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 25, 2024

dtantsur force-pushed the ironic-standalone-operator branch 4 times, most recently from 27ec8ca to 04f201c Compare August 1, 2024 16:06

dtantsur marked this pull request as ready for review August 1, 2024 16:07

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 Aug 1, 2024

metal3-io-bot requested a review from zaneb August 1, 2024 16:07

dtantsur force-pushed the ironic-standalone-operator branch from 04f201c to 23135d5 Compare August 1, 2024 16:11

Ironic Standalone Operator

7be12d1

This design proposal discusses ironic-standalone-operator: provides a motivation for its creation, describes its goals and outlines the current design and features, as well as the plans for the nearest future. Signed-off-by: Dmitry Tantsur <[email protected]>

dtantsur force-pushed the ironic-standalone-operator branch from 23135d5 to 7be12d1 Compare August 1, 2024 16:16

elfosardo reviewed Aug 5, 2024

View reviewed changes

MahnoorAsghar reviewed Aug 16, 2024

View reviewed changes

hardys reviewed Aug 21, 2024

View reviewed changes

Rozzii reviewed Sep 25, 2024

View reviewed changes

metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Sep 25, 2024

metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 25, 2024

kashifest reviewed Sep 25, 2024

View reviewed changes

metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ironic Standalone Operator #461

Ironic Standalone Operator #461

dtantsur commented Jul 25, 2024

metal3-io-bot commented Jul 25, 2024

elfosardo Aug 5, 2024

dtantsur Aug 5, 2024

MahnoorAsghar Aug 16, 2024

dtantsur Aug 21, 2024

kashifest Sep 25, 2024

MahnoorAsghar Aug 16, 2024

dtantsur Aug 21, 2024

MahnoorAsghar Aug 16, 2024

dtantsur Aug 21, 2024

dtantsur Aug 21, 2024

MahnoorAsghar Aug 21, 2024

dtantsur Aug 22, 2024

MahnoorAsghar Aug 16, 2024

dtantsur Aug 21, 2024

MahnoorAsghar Aug 21, 2024

dtantsur Aug 22, 2024

hardys Aug 21, 2024

dtantsur Aug 22, 2024

Rozzii Sep 25, 2024

Rozzii left a comment

Rozzii left a comment •

edited

Loading

Rozzii commented Sep 25, 2024

kashifest left a comment

kashifest Sep 25, 2024

kashifest Sep 25, 2024

kashifest Sep 25, 2024

kashifest Sep 25, 2024

kashifest Sep 25, 2024

kashifest commented Sep 25, 2024

metal3-io-bot commented Sep 25, 2024


		## Motivation

		Ironic is not trivial to install and operate. Also we provide

	components are run in a deployment.
	components are run in a single deployment.


		## Design Details

		This proposal adds a new project under the Metal3 umbrella:

Ironic Standalone Operator #461

Are you sure you want to change the base?

Ironic Standalone Operator #461

Conversation

dtantsur commented Jul 25, 2024

metal3-io-bot commented Jul 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rozzii left a comment

Choose a reason for hiding this comment

Rozzii left a comment • edited Loading

Choose a reason for hiding this comment

Rozzii commented Sep 25, 2024

kashifest left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kashifest commented Sep 25, 2024

metal3-io-bot commented Sep 25, 2024

Rozzii left a comment •

edited

Loading