Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Various os_capacity fixes #697

Merged
merged 1 commit into from
Feb 21, 2024
Merged

Conversation

assumptionsandg
Copy link
Contributor

@assumptionsandg assumptionsandg commented Oct 9, 2023

  • Various typo fixes.
  • Renamed os_exporter to os_capacity.
  • Renamed 70-oscapacity.yml.j2 to 70-oscapacity.yml to ensure correct templating.
  • Added 70-oscapacity.yml to .yamlint ignore list.
  • Conditional flag stackhpc_enable_os_capacity added to prevent HAProxy config and Prometheus scrape targets being copied in before manual deployment.
  • Conditional flag stackhpc_os_capacity_openstack_verify added where certificate verification can be overridden.
  • Fixed interval errors

@assumptionsandg assumptionsandg force-pushed the os-capacity-more-fixes branch 2 times, most recently from 83f33ee to ac765f6 Compare October 9, 2023 09:40
Copy link
Member

@priteau priteau left a comment

Choose a reason for hiding this comment

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

Existing docs say:

StackHPC Kayobe Config includes this exporter by default

Should this be changed? Please also fix the typo: avaliable

etc/kayobe/ansible/templates/os_capacity-clouds.yml.j2 Outdated Show resolved Hide resolved
etc/kayobe/kolla/config/haproxy/services.d/os_exporter.cfg Outdated Show resolved Hide resolved
doc/source/configuration/monitoring.rst Outdated Show resolved Hide resolved
@assumptionsandg assumptionsandg force-pushed the os-capacity-more-fixes branch 4 times, most recently from 702df15 to 8fad37d Compare October 9, 2023 09:56
@assumptionsandg
Copy link
Contributor Author

Existing docs say:

StackHPC Kayobe Config includes this exporter by default

Should this be changed? Please also fix the typo: avaliable

I've made an update to the wording to show deployment needs to be triggered manually, hopefully that's better.

@assumptionsandg assumptionsandg force-pushed the os-capacity-more-fixes branch 3 times, most recently from 3a77329 to ccf7b79 Compare October 9, 2023 11:08
@assumptionsandg assumptionsandg marked this pull request as ready for review October 9, 2023 11:13
@assumptionsandg assumptionsandg requested a review from a team as a code owner October 9, 2023 11:13
mnasiadka
mnasiadka previously approved these changes Oct 11, 2023
.yamllint Outdated Show resolved Hide resolved
@assumptionsandg assumptionsandg force-pushed the os-capacity-more-fixes branch 2 times, most recently from 7a95ca8 to f8477f9 Compare October 18, 2023 14:21
Alex-Welsh
Alex-Welsh previously approved these changes Oct 20, 2023
Copy link
Contributor

@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.

LGTM

@assumptionsandg assumptionsandg force-pushed the os-capacity-more-fixes branch 2 times, most recently from b993fc2 to 27cc49c Compare October 20, 2023 13:42
@assumptionsandg
Copy link
Contributor Author

  • Minor dashboard updates for graph intervals.
  • Dashboard overhaul for OpenStack Project Metrics now including project and instance utilization

@assumptionsandg assumptionsandg marked this pull request as draft October 20, 2023 13:58
.yamllint Outdated Show resolved Hide resolved
etc/kayobe/ansible/deploy-os-capacity-exporter.yml Outdated Show resolved Hide resolved
@priteau
Copy link
Member

priteau commented Feb 16, 2024

There is also a conflict caused by the rename of the HAProxy file: deployments which already have deployed os_exporter.cfg will get the following HAProxy error:

[ALERT]    (42) : Parsing [/etc/haproxy/services.d/os_capacity.cfg:2]: frontend 'os_capacity_frontend' has the same name as frontend 'os_capacity_frontend' declared at /etc/haproxy/services.d/os_exporter.cfg:2.
[ALERT]    (42) : Parsing [/etc/haproxy/services.d/os_capacity.cfg:11]: backend 'os_capacity_backend' has the same name as backend 'os_capacity_backend' declared at /etc/haproxy/services.d/os_exporter.cfg:11.
[ALERT]    (42) : Error(s) found in configuration file : /etc/haproxy/services.d/os_capacity.cfg

