Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

rook-ceph: Allow user to provide sub-component resources #1483

Merged
merged 3 commits into from
Jun 18, 2021
Merged

rook-ceph: Allow user to provide sub-component resources #1483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19a0919
rook-ceph: Allow devs to specify custom test func
surajssd Jun 4, 2021
330ce9e
testutil: Add a new match function for JSON values
surajssd Jun 4, 2021
aafdcca
rook-ceph: Allow user to provide sub-component resources
surajssd May 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions docs/configuration-reference/components/rook-ceph.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,28 @@ component "rook-ceph" {
default = true
reclaim_policy = "Delete"
}
}

resources {
osd {
requests {
cpu = "5"
memory = "5Gi"
}

limits {
cpu = "5"
memory = "5Gi"
}
}

mon {}
mgr {}
mds {}
prepareosd {}
crashcollector {}
mgr_sidecar {}
}
}
```

The Ceph cluster needs to be deployed in the same namespace as the Rook operator at the moment.
Expand All @@ -65,14 +85,21 @@ Table of all the arguments accepted by the component.
| Argument | Description | Default | Type | Required |
|--------------------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|:--------:|:---------------------------------------------------------------------------------------------------------------|:--------:|
| `namespace` | Namespace to deploy the Ceph cluster into. Must be the same as the rook operator. | "rook" | string | false |
| `monitor_count` | Number of Ceph monitors to deploy. An odd number like 3 or 5 is recommended which should also be sufficient for most cases. | 1 | number | false |
| `monitor_count` | Number of Ceph monitors to deploy. An odd number like 3 or 5 is recommended which should also be sufficient for most cases. | 1 | number | false |
| `enable_toolbox` | Deploy the [toolbox pod](https://rook.io/docs/rook/master/ceph-toolbox.html) to debug and manage the Ceph cluster. | false | bool | false |
| `node_affinity` | Node affinity for deploying the Ceph cluster pods. | - | list(object({key = string, operator = string, values = list(string)})) | false |
| `toleration` | Tolerations that the Ceph cluster pods will tolerate. | - | list(object({key = string, effect = string, operator = string, value = string, toleration_seconds = string })) | false |
| `metadata_device` | Name of the device to store the metadata on each storage machine. **Note**: Provide just the name of the device and skip prefixing with `/dev/`. | - | string | false |
| `storage_class.enable` | Install Storage Class config. | false | bool | false |
| `storage_class.default` | Make this Storage Class as a default one. | false | bool | false |
| `node_affinity` | Node affinity for deploying the Ceph cluster pods. | - | list(object({key = string, operator = string, values = list(string)})) | false |
| `toleration` | Tolerations that the Ceph cluster pods will tolerate. | - | list(object({key = string, effect = string, operator = string, value = string, toleration_seconds = string })) | false |
| `metadata_device` | Name of the device to store the metadata on each storage machine. **Note**: Provide just the name of the device and skip prefixing with `/dev/`. | - | string | false |
| `storage_class.enable` | Install Storage Class config. | false | bool | false |
| `storage_class.default` | Make this Storage Class as a default one. | false | bool | false |
| `storage_class.reclaim_policy` | Persistent volumes created with this storage class will have this reclaim policy. This field decides what happens to the volume after a user deletes a PVC. Valid values: `Retain`, `Recycle` and `Delete`. Read more in the [Kubernetes docs](https://kubernetes.io/docs/concepts/storage/persistent-volumes/#reclaiming). | `Retain` | string | false |
| `resources.osd` | Resource request and/or limits for OSDs. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |
| `resources.mon` | Resource request and/or limits for MONs. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |
| `resources.mgr` | Resource request and/or limits for MGRs. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |
| `resources.mds` | Resource request and/or limits for MDS. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |
| `resources.prepareosd` | Resource request and/or limits for OSD prepare job. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |
| `resources.crashcollector` | Resource request and/or limits for Crashcollector. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |
| `resources.mgr_sidecar` | Resource request and/or limits for MGR sidecar. Read [this doc](https://rook.io/docs/rook/v1.6/ceph-cluster-crd.html#cluster-wide-resources-configuration-settings), to find out the minimum prescribed resource requirements. | - | object(request {cpu = string, memory = string}, limits {cpu = string, memory = string}) | false |

## Applying

Expand Down
24 changes: 24 additions & 0 deletions pkg/components/internal/testutil/jsonpath.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package testutil

import (
"encoding/json"
"fmt"
"reflect"
"strings"
Expand Down Expand Up @@ -103,6 +104,29 @@ func MatchJSONPathInt64Value(t *testing.T, yamlConfig string, jsonPath string, e
}
}

// MatchJSONPathJSONValue is a helper function for component unit tests. It compares the JSON values
Copy link
Member

Choose a reason for hiding this comment

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

Any new util functions that need not be visible should be part of the internal package

example this file : internal/util_test.go

Copy link
Member Author

Choose a reason for hiding this comment

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

But this pkg/components/internal/testutil/jsonpath.go has all the relevant code that deals with finding values at JSON path, converting into appropriate concrete data types and comparing values.

Copy link
Member Author

Choose a reason for hiding this comment

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

And it will be needed in various _test.go files, I don't fully understand what are you suggesting?

Copy link
Member

Choose a reason for hiding this comment

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

I'm suggesting to use the internal package for utility functions such as the one below

https://github.com/kinvolk/lokomotive/blob/master/internal/util_test.go

Copy link
Member Author

Choose a reason for hiding this comment

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

But the file that you are linking is a test code of the internal package. And internal package is consumed by anyone and everyone meanwhile the testutil is only consumed by tests. I still don't understand what is the rationale behind putting the test helper code away from the similar code?

// at a JSON path in a YAML config to the expected JSON string given by the user.
// e.g.
//
// spec:
// resources:
// osd: {"requests":{"cpu":"5","memory":"5Gi"},"limits":{"cpu":"5","memory":"5Gi"}}
func MatchJSONPathJSONValue(t *testing.T, yamlConfig string, jsonPath string, expected string) {
obj, err := jsonPathValue(yamlConfig, jsonPath)
if err != nil {
t.Fatalf("Extracting JSON path value: %v", err)
}

got, err := json.Marshal(obj)
if err != nil {
t.Fatalf("Marshalling JSON object: %v", err)
}

if string(got) != expected {
t.Fatalf("Expected: %s, Got: %s", expected, got)
}
}

// JSONPathExists checks if the given YAML config has an object at the given JSON path, also provide
// what error to expect.
func JSONPathExists(t *testing.T, yamlConfig string, jsonPath string, errExp string) {
Expand Down
63 changes: 63 additions & 0 deletions pkg/components/rook-ceph/component.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,27 @@ type component struct {
TolerationsRaw string
StorageClass *StorageClass `hcl:"storage_class,block"`
EnableToolbox bool `hcl:"enable_toolbox,optional"`

Resources *Resources `hcl:"resources,block"`
}

// Resources struct allows user to specify resource request and limits on the rook-ceph
// sub-components.
type Resources struct {
MON *util.ResourceRequirements `hcl:"mon,block"`
MONRaw string
MGR *util.ResourceRequirements `hcl:"mgr,block"`
MGRRaw string
OSD *util.ResourceRequirements `hcl:"osd,block"`
OSDRaw string
MDS *util.ResourceRequirements `hcl:"mds,block"`
MDSRaw string
PrepareOSD *util.ResourceRequirements `hcl:"prepareosd,block"`
PrepareOSDRaw string
CrashCollector *util.ResourceRequirements `hcl:"crashcollector,block"`
CrashCollectorRaw string
MGRSidecar *util.ResourceRequirements `hcl:"mgr_sidecar,block"`
MGRSidecarRaw string
}

// StorageClass provides struct to enable it or make it default.
Expand Down Expand Up @@ -69,6 +90,44 @@ func (c *component) LoadConfig(configBody *hcl.Body, evalContext *hcl.EvalContex
return gohcl.DecodeBody(*configBody, evalContext, c)
}

func (c *component) addResourceRequirements() error {
if c.Resources == nil {
return nil
}

var err error

if c.Resources.MONRaw, err = util.RenderResourceRequirements(c.Resources.MON); err != nil {
return fmt.Errorf("rendering resources.mon: %w", err)
}

if c.Resources.MGRRaw, err = util.RenderResourceRequirements(c.Resources.MGR); err != nil {
return fmt.Errorf("rendering resources.mgr: %w", err)
}

if c.Resources.OSDRaw, err = util.RenderResourceRequirements(c.Resources.OSD); err != nil {
return fmt.Errorf("rendering resources.osd: %w", err)
}

if c.Resources.MDSRaw, err = util.RenderResourceRequirements(c.Resources.MDS); err != nil {
return fmt.Errorf("rendering resources.mds: %w", err)
}

if c.Resources.PrepareOSDRaw, err = util.RenderResourceRequirements(c.Resources.PrepareOSD); err != nil {
return fmt.Errorf("rendering resources.prepareosd: %w", err)
}

if c.Resources.CrashCollectorRaw, err = util.RenderResourceRequirements(c.Resources.CrashCollector); err != nil {
return fmt.Errorf("rendering resources.crashcollector: %w", err)
}

if c.Resources.MGRSidecarRaw, err = util.RenderResourceRequirements(c.Resources.MGRSidecar); err != nil {
return fmt.Errorf("rendering resources.mgr_sidecar: %w", err)
}

return nil
}

// TODO: Convert to Helm chart.
func (c *component) RenderManifests() (map[string]string, error) {
// Generate YAML for Ceph cluster.
Expand All @@ -78,6 +137,10 @@ func (c *component) RenderManifests() (map[string]string, error) {
return nil, fmt.Errorf("rendering tolerations: %w", err)
}

if err := c.addResourceRequirements(); err != nil {
return nil, fmt.Errorf("rendering resources field: %w", err)
}

ret := make(map[string]string)

// Parse template with values
Expand Down
Loading