Skip to content

Add Openstack Capacity to StackHPC Kayobe Config #678

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

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

assumptionsandg
Copy link
Contributor

@assumptionsandg assumptionsandg commented Sep 29, 2023

This patch adds Openstack Capacity to StackHPC Kayobe Config.

@assumptionsandg assumptionsandg requested a review from a team as a code owner September 29, 2023 11:47
@assumptionsandg assumptionsandg changed the title Add os_exporter playbook, config and dashboards Add Openstack Capacity to StackHPC Kayobe Config Sep 29, 2023
@Alex-Welsh
Copy link
Member

I think we need two more things.

  1. Either some documentation on what the playbook is an how to deploy it OR a hook to run it automatically
  2. A release note

@Alex-Welsh
Copy link
Member

Current CI failures are just from a lack of space on SMS. As long as one of them passes, I think we an force push these changes through

@mnasiadka
Copy link
Member

I would vote for some documentation, especially we require defined variables in secrets

@assumptionsandg
Copy link
Contributor Author

assumptionsandg commented Sep 29, 2023

I would vote for some documentation, especially we require defined variables in secrets

Updated the monitoring docs to include information on how to define secrets.

@Alex-Welsh
Copy link
Member

Couple of tox docs errors but otherwise looks good

@assumptionsandg
Copy link
Contributor Author

Couple of tox docs errors but otherwise looks good

Classic mistake 😅

@technowhizz
Copy link
Contributor

We should merge the PR on smslab that is pending first and ensure all comments from there are addressed to avoid having two open PR's with different discussions @assumptionsandg

@assumptionsandg assumptionsandg force-pushed the os_capacity branch 4 times, most recently from 5367e6f to 737376f Compare September 29, 2023 15:46
@assumptionsandg
Copy link
Contributor Author

Removed the playbook hook from service deploy to leave for manual deployment, as this exporter requires application credentials to be defined prior to deploying the exporter.

@Alex-Welsh
Copy link
Member

Alex-Welsh commented Oct 2, 2023

We should merge the PR on smslab that is pending first and ensure all comments from there are addressed to avoid having two open PR's with different discussions @assumptionsandg

I agree that we should resolve all the conversations, but I think it would be best to merge this one first, then sync smslab/yoga with skc/yoga so we don't end up with any merge weirdness. Or we could cherry-pick this commit to the SMS PR so the hash is the same which would have the same effect. Either way, best to get this through asap

Alex-Welsh
Alex-Welsh previously approved these changes Oct 2, 2023
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.

There's a few small things we could change with the documentation, and you could add a link in the release note to the relevant section, but I think we're hitting diminishing returns on polishing this PR. I'm happy with it as is

@Alex-Welsh
Copy link
Member

Something funky seems to have happened to your rebase. Might need a bit of git untangling

@assumptionsandg
Copy link
Contributor Author

Something funky seems to have happened to your rebase. Might need a bit of git untangling

Not sure what happened with that rebase, should be good now though! 😄

@@ -0,0 +1,9 @@
{% raw %}
scrape_configs:
- job_name: os-capacity
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail on a system with internal TLS. Requires this:

{% if kolla_enable_tls_internal | bool %}
    scheme: https
{% endif %}

Copy link
Contributor

Choose a reason for hiding this comment

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

Bumping this issue.

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.

6 participants