Fewer steps to remove saved reports

[srpomeroy]

What problem are you trying to solve? Easier management of saved reports.

Describe the solution you'd like The ability to unsave/delete saved reports from the Reports (/reports.html) view.

Describe alternatives you've considered Helm makes sense for defining dashboards that are known to be useful.

How would users interact with this feature? Browse to Reports page, find the report in question, select unsave/delete button, confirm unsave/deletion.

[AjayTripathy]

This isn't really feasible if you've added your reports from configmap. A configmap and ultimately your helm values stored in git should be the source of truth for defining data in the application-- we've gone down this hole before. UI updates are for demos and temporary fixes. Can you add a little more context about why updating from values isn't enough?

[srpomeroy]

So the expectation is to update the helm values as users define new dashboards?

[kbrwn]

We should have support for updating/merging to the saved reports configmap. We could let users mount the CM dynamically if they want this feature. Defining all saved reports in values.yaml is fine if the user follows gitops. It creates a lot of friction if not. How does a user go from a generated to saved report? The yaml that needs to be passed is not obvious. The yaml that may need to be deleted to remove a report is not obvious. There may be users that interact with kubecost but not the administration team of kubecost or the cluster who would like saved reports to survive updates and have persistence or decide when to delete them. The UI does not feel first class if it cannot make persistent modifications to the backend and application configuration.

[jessegoodier]

Agree that "Saved reports" should be more manageable. Does the query string for the report contain all the information needed? If so, we could really simplify the creation by just using that in the config map. Alternatively have a json export like grafana. Perhaps 2 config maps? one for imported reports and one for user-created?

[AjayTripathy]

This is a pretty large project that involves changing the scope of our permissions. We would at minimum need confimap-write permissions to kubecost. I don't think now is the right time to take this on, but I'm happy to add it to the backlog.

[dwbrown2]

Are there two potential feature requests in this issue?

1) Simplify the Report UI to remove reports -- i.e. add a button next to OPEN called DELETE (for reports created in UI) 2) Allow users to modify configmaps from the UI

The former seems simple and straightforwad, the latter seems more tricky given that a new permission would be required... which potentially could be done via the Kubecost controller.

[AjayTripathy]

OPEN and DELETE don't sync back to the configmap anyway, so it really doesn't matter. If you're running the configurations from a configmap, you can't expect your UI changes to stick without write permissions.

[dwbrown2]

OPEN and DELETE don't sync back to the configmap anyway, so it really doesn't matter. If you're running the configurations from a configmap, you can't expect your UI changes to stick without write permissions.

Point 1 is only actionable for reports from UI, but also a good way to indicate that certain reports are from configmap...

[dwbrown2]

@srpomeroy any feedback on https://github.com/kubecost/cost-analyzer-helm-chart/issues/1284#issuecomment-1068537530 is point 1 relevant or are we really just focused on reports created via configmap?

[srpomeroy]

The intent of my original request was for reports created via the UI. So yes, point 1 is relevant.

[dwbrown2]

@nealormsbee hoping this is quick, but here are states that seem potentially helpful to communicate via the UI.

1) non-admin can't delete, i.e. user does not have permissions 2) created via configmap can't delete, i.e. kubecost does not have permissions 3) non-admin + config can't delete, i.e. neither user nor kubecost has permissions 4) created via UI can delete

[nealormsbee]

Going to start on this today. I have one question.

In the Cost Allocation UI currently, we don't differentiate between configmap-created and UI-created reports. We let users delete configmap-created reports -- they just come back the next time the CM is used to seed the known reports list.

Is the proposal here to change the whole paradigm to one where users cannot delete configmap-supplied reports in the Cost Allocation UI as well?

[srpomeroy]

What circumstances do we reload the configmap under?

On Tue, Mar 29, 2022 at 12:22 PM Neal Ormsbee @.***> wrote:

Going to start on this today. I have one question.

In the Cost Allocation UI currently, we don't differentiate between configmap-created and UI-created reports. We let users delete configmap-created reports -- they just come back the next time the CM is used to seed the known reports list.

Is the proposal here to change the whole paradigm to one where users cannot delete configmap-supplied reports in the Cost Allocation UI as well?

— Reply to this email directly, view it on GitHub https://github.com/kubecost/cost-analyzer-helm-chart/issues/1284#issuecomment-1082089902, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADXPL5GBE4DZOEEPSTKLIETVCMU4HANCNFSM5PVC3X5A . You are receiving this because you were mentioned.Message ID: @.***>

-- Sean Pomeroy Solution Engineer

Docs http://docs.kubecost.com/ | Blog https://blog.kubecost.com/ | Slack https://kubecost.slack.com/ | Github https://github.com/kubecost | kubecost.com

[nealormsbee]

Any time the pod goes down and comes back up. So it won't happen on a smooth Helm upgrade, but uninstall / reinstall or a downed pod will trigger this.

[AjayTripathy]

The pod most likely will restart on a helm upgrade. In general, I would not try and predict when we will sync to the state of the configmap, only that we certainly will sync to the state of the configmap at some point.

Is the proposal here to change the whole paradigm to one where users cannot delete configmap-supplied reports in the Cost Allocation UI as well?

The invariant is; if you define anything via configmap, UI changes will only be temporary, and at any time can be overwritten. Define everything in the UI and use a persistent volume or everything via configmap. It's a non-goal today to write UI changes back to the configmap. I'm open to such a project but there are pretty big implications about expanding the scope of our permissions that make it seem out of scope.

[nealormsbee]

Thank you for clarifying. I ran a Helm upgrade on my install to test and the changes stayed -- I must have gotten lucky (or unlucky, depending on how you think about it). The More You Know^TM

I'd break the ideas from this issue into three layers:

(1) Making the experience of deleting reports less arduous. Currently you have to open each report, then click "Unsave" from the cost allocation menu. Resolution here could be as simple as adding a "DELETE" button to every report on the Reports page, so they can be deleted from that list without opening.

(2) Making CM-driven reports clearly immutable via the UI. This is a break from our current MO, which is that you can modify whatever you want, just know that it all resets when your pod restarts. Instead, we'd be saying "since this was put here by your GitOps overlords, it stays".

(3) Per Kyle's comment, making it easier to permanently persist alerts created in the UI. We've talked about different approaches to this before: using CM write-permissions to let the UI modify the CM; making it easy to export a Report's yaml so that it can be added to the values.yaml. These are the two primary ideas I'm aware of; the former circumvents GitOps to an extent, while the latter makes it easier to export to GitOps formats.

These go ascending order of how thoughtful we'd need to be. Keeping the rule the same, and allowing deletes from Reports, is straightforward and noncontroversial. Making CM-driven reports immutable via the UI (what I think Webb was suggesting) is fairly straightforward, but a different MO from how we've handled things up to now. Actually modifying the configmap has fairly large implications and is a place to tread carefully.

I'll plan on implementing (1) for now, because it seems like that gets us what we want here. Totally open to discussions around (3) for the longer-term, but agree with Ajay that it's a much larger scope of discussion. If I'm wrong and we really need (2) in the near-term for some reason, somebody correct me and we can discuss.

[dwbrown2]

I'll plan on implementing (1) for now, because it seems like that gets us what we want here.

SGTM, I'd just note the permissions above and ideally show a button in an inactive state or some equivalent if possible!

[AjayTripathy]

For 2, could be a help to support if we simply warned the user that updates may not persist in "configmap" mode. How feasible is that?

[nealormsbee]

Showing that information seems totally reasonable to me :+1:

[jessegoodier]

Potentially for future consideration-

I understand that CM's are designed to be read only. I think that was just a suggestion for ease of use.

Alternatively: For "ui-created-reports" there could be a new PV that must be manually created (or flag via helm).

For this volume, the logic could be: if present: save all UI created reports to the that volume. otherwise: use existing behavior.

A side benefit is that this is a completely unmanaged (by helm) object that the users are responsible for. This volume could be used for the alerts too.

In the docs/guide we would explain that it is the admin's responsibility to backup that volume.

Another nice to have would be an API for json import/export (like grafana) for managing that volume.

[nealormsbee]

Pull request open here: https://github.com/kubecost/cost-analyzer-frontend/pull/576

Have a look and weigh in! If you'd like it deployed live somewhere, let me know and I can set it up.

[dwbrown2]

Would be awesome to see it live if that's easy to do!

[nealormsbee]

http://a642d8019cb7d4e3697c0fb950b73f1c-687369336.us-east-2.elb.amazonaws.com:9090/reports.html @dwbrown2