Operators need to be aware that they must delete /etc/kolla/haproxy/services.d/os_exporter.cfg first.

@mnasiadka
Copy link
Member

There is also a conflict caused by the rename of the HAProxy file: deployments which already have deployed os_exporter.cfg will get the following HAProxy error:

[ALERT]    (42) : Parsing [/etc/haproxy/services.d/os_capacity.cfg:2]: frontend 'os_capacity_frontend' has the same name as frontend 'os_capacity_frontend' declared at /etc/haproxy/services.d/os_exporter.cfg:2.
[ALERT]    (42) : Parsing [/etc/haproxy/services.d/os_capacity.cfg:11]: backend 'os_capacity_backend' has the same name as backend 'os_capacity_backend' declared at /etc/haproxy/services.d/os_exporter.cfg:11.
[ALERT]    (42) : Error(s) found in configuration file : /etc/haproxy/services.d/os_capacity.cfg

Operators need to be aware that they must delete /etc/kolla/haproxy/services.d/os_exporter.cfg first.

I guess it would make sense to delete that in the deployment playbook

@priteau
Copy link
Member

priteau commented Feb 16, 2024

There is also a conflict caused by the rename of the HAProxy file: deployments which already have deployed os_exporter.cfg will get the following HAProxy error:

[ALERT]    (42) : Parsing [/etc/haproxy/services.d/os_capacity.cfg:2]: frontend 'os_capacity_frontend' has the same name as frontend 'os_capacity_frontend' declared at /etc/haproxy/services.d/os_exporter.cfg:2.
[ALERT]    (42) : Parsing [/etc/haproxy/services.d/os_capacity.cfg:11]: backend 'os_capacity_backend' has the same name as backend 'os_capacity_backend' declared at /etc/haproxy/services.d/os_exporter.cfg:11.
[ALERT]    (42) : Error(s) found in configuration file : /etc/haproxy/services.d/os_capacity.cfg

Operators need to be aware that they must delete /etc/kolla/haproxy/services.d/os_exporter.cfg first.

I guess it would make sense to delete that in the deployment playbook

Indeed. @assumptionsandg do you think you can make this change?

Copy link
Member

@priteau priteau left a comment

Choose a reason for hiding this comment

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

The "CPUs Used by Project" panel is reporting its values with MB unit.

@assumptionsandg
Copy link
Contributor Author

The "CPUs Used by Project" panel is reporting its values with MB unit.

Changed unit type to none

Copy link
Member

@priteau priteau left a comment

Choose a reason for hiding this comment

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

I can't see it in the diff but I also spotted a typo Avaliable in the dashboard.

@assumptionsandg assumptionsandg force-pushed the os-capacity-more-fixes branch 9 times, most recently from 93df838 to a60e4bc Compare February 20, 2024 14:03
@priteau priteau enabled auto-merge February 20, 2024 14:11
priteau
priteau previously approved these changes Feb 20, 2024
etc/kayobe/stackhpc-monitoring.yml Outdated Show resolved Hide resolved
etc/kayobe/stackhpc-monitoring.yml Outdated Show resolved Hide resolved
doc/source/configuration/monitoring.rst Outdated Show resolved Hide resolved
doc/source/configuration/monitoring.rst Outdated Show resolved Hide resolved
releasenotes/notes/os-capacity-94006f03f16583e4.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

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

Let's get this thing in.

@markgoddard markgoddard merged commit ed7b03f into stackhpc/yoga Feb 21, 2024
15 checks passed
@markgoddard markgoddard deleted the os-capacity-more-fixes branch February 21, 2024 11:01
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.

10 participants