search

kubernetes-csi / external-snapshotter

Sidecar container that watches Kubernetes Snapshot CRD objects and triggers CreateSnapshot/DeleteSnapshot against a CSI endpoint.
Apache License 2.0
484 stars 369 forks source link

No resource limits in the snapshot controller #620

Closed LarsBingBong closed 2 years ago

LarsBingBong commented 2 years ago

I can see that there are no resource limit section in the workload spun up by this deployment.

Is that on purpose?

I have a Kubernetes linting tool warning me of this and in general, I thought this to be a best practice. Or is that a special case?

Looking forward to hearing from you.

Thank you very much

Kartik494 commented 2 years ago

@LarsBingBong Could you describe here warnings you got for not having resource limit in deployment file?

LarsBingBong commented 2 years ago

Sure ... I'm using [datree]() a Kubernetes YAML linter and quality tool. The warning I get is:

>>  File: infrastructure-services/storage/velero/server/csi-snapshot-support/setup-snapshot-controller.yaml

[V] YAML validation
[V] Kubernetes schema validation

[X] Policy check

❌  Ensure each container has a configured memory request  [1 occurrence]
    — metadata.name: snapshot-controller (kind: Deployment)
💡  Missing property object `requests.memory` - value should be within the accepted boundaries recommended by the organization

❌  Ensure each container has a configured CPU request  [1 occurrence]
    — metadata.name: snapshot-controller (kind: Deployment)
💡  Missing property object `requests.cpu` - value should be within the accepted boundaries recommended by the organization

❌  Ensure each container has a configured memory limit  [1 occurrence]
    — metadata.name: snapshot-controller (kind: Deployment)
💡  Missing property object `limits.memory` - value should be within the accepted boundaries recommended by the organization

❌  Ensure each container has a configured CPU limit  [1 occurrence]
    — metadata.name: snapshot-controller (kind: Deployment)
💡  Missing property object `limits.cpu` - value should be within the accepted boundaries recommended by the organization

❌  Ensure each container has a configured liveness probe  [1 occurrence]
    — metadata.name: snapshot-controller (kind: Deployment)
💡  Missing property object `livenessProbe` - add a properly configured livenessProbe to catch possible deadlocks

❌  Ensure each container has a configured readiness probe  [1 occurrence]
    — metadata.name: snapshot-controller (kind: Deployment)
💡  Missing property object `readinessProbe` - add a properly configured readinessProbe to notify kubelet your Pods are ready for traffic

(Summary)

- Passing YAML validation: 1/1

- Passing Kubernetes (1.18.0) schema validation: 1/1

- Passing policy check: 0/1

This output can be seen in the console of my IDE as Datree executes as part of a git pre-commit hook.

Also, I got this very telling and good answer over on Slack. Mr. Patrick Ohly answering ( part of the answer ):

The problem is that any kind of limit will be rather arbitrary. The actual limits depend on the kind of load that the sidecar will have to handle. This is something that cannot be generalized easily.We have an issue open for the csi hostpath driver example deployments, with no good solution....

I hope that clarifies it.

xing-yang commented 2 years ago

Add this issue here for reference: https://github.com/kubernetes-csi/csi-driver-host-path/issues/47

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

k8s-triage-robot commented 2 years ago

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot commented 2 years ago

@k8s-triage-robot: Closing this issue.

In response to [this](https://github.com/kubernetes-csi/external-snapshotter/issues/620#issuecomment-1112474227): >The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. > >This bot triages issues and PRs according to the following rules: >- After 90d of inactivity, `lifecycle/stale` is applied >- After 30d of inactivity since `lifecycle/stale` was applied, `lifecycle/rotten` is applied >- After 30d of inactivity since `lifecycle/rotten` was applied, the issue is closed > >You can: >- Reopen this issue or PR with `/reopen` >- Mark this issue or PR as fresh with `/remove-lifecycle rotten` >- Offer to help out with [Issue Triage][1] > >Please send feedback to sig-contributor-experience at [kubernetes/community](https://github.com/kubernetes/community). > >/close > >[1]: https://www.kubernetes.dev/docs/guide/issue-triage/ Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.