feat: set Flavor on enroll #481

skrobul · 2024-11-14T18:14:53Z

This PR enhances the "enroll-server" workflow with a capability of setting the baremetal node's flavour. We have previously attempted to do this through Ironic inspection hooks, but found that running Ironic's inspection has some undesired side effects (i.e. modifying the baremetal node ports for no reason). As a result, we cannot match the node's hardware specs to a given flavor during the inspection.

What we do instead is retrieve all the required inputs through Redfish interface as part of the 'enroll' workflow, then feed this information to the flavor matching code. The resulting flavor is set immediately when the Ironic node is created or updated.

In order to facilitate this, some additional changes were required:

the flavor matching code has been extracted into separate, independent module in case we want to later move this functionality back to Ironic. For now this is not possible until inspection issues are addressed upstream.
flavor-sync has been moved to a separate Deployment backed by a PVC that is shared with instances of the enroll-server workflow.
when the flavors are matched, we now strip first chunk from the name (i.e. 'prod')

Closes https://rackspace.atlassian.net/browse/PUC-591

skrobul · 2024-11-18T14:23:51Z

side note: I really don't like how the pre-commit/pyright setup is done when it comes to installing a "local" package, but this was the only way I could get it to work. Open to any suggestions how to do this better.
The pyright-python page suggests that we should either:

tell pre-commit to install those dependencies, which is great for published modules, but it intentionally does not work for modules/packages included in the source code.
explicitly tell pyright which virtual environment to use by updating your pyright configuration file - this may work, but I was hesitant to manipulate this because each user will have different configuration settings with very different paths to their virtualenvs. We may be able to inject ephemeral pyrightconfig.json into a workspace, but that would also require us to create a virtualenv earlier and install our packages there. Feels like a hack, hence looking for better options.

stevekeay · 2024-11-18T15:29:45Z

python/understack-workflows/understack_workflows/bmc_disk.py

+logger = setup_logger(__name__)
+
+
+REDFISH_DISKS_PATH = "/redfish/v1/Systems/System.Embedded.1/Storage/RAID.SL.1-1"


It seems like this would be different on boxes with a different raid controller, but I think this is okay given our strictly heterogeneous target hardware models right now.

Yeah, that can be different even when the disks are connected different way (some of the Dell's examples list RAID.Mezzanine.1-1 for example. As we discussed in the other PR, I went with hardcoded paths that we can differentiate on later.

edit: note for future me - SL stands for "slimline cable", not "slot"

python/understack-workflows/understack_workflows/bmc_disk.py

stevekeay · 2024-11-18T16:01:47Z

python/understack-flavor-matcher/tests/test_flavor_spec.py

+        cpu="Non-Existent CPU Model",
+        disk_gb=500,
+        model="Dell XPS1319",
+    )
    assert all(flavor.score_machine(machine) for flavor in flavors) == 0


Are these asserts doing what it looks like they should be doing? all() answers a boolean, which == compares as a truthy/falsey, so I think this is asserting that at least ONE of the scores is zero, which is all mighty confusing.

You are absolutely right - they were indeed matching incorrectly - it was meant to be

assert all(flavor.score_machine(machine) == 0 for flavor in flavors)

addressed in 93a3caa

This will allow us to reuse the same code betwen the custom Argo workflows for enrollment as well as Ironic hooks/inspection interfaces. Eventually when we fully migrate to using ironic hooks, this can be inlined again.

The responsibility of providing that volume with fresh data is delegated to the undercloud-deploy repo as it may be different for each deployments.

Up till now we ignored the 'chassis model' because there was low risk of the overlap between the flavors, but turns out this is actually needed to distinguish between for example gp1.small and gp2.small.

There are no any tests there anymore.

skrobul · 2024-11-18T17:09:43Z

Merge when the https://github.com/RSS-Engineering/undercloud-deploy/pull/222/ is reviewed and approved

skrobul force-pushed the flavor-on-enroll branch 15 times, most recently from d4cff9f to f79520c Compare November 18, 2024 14:01

skrobul changed the title ~~WIP: set Flavor on enroll~~ feat: set Flavor on enroll Nov 18, 2024

skrobul marked this pull request as ready for review November 18, 2024 14:12

skrobul requested a review from a team November 18, 2024 14:13

stevekeay approved these changes Nov 18, 2024

View reviewed changes

skrobul force-pushed the flavor-on-enroll branch from 0fb3cd1 to 93a3caa Compare November 18, 2024 16:50

skrobul added 2 commits November 18, 2024 16:51

enroll: update docstring for flavor matching

1b4d8dd

enroll/bmc: collect RAM and CPU info

dffc1c4

skrobul force-pushed the flavor-on-enroll branch from 93a3caa to 262ab1f Compare November 18, 2024 16:56

skrobul added 6 commits November 18, 2024 17:00

enroll: add support for obtaining disk info

3388a76

extract understack-flavor-matcher module

1e34c7e

This will allow us to reuse the same code betwen the custom Argo workflows for enrollment as well as Ironic hooks/inspection interfaces. Eventually when we fully migrate to using ironic hooks, this can be inlined again.

enroll: wire in flavor matching code

2e77a18

understack-workflows: install understack-flavor-matcher dependency

8b9de6e

enroll-server: mount the understack-flavors PVC

5bf2d43

The responsibility of providing that volume with fresh data is delegated to the undercloud-deploy repo as it may be different for each deployments.

flavors: strip the prefix

0fc0b15

skrobul added 3 commits November 18, 2024 17:00

flavors: match on chassis model

6f6a96d

Up till now we ignored the 'chassis model' because there was low risk of the overlap between the flavors, but turns out this is actually needed to distinguish between for example gp1.small and gp2.small.

fix: pre-commit setup for pyright

28ceb4e

CI: don't run ironic tests

b6565af

There are no any tests there anymore.

skrobul force-pushed the flavor-on-enroll branch from 262ab1f to b6565af Compare November 18, 2024 17:00

skrobul added this pull request to the merge queue Nov 18, 2024

Merged via the queue into main with commit b5a7302 Nov 18, 2024
24 checks passed

skrobul deleted the flavor-on-enroll branch November 18, 2024 17:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: set Flavor on enroll #481

feat: set Flavor on enroll #481

skrobul commented Nov 14, 2024 •

edited

Loading

skrobul commented Nov 18, 2024

stevekeay Nov 18, 2024

skrobul Nov 18, 2024 •

edited

Loading

stevekeay Nov 18, 2024

skrobul Nov 18, 2024 •

edited

Loading

skrobul commented Nov 18, 2024

		logger = setup_logger(__name__)


		REDFISH_DISKS_PATH = "/redfish/v1/Systems/System.Embedded.1/Storage/RAID.SL.1-1"

feat: set Flavor on enroll #481

feat: set Flavor on enroll #481

Conversation

skrobul commented Nov 14, 2024 • edited Loading

skrobul commented Nov 18, 2024

stevekeay Nov 18, 2024

Choose a reason for hiding this comment

skrobul Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

stevekeay Nov 18, 2024

Choose a reason for hiding this comment

skrobul Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

skrobul commented Nov 18, 2024

skrobul commented Nov 14, 2024 •

edited

Loading

skrobul Nov 18, 2024 •

edited

Loading

skrobul Nov 18, 2024 •

edited

Loading