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

project field can now list project names #554

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

leifmadsen
Copy link
Member

If CeilometerEnableTenantDiscovery is enabled, ceilometer samples
will contain project_name field, values from which are populated
in project field.

This change affects 2 dashboards, 'Cloud View' and 'Virtual Machine View',
only these allow switching between openstack projects.

If samples are missing project_name field, the regex for project
variable allows fallback to project field

@leifmadsen
Copy link
Member Author

I'm migrating this change over from infrawatch/dashboards#59 to give us more time to test this and update documentation without adjusting the base dashboards right now, and so that I can close the PR in the dashboards repo and set it to read-only.

Going forward, we want to make the changes here and not have changes land in the dashboard repo and then result in a reversion here. All dashboard changes for the core dashboards should come into the service-telemetry-operator repo as we'll provide management for them in STF 1.5.4.

Copy link
Collaborator

@vkmc vkmc left a comment

Choose a reason for hiding this comment

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

LGTM

@leifmadsen
Copy link
Member Author

I'm going to throw a do-not-merge label on this until some additional testing can be confirmed. This is in support of vanity names, but I haven't directly tested if this change continues to work with non-vanity OSP configuration. I'm under the impression it should work with both, but without confirmation I'd like to avoid merging this and breaking existing managed dashboards.

@leifmadsen leifmadsen added the do-not-merge Code is not ready to be merged label Jan 18, 2024
@yadneshk yadneshk force-pushed the yadneshk/project-names-in-dashboards branch from 1c11677 to 904de83 Compare February 9, 2024 12:52
@yadneshk
Copy link
Collaborator

yadneshk commented Feb 9, 2024

Screenshot from 2024-02-09 18-54-18

Did a little bit of testing today and here's how this looks with instances from multiple projects

@csibbitt
Copy link
Collaborator

Did a little bit of testing today and here's how this looks with instances from multiple projects

Did you get a chance to test how it looks when the OSP has not been configured to send the project names? That seems to be the concern that is blocking the merge.

If `CeilometerEnableTenantDiscovery` is enabled, ceilometer samples
will contain `project_name` field, values from which are populated
in `project` field.

This change affects 2 dashboards, 'Cloud View' and 'Virtual Machine View',
only these allow switching between openstack projects.

If samples are missing `project_name` field, the regex for `project`
variable allows fallback to `project` field
@yadneshk yadneshk force-pushed the yadneshk/project-names-in-dashboards branch from 904de83 to 512ea6b Compare February 27, 2024 13:43
@yadneshk
Copy link
Collaborator

@csibbitt If ceilometer doesn't send project names it would fallback to project ids. I lost the testing environment long back but I can retest and post a screenshot if needed

@csibbitt
Copy link
Collaborator

@csibbitt If ceilometer doesn't send project names it would fallback to project ids. I lost the testing environment long back but I can retest and post a screenshot if needed

(I don't say this often, but) I believe you. :) That seems like the expected behavior to me. Thanks!

@leifmadsen leifmadsen added the 1.5.5 STF 1.5 z5 release target label Mar 19, 2024
@leifmadsen
Copy link
Member Author

I will leave the do-not-merge label on this for now, but the team is welcome to discuss how to manage this going forward.

I tagged this for 1.5.5 (RHOCP 4.16 rebase) for now. I would suggest a tracking issue for this in order to provide the appropriate documentation and manual testing prior to release (QE).

Suggest checking with PM about level of effort to apply to have this landing in a future STF release, and how visible you'd like the feature to be, or whether this can land silently as an easter egg :)

@yadneshk
Copy link
Collaborator

Screenshot_20240321_161636

Here's how this looks if tenant discovery is disabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.5.5 STF 1.5 z5 release target do-not-merge Code is not ready to be merged
Development

Successfully merging this pull request may close these issues.

4 participants