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

[ISSUE-572] ability to set mem/cpu for csi containers #82

Merged
merged 31 commits into from
Nov 1, 2021

Conversation

yurkov-anton
Copy link
Contributor

@yurkov-anton yurkov-anton commented Oct 13, 2021

Purpose

Issue dell/csi-baremetal#572

  • Add Resources *corev1.ResourceRequirements to all components api/v1/components
  • Add follow tree to each csi component in csi-baremetal-deployment and value.yaml:
      resources:
        requests:
          memory: 
          cpu: 
        limits:
          memory: 
          cpu:

PR checklist

  • Add link to the issue
  • Choose PR label
  • New unit tests added
  • Modified code has meaningful comments
  • All TODOs are linked with the issues
  • All comments are resolved

Testing

Check current resources:

  1. deploy csi-operator from master branch
helm install csi-baremetal-operator http://asdrepo.isus.emc.com:8081/artifactory/atlantic-helm-build/csi-operator/${CSI_OPERATOR}/csi-baremetal-operator-${CSI_OPERATOR}.tgz --set image.tag=${CSI_OPERATOR} --set global.registry=asdrepo.isus.emc.com:9042 --set log.level=debug
  1. deploy csi driver:
helm install csi-baremetal http://asdrepo.isus.emc.com:8081/artifactory/atlantic-helm-build/csi-operator/${CSI_OPERATOR}/csi-baremetal-deployment-${CSI_OPERATOR}.tgz --set image.tag=${CSI_VERSION} --set global.registry=asdrepo.isus.emc.com:9042 --set log.level=debug --set feature.extender=true  --set driver.drivemgr.type=halmgr  --set driver.alerts.mountConfig=true --set platform=vanilla
  1. check csi containers resources:
$ kubectl get pod csi-baremetal-controller-64d9bd7bb5-mbv5x --output=yaml | grep -A 6 resources
    resources: {}
    ...
--
    resources: {}
    ...
--
    resources: {}
	...
--
    resources: {}
    ...
$ kubectl get pod csi-baremetal-node-controller-6d85489d95-gxck4 --output=yaml | grep -A 6 resources
    resources: {}
    ...
$ kubectl get pod csi-baremetal-node-kernel-5.4-2jbtm --output=yaml | grep -A 6 resources
    resources: {}
    ...
--
    resources: {}
    ...
--
    resources: {}
    ...
--
    resources: {}
    ...
  1. upgrade csi-operator to version with CPU/memory values:
export CSI_OPERATOR=0.5.0-100.019c8f4
helm upgrade csi-baremetal-operator http://asdrepo.isus.emc.com:8081/artifactory/atlantic-helm-build/csi-operator/${CSI_OPERATOR}/csi-baremetal-operator-${CSI_OPERATOR}.tgz --set image.tag=${CSI_OPERATOR} --set global.registry=asdrepo.isus.emc.com:9042 --set log.level=debug
  1. upgrade csi driver:
helm upgrade csi-baremetal http://asdrepo.isus.emc.com:8081/artifactory/atlantic-helm-build/csi-operator/${CSI_OPERATOR}/csi-baremetal-deployment-${CSI_OPERATOR}.tgz --set image.tag=${CSI_VERSION} --set global.registry=asdrepo.isus.emc.com:9042 --set log.level=debug --set feature.extender=true  --set driver.drivemgr.type=halmgr  --set driver.alerts.mountConfig=true --set platform=vanilla --set  driver.controller.resources.requests.memory=50Mi --set driver.node.resources.limits.cpu=200m
  1. check csi containers resources:
anton@ubuntu:~/workspace$ kubectl get pod csi-baremetal-controller-776c84b68d-clpk6 --output=yaml | grep -A 6 resources
    resources:
      requests:
        memory: 50Mi
    ...
--
    resources: {}
    ...
--
    resources: {}
    ...
--
    resources: {}
    ...
$ kubectl get pod csi-baremetal-node-controller-6d85489d95-gxck4 --output=yaml | grep -A 6 resources
    resources: {}
    ...
$ kubectl get pod csi-baremetal-node-kernel-5.4-gmq4g --output=yaml | grep -A 6 resources
    resources: {}
    ...
--
    resources: {}
    ...
--
    resources:
      limits:
        cpu: 200m
      requests:
        cpu: 200m
    ...
--
    resources: {}

$ kubectl get pod csi-baremetal-se-tzjcf --output=yaml | grep -A 6 resources
    resources: {}
    ...

@yurkov-anton yurkov-anton requested a review from a team as a code owner October 13, 2021 02:27
@codecov
Copy link

codecov bot commented Oct 13, 2021

Codecov Report

Merging #82 (4b7c5c3) into master (9de0124) will decrease coverage by 0.07%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #82      +/-   ##
==========================================
- Coverage   29.67%   29.59%   -0.08%     
==========================================
  Files          12       12              
  Lines        1065     1071       +6     
==========================================
+ Hits          316      317       +1     
- Misses        699      704       +5     
  Partials       50       50              
Flag Coverage Δ
unittests 29.59% <16.66%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/node/node_daemonset.go 0.00% <0.00%> (ø)
pkg/patcher/scheduler_patcher_vanilla.go 0.00% <0.00%> (ø)
pkg/patcher/patcher_configuration.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9de0124...4b7c5c3. Read the comment docs.

Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
@yurkov-anton yurkov-anton force-pushed the feature-issue-572-mem-cpu-chart-values branch from fbfd7a8 to 337e146 Compare October 13, 2021 02:32
@safronovD
Copy link
Contributor

Could you add resources fields into Operator chart values too?

Signed-off-by: Anton Yurkov <[email protected]>
Copy link
Contributor

@rkiselenko rkiselenko left a comment

Choose a reason for hiding this comment

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

Since components specs changed we need to regenerate CRDs

@yurkov-anton yurkov-anton added the enhancement New feature or request label Oct 14, 2021
api/v1/components/controller.go Outdated Show resolved Hide resolved
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
Signed-off-by: Anton Yurkov <[email protected]>
@yurkov-anton yurkov-anton marked this pull request as draft October 28, 2021 07:49
Signed-off-by: Anton Yurkov <[email protected]>
@mishoyama mishoyama marked this pull request as ready for review October 29, 2021 09:19
Copy link
Contributor

@rkiselenko rkiselenko left a comment

Choose a reason for hiding this comment

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

LGTM

@yurkov-anton yurkov-anton merged commit 3af3d6f into master Nov 1, 2021
@yurkov-anton yurkov-anton deleted the feature-issue-572-mem-cpu-chart-values branch November 1, 2021 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants