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

refact(cspc): add schema for auxResource in poolConfig #1567

Merged
merged 3 commits into from
Dec 30, 2019

Conversation

shubham14bajpai
Copy link
Contributor

@shubham14bajpai shubham14bajpai commented Dec 26, 2019

Signed-off-by: shubham [email protected]

What this PR does / why we need it:
This PR will add schema to pass auxResource limit to cStor pool pod side car containers via poolConfig.
The DefaultAuxResource is set if auxResource is not present in the poolConfig for the particular pool.

apiVersion: openebs.io/v1alpha1
kind: CStorPoolCluster
metadata:
  name: sparse-claim-auto
  namespace: openebs
spec:
  auxResources:
    requests:
      memory: "64Mi"
      cpu: "250m"
    limits:
      memory: "128Mi"
      cpu: "500m"
  pools:
  - nodeSelector:
      kubernetes.io/hostname: 127.0.0.1
    poolConfig:
      cacheFile: /tmp/sparse-claim-auto.cache
      compression: ""
      defaultRaidGroupType: mirror
      overProvisioning: false
      # AuxResources are applied to cstor-pool-mgmt and maya-exporter containers.  
      auxResources:
        requests:
           memory: "50Mi"
           cpu: "400m"
        limits:
          memory: "100Mi"
          cpu: "400m"
    raidGroups:
    - blockDevices:
      - blockDeviceName: sparse-37a7de580322f43a13338bf2467343f5
      - blockDeviceName: sparse-5a92ced3e2ee21eac7b930f670b5eab5
      isWriteCache: false
      isSpare: false

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Checklist:

  • Fixes #
  • Labelled this PR & related issue with documentation tag
  • PR messages has document related information
  • Labelled this PR & related issue with breaking-changes tag
  • PR messages has breaking changes related information
  • Labelled this PR & related issue with requires-upgrade tag
  • PR messages has upgrade related information
  • Commit has unit tests
  • Commit has integration tests

Copy link
Contributor

@prateekpandey14 prateekpandey14 left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@sonasingh46 sonasingh46 left a comment

Choose a reason for hiding this comment

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

lgtm

@sonasingh46 sonasingh46 added the pr/release-note PR should be included in release notes label Dec 27, 2019
@@ -395,8 +395,15 @@ func getResourceRequirementForCStorPool(cspi *apis.CStorPoolInstance) *corev1.Re
return resourceRequirements
}

func getAuxResourceRequirement(cspi *apis.CStorPoolInstance) corev1.ResourceRequirements {
return cspi.Spec.AuxResources
Copy link
Contributor

Choose a reason for hiding this comment

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

are we trying NOT to use AuxResources of cspi.Spec? If so, better to remove from the struct itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getAuxResourceRequirement is used to populate the resourceRequirements for sidecar containers. The cspi is populated using either using the CSPC PoolConfig AuxResources or the DefaultAuxResources. If both of them are nil then an empty resourceRequirements needs to be set on the sidecar containers.

@vishnuitta vishnuitta merged commit a40a6ee into openebs-archive:master Dec 30, 2019
@kmova kmova added this to the 1.6.0 milestone Jan 2, 2020
@shubham14bajpai shubham14bajpai deleted the auxres branch March 17, 2020 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/release-note PR should be included in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants