Skip to content

Conversation

@m-bull
Copy link
Contributor

@m-bull m-bull commented Sep 22, 2025

The Kayobe monitoring group, not the contollers group, is mapped to the Kolla-Ansible monitoring group by default.

@m-bull m-bull requested a review from a team as a code owner September 22, 2025 10:13
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request provides a documentation fix within a commented-out example in etc/kayobe/kolla.yml. The change corrects the default group mapping for monitoring services from controllers to monitoring under kolla_overcloud_inventory_top_level_group_map. This aligns the example with the intended default behavior, improving configuration clarity for users. The change is correct and I have no further feedback.

@MoteHue
Copy link
Contributor

MoteHue commented Sep 22, 2025

Should we change the comment to just link to the defaults instead?

The network group is no longer the controllers too, and there's now a compute-vgpu group we don't mention at all

@m-bull
Copy link
Contributor Author

m-bull commented Sep 22, 2025

I guess we'd have to remember to update the link each release, but a grep -r <previous-release> should catch it - we have to do that in a few places, so seems like a reasonable tradeoff to me.

@Alex-Welsh
Copy link
Member

I guess we'd have to remember to update the link each release, but a grep -r <previous-release> should catch it - we have to do that in a few places, so seems like a reasonable tradeoff to me.

We've got NOTE(upgrade): which we use for stuff like that, as a thing to search for and update each release

Alex-Welsh
Alex-Welsh previously approved these changes Oct 21, 2025
Copy link
Member

@Alex-Welsh Alex-Welsh left a comment

Choose a reason for hiding this comment

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

We can tweak this in a follow up PR if needed. Lets just fix it as-is for now

@Alex-Welsh Alex-Welsh enabled auto-merge (rebase) October 21, 2025 11:55
@MoteHue
Copy link
Contributor

MoteHue commented Oct 21, 2025

We can tweak this in a follow up PR if needed. Lets just fix it as-is for now

We should still update the other groups to be correct. i.e. change network, storage, and add compute-vgpu.
https://github.com/openstack/kayobe/blob/33f39fa6cace1f7b408163f7d5faa901a6491be4/ansible/inventory/group_vars/all/kolla#L360-L378

kolla_overcloud_inventory_top_level_group_map:
  control:
    groups:
      - controllers
  network:
    groups:
      - network
  compute:
    groups:
      - compute
  compute-vgpu:
    groups:
      - compute-vgpu
  monitoring:
    groups:
      - monitoring
  storage:
    groups:
      "{{ kolla_overcloud_inventory_storage_groups }}"

@MoteHue MoteHue disabled auto-merge October 21, 2025 13:09
@Alex-Welsh
Copy link
Member

We can tweak this in a follow up PR if needed. Lets just fix it as-is for now

We should still update the other groups to be correct. i.e. change network, storage, and add compute-vgpu. https://github.com/openstack/kayobe/blob/33f39fa6cace1f7b408163f7d5faa901a6491be4/ansible/inventory/group_vars/all/kolla#L360-L378

kolla_overcloud_inventory_top_level_group_map:
  control:
    groups:
      - controllers
  network:
    groups:
      - network
  compute:
    groups:
      - compute
  compute-vgpu:
    groups:
      - compute-vgpu
  monitoring:
    groups:
      - monitoring
  storage:
    groups:
      "{{ kolla_overcloud_inventory_storage_groups }}"

Ah, of course 🤦. My fault, trying to rush through PRs again

@Alex-Welsh Alex-Welsh dismissed their stale review October 21, 2025 13:17

Bad review

@Alex-Welsh Alex-Welsh force-pushed the fix-group-map-comment branch from a5e0564 to bd65bcd Compare December 18, 2025 10:24
@Alex-Welsh
Copy link
Member

Updated to link to the right section. I've made the link generic enough that it shouldn't need to be changed between releases

The previous comment included incorrect values for the default values in
Kayobe. This commit corrects the comment to instead just provide a link
to the current default values in the Kayobe source.
@Alex-Welsh Alex-Welsh enabled auto-merge (rebase) December 18, 2025 10:54
@Alex-Welsh Alex-Welsh merged commit f5d1992 into stackhpc/2025.1 Dec 18, 2025
40 of 41 checks passed
@Alex-Welsh Alex-Welsh deleted the fix-group-map-comment branch December 18, 2025 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants