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

Add billing to services #240

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Add billing to services #240

merged 1 commit into from
Oct 14, 2024

Conversation

zugao
Copy link
Collaborator

@zugao zugao commented Oct 4, 2024

Summary

  • Added a label to each of our services to allow easy scrape of billing metrics

Checklist

  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency
    as they show up in the changelog
  • Update tests.
  • Link this PR to related issues.

@zugao zugao added bug Something isn't working patch labels Oct 4, 2024
@zugao zugao requested review from a team, Kidswiss, TheBigLee and wejdross and removed request for a team October 7, 2024 15:14
@zugao zugao force-pushed the add-service-billing-label branch 6 times, most recently from 7d030bf to 7be3a42 Compare October 8, 2024 11:59
apis/vshn/v1/common_types.go Outdated Show resolved Hide resolved
@wejdross
Copy link
Member

wejdross commented Oct 8, 2024

build is also failing and needs to be fixed

Copy link
Member

@TheBigLee TheBigLee left a comment

Choose a reason for hiding this comment

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

Please also fix the failing build action

pkg/comp-functions/functions/common/interfaces.go Outdated Show resolved Hide resolved
@zugao zugao force-pushed the add-service-billing-label branch 2 times, most recently from b1695ff to e8b5a59 Compare October 9, 2024 08:18
Copy link
Member

@wejdross wejdross left a comment

Choose a reason for hiding this comment

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

This code won't work if we have multiple deployments/statefullsets in single instance namespace, but that was never in acceptance criteria, therefore I'll aproove that.

apis/vshn/v1/vshn_minio.go Show resolved Hide resolved
@zugao
Copy link
Collaborator Author

zugao commented Oct 9, 2024

This code won't work if we have multiple deployments/statefullsets in single instance namespace, but that was never in acceptance criteria, therefore I'll aproove that.

This is exactly the reason why this PR exists, to filter other deployments/statefulsets. Currently we have 2 deployments for nextcloud, one for the service itself and one for the metrics and with current code we count nextcloud twice into billing. I understand that in the future other deployments and statefulsets might reside into the instance namespace that HAVE to be counted towards billing but this is not the case right now and we should never implement something for the future as we don't know how it will look like from requirements point of view.

Update: the point is simple, we cannot know which statefulsets/deployments must be billed, metrics - no, collabora - yes, X no or yes? this has to be done when a new deployment/staefulset is added.

@zugao zugao force-pushed the add-service-billing-label branch 4 times, most recently from 8d33a34 to 7af8dfe Compare October 14, 2024 08:10
@zugao zugao merged commit 64e3bad into master Oct 14, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working patch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants