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

Lack of extended validation for resyncPeriod #1483

Closed
cnCloXie opened this issue Apr 8, 2024 · 4 comments · Fixed by #1505
Closed

Lack of extended validation for resyncPeriod #1483

cnCloXie opened this issue Apr 8, 2024 · 4 comments · Fixed by #1505
Labels
triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@cnCloXie
Copy link

cnCloXie commented Apr 8, 2024

Describe the bug

The GrafanaDashboard get resynced every 5 mins even have provided a different value(like 30d) to resyncPeriod.

Version

v5.4.0

To Reproduce
Steps to reproduce the behavior:

  1. Create resource
apiVersion: grafana.integreatly.org/v1beta1
kind: GrafanaDashboard
metadata:
  name: dashboard-a
  labels:
    app: grafana
    grafana: dashboards
spec:
  folder: "A"
  resyncPeriod: 30d
  instanceSelector:
    matchLabels:
      dashboards: "grafana"
  json: ""
  1. Check the LAST RESYNC field with kubectl get grafanadashboard dashboard-a and you can see it still gets synced every 5 mins and all the temporary changes on it have been reverted...

Expected behavior

Resync every 30d.

Suspect component/Location where the bug might be occurring
Please provide this if you know where this bug might occur otherwise leave as unknown

Screenshots
If applicable, add screenshots to help explain your problem.

image

Runtime (please complete the following information):

  • OS: Linux
  • Grafana Operator Version [v5.4.0]
  • Environment: Kubernetes
  • Deployment type: [deployed]

Additional context
Add any other context about the problem here.

@cnCloXie cnCloXie added bug Something isn't working needs triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 8, 2024
@pb82
Copy link
Collaborator

pb82 commented Apr 9, 2024

@cnCloXie the problem is that 30d is not a valid duration, see here. You could write it as hours: resyncPeriod: 720h

@HVBE HVBE added wontfix This will not be worked on triage/unresolved Indicates an issue that can not or will not be resolved. and removed bug Something isn't working needs triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Apr 9, 2024
@pb82 pb82 added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed triage/unresolved Indicates an issue that can not or will not be resolved. wontfix This will not be worked on labels Apr 9, 2024
@NissesSenap
Copy link
Collaborator

So let's solve this by making use of CRD validation, we can see an example on how to make sure you can't use anything above h

// +kubebuilder:validation:Type=string

@weisdd weisdd changed the title [Bug] GrafanaDashboard sync doesnt respect to resyncPeriod Lack of extended validation for resyncPeriod Apr 9, 2024
@weisdd
Copy link
Collaborator

weisdd commented Apr 9, 2024

After internal discussion, we decided that we should add additional validation rules for resyncPeriod to our CRDs to make sure k8s API server accepts only those values that can be parsed by Go runtime.

@cnCloXie
Copy link
Author

cnCloXie commented Apr 10, 2024

Thank you guys !
Ah yes adding validation in the CR, complaining to the operator's logs or translating d/m/y into hours, either way will work for me which could make me myself be aware of my input mistake explicitly. Thank U for reply and the operator !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants