feat: add resources and controllers for bare metal infra provider #707

utkuozdemir · 2024-10-25T10:56:58Z

Introduce the concept of "static" infra providers, e.g., bare-metal infra provider, which manage a static set of machines contrary to the "regular" infra providers.

Add the following resources:

infra.Machine: similar to MachineRequest, lives in the infra-provider namespace, serving as the input of the owning static provider. It is created in the MachineController if there is a SideroLink connection with the static provider ID. Regular flow of Machine creation is blocked, until this infra.Machine is accepted.
infra.MachineStatus: similar to MachineRequestStatus, lives in the infra-provider namespace, serving as the output of the owning static provider. Its lifecycle must be bound to the corresponding infra.Machine.
infra.MachineState: a resource that is supposed to be shared by Omni and bare-metal provider bi-directionally - they both can read from and write to it. It is currently used to mark the machine as installed when we observe an installation (through SequenceEvents in the event sink), and to mark it as non-installed after we wipe it in the provider.
omni.InfraMachineConfig: a user-managed resource to mark the infra.Machines as accepted or set their desired power state. The acceptance information is then propagated to the infra.Machine resource. A machine which was already accepted cannot be unaccepted (checked by a validation), and this resource can only be removed when the siderolink.Link for the matching infra machine is removed.

internal/backend/runtime/omni/controllers/omni/infra_machine.go

internal/backend/runtime/omni/controllers/omni/machine.go

internal/backend/runtime/omni/controllers/omni/infra_machine.go

client/pkg/omni/resources/infra/machine_status.go

client/pkg/omni/resources/infra/machine.go

client/api/omni/specs/omni.proto

utkuozdemir · 2024-11-25T22:22:21Z

internal/backend/runtime/omni/controllers/omni/machine.go

+	if _, isStaticProvider := providerStatus.Metadata().Labels().Get(omni.LabelIsStaticInfraProvider); isStaticProvider { // static infra provider flow, e.g., bare-metal
+		return h.handleStaticInfraProvider(ctx, r, machine)
+	}
+
+	// provisioning (regular) infra provider flow, e.g., kubevirt
+	return h.handleProvisioningInfraProvider(ctx, r, link, machine)


Other than this branching here, the logic in this file is totally unchanged - just reworked for readability.

utkuozdemir · 2024-11-25T22:24:54Z

internal/backend/runtime/omni/controllers/omni/machine_status.go

@@ -249,7 +249,7 @@ func (ctrl *MachineStatusController) reconcileRunning(ctx context.Context, r con

 		spec.Connected = connected

-		helpers.CopyLabels(machine, m, omni.LabelMachineRequest, omni.LabelMachineRequestSet, omni.LabelNoManualAllocation)
+		helpers.CopyLabels(machine, m, omni.LabelMachineRequest, omni.LabelMachineRequestSet, omni.LabelNoManualAllocation, omni.LabelIsManagedByStaticInfraProvider)


Propagate this label from Machine to MachineStatus. We then check this label on MachineStatus in ClusterMachineConfigStatusController to find out if the machine is being prepared by the static infra provider (still not ready to be used, e.g., being wiped), and if it is, skip applying config to it.

utkuozdemir · 2024-11-25T22:38:24Z

internal/backend/runtime/omni/controllers/omni/machine.go

+// handleProvisioningInfraProvider checks if the machine managed by a provisioning infra provider has the machine request and machine request set labels set.
+//
+// It then matches the machine request set owner to the machine provision controller, and if it matches, sets the no manual allocation label on the machine.
+func (h *machineControllerHelper) handleProvisioningInfraProvider(ctx context.Context, r controller.Reader, link *siderolink.Link, machine *omni.Machine) error {


Better naming suggestions are welcome - my thought process here was: we have 2 distinct types of infra providers: "static", e.g., bare-metal, and "provisioning", e.g. kubevirt, aws etc.

utkuozdemir · 2024-11-25T22:40:14Z

internal/pkg/machinestatus/handler.go

For this to not be confusing: I simply this from machinestatus.Handler to be machineevent.Handler, as it now also handles SequenceEvents. GitHub couldn't figure out that it was moved and also modified afterwards.

utkuozdemir · 2024-11-25T22:40:44Z

internal/pkg/machineevent/handler.go

See: https://github.com/siderolabs/omni/pull/707/files#r1857434405

utkuozdemir · 2024-11-25T22:41:17Z

internal/pkg/machineevent/handler.go

+	case *machineapi.SequenceEvent:
+		return handler.handleSequenceEvent(ctx, event, machineID)


Added this (this case + the handleSequenceEvent function), otherwise no changes.

client/api/omni/specs/infra.proto

smira · 2024-11-26T16:48:27Z

go.mod

@@ -3,6 +3,9 @@ module github.com/siderolabs/omni
 go 1.23.3

 replace (
+	// todo: remove when merged
+	github.com/siderolabs/omni-infra-provider-bare-metal v0.0.0-20241014092637-c3e892b87244 => github.com/utkuozdemir/sidero-omni-infra-provider-bare-metal v0.0.0-20241021213554-cc93e594a0d2


is it okay that we create an import cycle, as the provider imports omni/client at the very least?

I know it's going to cause lots of fun when you do incompatible changes

That's a good point, and actually it was unused. The initial idea and the reason was to import the GRPC server API types that the provider serves, so that Omni could send commands to the machines (e.g. Reboot) over the reverse GRPC tunnel, Omni->Provider->Machine.

I initially did that, but then removed to keep the scope smaller in this PR.

So I now removed this import, and also decided to remove the whole reverse tunnel logic. I'll keep that in a separate Draft PR for now, as I'm having second thoughts on whether it is necessary at all. It would only be used for reboot for the time being, and imo it is perfectly doable over the resources at hand as well (similar to wipeID, something like rebootID).

So I'll discuss this with @Unix4ever when he is back :)

smira

I can't say that I understand all of changes, but I don't see anything that looks bad

Introduce the concept of "static" infra providers, e.g., bare-metal infra provider, which manage a static set of machines contrary to the "regular" infra providers. Add the following resources: - `infra.Machine`: similar to `MachineRequest`, lives in the `infra-provider` namespace, serving as the input of the owning static provider. It is created in the `MachineController` if there is a SideroLink connection with the static provider ID. Regular flow of `Machine` creation is blocked, until this `infra.Machine` is accepted. - `infra.MachineStatus`: similar to `MachineRequestStatus`, lives in the `infra-provider` namespace, serving as the output of the owning static provider. Its lifecycle must be bound to the corresponding `infra.Machine`. - `infra.MachineState`: a resource that is supposed to be shared by Omni and bare-metal provider bi-directionally - they both can read from and write to it. It is currently used to mark the machine as installed when we observe an installation (through `SequenceEvent`s in the event sink), and to mark it as non-installed after we wipe it in the provider. - `omni.InfraMachineConfig`: a user-managed resource to mark the `infra.Machine`s as accepted or set their desired power state. The acceptance information is then propagated to the `infra.Machine` resource. A machine which was already accepted cannot be unaccepted (checked by a validation), and this resource can only be removed when the `siderolink.Link` for the matching infra machine is removed. Signed-off-by: Utku Ozdemir <[email protected]>

utkuozdemir · 2024-11-26T23:31:39Z

/m

utkuozdemir force-pushed the bare-metal-provider-connection branch 3 times, most recently from 880d275 to dd2e6ad Compare October 29, 2024 21:35

utkuozdemir force-pushed the bare-metal-provider-connection branch 3 times, most recently from d5fb13e to a14b056 Compare November 8, 2024 23:52

utkuozdemir force-pushed the bare-metal-provider-connection branch 5 times, most recently from b4b296a to e02a76b Compare November 13, 2024 13:51

utkuozdemir changed the title ~~WIP: feat: bare metal stuff~~ feat: add resources and controllers for bare metal infra provider Nov 13, 2024

utkuozdemir marked this pull request as ready for review November 13, 2024 13:53

talos-bot added the status/ok-to-test label Nov 13, 2024

utkuozdemir commented Nov 13, 2024

View reviewed changes

internal/backend/runtime/omni/controllers/omni/infra_machine.go Outdated Show resolved Hide resolved

Unix4ever reviewed Nov 13, 2024

View reviewed changes

utkuozdemir force-pushed the bare-metal-provider-connection branch 5 times, most recently from c2bb7f8 to fd438ee Compare November 20, 2024 09:46

utkuozdemir force-pushed the bare-metal-provider-connection branch 6 times, most recently from c753e9a to 8a6f925 Compare November 25, 2024 22:14

utkuozdemir commented Nov 25, 2024

View reviewed changes

utkuozdemir force-pushed the bare-metal-provider-connection branch from 8a6f925 to d6063fc Compare November 25, 2024 22:34

utkuozdemir commented Nov 25, 2024

View reviewed changes

internal/pkg/machineevent/handler.go

Copy link

Member Author

utkuozdemir Nov 25, 2024

Choose a reason for hiding this comment

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

See: https://github.com/siderolabs/omni/pull/707/files#r1857434405

utkuozdemir commented Nov 25, 2024

View reviewed changes

smira reviewed Nov 26, 2024

View reviewed changes

client/api/omni/specs/infra.proto Show resolved Hide resolved

smira reviewed Nov 26, 2024

View reviewed changes

smira approved these changes Nov 26, 2024

View reviewed changes

utkuozdemir force-pushed the bare-metal-provider-connection branch from d6063fc to 5a26d4c Compare November 26, 2024 23:00

talos-bot added the status/ok-to-merge label Nov 26, 2024

talos-bot merged commit 5a26d4c into siderolabs:main Nov 26, 2024
22 checks passed

talos-bot removed status/ok-to-test status/ok-to-merge labels Nov 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add resources and controllers for bare metal infra provider #707

feat: add resources and controllers for bare metal infra provider #707

utkuozdemir commented Oct 25, 2024 •

edited

Loading

utkuozdemir Nov 25, 2024

utkuozdemir Nov 25, 2024

utkuozdemir Nov 25, 2024 •

edited

Loading

utkuozdemir Nov 25, 2024

utkuozdemir Nov 25, 2024

utkuozdemir Nov 25, 2024 •

edited

Loading

smira Nov 26, 2024

utkuozdemir Nov 26, 2024

smira left a comment

utkuozdemir commented Nov 26, 2024

		case *machineapi.SequenceEvent:
		return handler.handleSequenceEvent(ctx, event, machineID)

feat: add resources and controllers for bare metal infra provider #707

feat: add resources and controllers for bare metal infra provider #707

Conversation

utkuozdemir commented Oct 25, 2024 • edited Loading

utkuozdemir Nov 25, 2024

Choose a reason for hiding this comment

utkuozdemir Nov 25, 2024

Choose a reason for hiding this comment

utkuozdemir Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

utkuozdemir Nov 25, 2024

Choose a reason for hiding this comment

utkuozdemir Nov 25, 2024

Choose a reason for hiding this comment

utkuozdemir Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

smira Nov 26, 2024

Choose a reason for hiding this comment

utkuozdemir Nov 26, 2024

Choose a reason for hiding this comment

smira left a comment

Choose a reason for hiding this comment

utkuozdemir commented Nov 26, 2024

utkuozdemir commented Oct 25, 2024 •

edited

Loading

utkuozdemir Nov 25, 2024 •

edited

Loading

utkuozdemir Nov 25, 2024 •

edited

Loading