Skip to content

Conversation

@synackd
Copy link
Collaborator

@synackd synackd commented Sep 2, 2025

Closes #32

Changes

  • Add groups directive to static discovery specification to specify a list of groups to add a node to during discovery.
    • These groups will get created in SMD.
  • Deprecate group directive in static discovery specification.
    • Add deprecation warnings for group and remove it from examples
    • If used with groups, the group specified is merged with the groups list
    • group will be removed in a future release of ochami
  • Refactor deduplication maps to be defined outside of node loop (feat: add fake power actions #47 (comment))

Testing

Write a nodes.yaml that includes combinations of groups and group and attempt static discovery via ochami discover static -f yaml -d @nodes.yaml.

  • groups only:

    nodes:
    ...
     - name: node01
       groups:
         - group1
         - group2
       ...

    Result: node01 should be added to groups group1 and group2, which get added to SMD.

  • group only:

    nodes:
    ...
     - name: node01
       group: group1
       ...

    Result: node01 should be added to group group1, which gets added to SMD. The ochami command should display a warning that group was used for node01 and that it is deprecated.

  • group and groups:

    CASE 1: group is also a member of groups

    nodes:
    ...
     - name: node01
       group: group1
       groups:
         - group1
         - group2
       ...

    Result: node01 should be added to groups group1 and group2, which get added to SMD. The ochami command should display a warning that group was used for node01 and that it is deprecated.

    CASE 2: group is not a member of groups

    nodes:
    ...
     - name: node01
       group: group3
       groups:
         - group1
         - group2
       ...

    Result: node01 should be added to groups group1, group2, and group3, which get added to SMD. The ochami command should display a warning that group was used for node01 and that it is deprecated.

Notes

@synackd synackd added enhancement New feature or request needs testing Needs more testing before approval labels Sep 2, 2025
@synackd synackd force-pushed the feat/discovery-multiple-groups branch from 6dc0c87 to e1591d8 Compare September 2, 2025 00:06
@davidallendj
Copy link
Contributor

Looks like everything works and the deprecated message is shown properly from my testing.

Copy link
Contributor

@davidallendj davidallendj left a comment

Choose a reason for hiding this comment

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

LGTM

Looks like it needs to be rebased as well.

Add a 'groups' directive in the static discovery specification that
allows nodes to be members of multiple groups. These groups are
deduplicated and created in SMD during discovery.

Deprecate the 'group' directive in the static discovery specification.
As of this commit, 'groups' will still work, but a deprecation warning
is printed if it is used in a node definition. If both 'group' and
'groups' are specified, the 'group' entry is merged with the 'groups'
list as if it were another entry in the list. The 'group' directive will
be removed in a future release.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
Deduplication maps for BMC managers and Systems were declared within the
loop that iterates over each node, which is where duplication checks
happen. This is obviously futile, so the declarations are moved outside
of the loop so that the checks occur properly.

Most testing has used a single unique BMC per node, which is probably
why this was not caught earlier.

Signed-off-by: Devon Bautista <17506592+synackd@users.noreply.github.com>
@synackd synackd force-pushed the feat/discovery-multiple-groups branch from e1591d8 to ada0f41 Compare September 2, 2025 19:32
@synackd
Copy link
Collaborator Author

synackd commented Sep 2, 2025

Rebased and checks are passing. Hitting merge.

@synackd synackd merged commit 35f3b89 into main Sep 2, 2025
9 checks passed
@synackd synackd deleted the feat/discovery-multiple-groups branch September 2, 2025 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request needs testing Needs more testing before approval

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[DEV] Support multiple groups for static discovery

3 participants