proposed design for handling disk objects by node-disk-manager - #1
Closed
kmova wants to merge 57 commits into
Closed
proposed design for handling disk objects by node-disk-manager#1kmova wants to merge 57 commits into
kmova wants to merge 57 commits into
Conversation
Signed-off-by: kmova <kiran.mova@openebs.io>
kmova
requested review from
ajeshbaby,
gila,
pawanpraka1,
satyamz and
umamukkara
March 7, 2018 09:05
gila
reviewed
Mar 7, 2018
| - Need to know about the different types of disks available in the Cluster | ||
| - Need to determine the physical location of the disks in the cluster in terms of Node, Rack, DataCenter/Zone | ||
| - Need to associate the required storage to OpenEBS Storage Pods, taking into consideration the capacity, qos and availability requirements | ||
| - Need immediate notifications on the faults on the storage disks and ability to perform online corrections |
There was a problem hiding this comment.
we also need to understand physical layout it terms of NUMA such that we can assign proper cores to the reactors, that's not on the top of the list right now though, but still we need something like this in the future
There was a problem hiding this comment.
Whats the format we are going to store in etcd?
| - OpenEBS Provisioner could then query for the PVs using `kubectl get pv` (filtered with local-provisioner type) to get information on all the disks in the Cluster. | ||
| - OpenEBS Provisioner can then create OpenEBS Storage Pods that make use of the PV. | ||
| - Have the Node-Bot monitor for faults on the PVs and send notification to the OpenEBS Storage Pods using them. The monitoring includes inforamtion like: | ||
| * gathering smart statistics of the disk |
There was a problem hiding this comment.
We also need to detect IO errors and the sense data (if any) that go with them -- all io errors should have sense data however.
| - OpenEBS Provisioner can then create OpenEBS Storage Pods that make use of the PV. | ||
| - Have the Node-Bot monitor for faults on the PVs and send notification to the OpenEBS Storage Pods using them. The monitoring includes inforamtion like: | ||
| * gathering smart statistics of the disk | ||
| * probing or listening for events generated by the underlying disk |
There was a problem hiding this comment.
Is this what you mean with sense data perhaps?
Contributor
Author
There was a problem hiding this comment.
yes @gila. I will modify the text to say sense data
kmova
commented
Mar 7, 2018
| * probing or listening for events generated by the underlying disk | ||
|
|
||
| However with this approach of using PV: | ||
| - Once Storage Pod is running, any of the following operations will require a Storage Pod Restart (TODO Kiran - link the issue here): |
Contributor
Author
There was a problem hiding this comment.
@gila : can a cStorPool restart be considered acceptable? There are lots of merits to using the default PVs instead of introducing the concept of PD.
There was a problem hiding this comment.
A restart will require a rebuild to all replica's and, depending on the # of replicas will go to RO -- so IMHO we should be careful with this, right?
Contributor
Author
There was a problem hiding this comment.
thanks @gila I have changed the titles a bit to say that the proposed approach doesn't involve a restart of the pod.
|
User story around Node bot may be helpful. It can be a separate PR. |
umamukkara
previously approved these changes
Mar 7, 2018
umamukkara
left a comment
umamukkara
left a comment
There was a problem hiding this comment.
I would think the approach of keeping a separate disk related CRDs is scalable. Node bot will be in general useful for managing the storage resources of k8s, perhaps it can go to kubernetes incubation at a later date.
AmitKumarDas
reviewed
Mar 7, 2018
| - OpenEBS Provisioner can update the cStorPool with the new PDs for expansion or replacement | ||
|
|
||
| Drawback/Issues: | ||
| - The underlying disks used/allocaed to OpenEBS Storage Pods should not be assigned by Kuberentes (say via Local Provisioner) to different workloads/pods. |
There was a problem hiding this comment.
Can local Storage manager make use of PD? or PD make use of Local Storage Manager.
Contributor
Author
There was a problem hiding this comment.
Local Storage Manager is being by-passed here. Tried to add more details to provide clarity. Please check if I am missing any references to the Local Storage Manager
kmova
changed the title
kmova
changed the title
Signed-off-by: kmova <kiran.mova@openebs.io>
Suggesting a goal section for the design
initial repo checkin
Signed-off-by: kmova <kiran.mova@openebs.io>
Signed-off-by: kmova <kiran.mova@openebs.io>
Signed-off-by: Pawan <pawanprakash101@gmail.com>
Signed-off-by: Pawan <pawanprakash101@gmail.com>
Initial commit for node-disk-manager cli - ndmctl
|
It is cumbersome to apply more than one yaml to deploy a solution. For NDM, we are applying crd yaml and deployment yaml for deployment. Writing a complete yaml, which will do everyting like apply the CRD yaml, create a service account, apply the deployment yaml would be useful for users. Signed-off-by: Pawan <pawanprakash101@gmail.com>
…tegration Test File is implemented using BDD style with help of GinkGo. I AM FOCUSING ON MINIKUBE FOR NOW, WE SHALL MODIFY THE SCRIPT LATER AS IMPROVEMENTS. (Even in SETUP of the Suit, there is `minikube start`.) **Actions:** - Runs minikube with this command: `sudo minikube start --vm-driver=none`. - In YAML changes value of `image` to match with newly built image and `imagePullPolicy` to `IfNotPresent`. - Apply the YAML. - Check the log. - Runs `lsblk -brdno name,size,model` on host and `ndm device list` inside the pod and matches the results. Assumptions: - User of this file is a Super user. - `curl` is present in the system. - Yaml for NDM daemonset: 'node-disk-manager.yaml' is there in node-disk-manager project directory. - Docker image name is same as the output of `echo "openebs/node-disk-manager-$(if [ -z "$XC_ARCH" ]; then XC_ARCH=$(go env GOARCH); fi && echo $XC_ARCH)"`. - Docker image tag is same as the output of `echo "$(if [ -z "$TAG" ]; then TAG=$(git rev-parse --abbrev-ref HEAD); fi && echo $TAG)"`. - node-disk-manager pods runs under default namespace (k8s.io/api/core/v1.NamespaceDefault). - Pod name starts with string "node-disk-manager". - Cluster has only one node. (which is true for minikube). - There is only one node-disk-manager pod (which is true for minikube). - There is only one container in the node-disk-manager pod. - Environment variables `USER` and `HOME` is well defined. - GOPATH has only one path (no colons). - In output of `lsblk -brdno name,size,model` none of `name`, `size`, `model` have only white-spaces in them. - If 'Started the controller' is in log, log is considered OK. Note: - `lsblk` in Ubuntu-Trusty does not have column `serial`; so when matching output of `lsblk -brdno name,size,model` and `ndm device list`, **`serial` is ignored**. - Calling `ClearContainers` from package `minikubeadm` or `Clean` from package `ndmutil` **removes all the Docker Containers** from the machine. To run this test manually, use following command: `go test -v github.com/openebs/node-disk-manager/integration_test -run ^TestIntegrationNDM$` Changes committed: modified: .travis.yml modified: Makefile new file: integration_test/common/k8s_specific.go new file: integration_test/common/string_operations.go new file: integration_test/common/system_commands.go new file: integration_test/ginkgo_integration_test.go new file: integration_test/k8s_util/k8s_util.go new file: integration_test/k8s_util/ndm_specific.go new file: integration_test/minikube_adm/minikube_adm.go new file: integration_test/ndm_util/ndm_util.go modified: ndm-operator.yaml modified: samples/node-disk-manager.yaml Signed-off-by: Abhishek Kashyap <abhishek.kashyap@mayadata.io>
Changes committed: modified: Gopkg.lock modified: Gopkg.toml new file: vendor/github.com/docker new file: vendor/github.com/onsi/ginkgo new file: vendor/github.com/onsi/gomega Signed-off-by: Abhishek Kashyap <abhishek.kashyap@mayadata.io>
Signed-off-by: prateekpandey14 <prateekpandey14@gmail.com>
Changes committed: - modified: .gitignore - modified: .travis.yml - modified: Makefile - new file: hack/test.sh Signed-off-by: Shovan <shovan.cse91@gmail.com>
Refactored the code to be more modular to extend with additional disk probes and to easily add unit test cases.
Couple of folders have been refactored:
- cmd/controller
- pkg/udev
---
cmd/controller
├── controller
│ ├── ctrl.go
│ ├── ctrl_main.go
│ └── ctrl_test.go
has been refactored as:
cmd/controller
├── controller
│ ├── controller.go
│ ├── disk.go
│ ├── diskstore.go
│ └── probe.go
├── probe
│ ├── eventhandler.go
│ ├── probe.go
│ └── udevprobe.go
---
pkg/udev
├── udev
│ └── udev.go
has been refactored as:
pkg/udev
├── udev
│ ├── common.go
│ ├── device.go
│ ├── enumerate.go
│ ├── listentry.go
│ └── udev.go
---
Changes committed:
- modified: cmd/app/command/start.go
- new file: cmd/controller/controller.go
- new file: cmd/controller/disk.go
- new file: cmd/controller/diskstore.go
- new file: cmd/probe/eventhandler.go
- new file: cmd/probe/probe.go
- new file: cmd/probe/udevprobe.go
- new file: pkg/udev/common.go
- new file: pkg/udev/device.go
Unit tests for this PR.
Changes committed:
- new file: cmd/controller/controller_test.go
- new file: cmd/controller/disk_test.go
- new file: cmd/controller/diskstore_test.go
- new file: cmd/controller/mockdata_test.go
- new file: cmd/controller/probe_test.go
- new file: cmd/probe/eventhandler_test.go
- new file: cmd/probe/probe_test.go
- new file: cmd/probe/udevprobe_test.go
- modified: integration_test/ginkgo_integration_test.go
- modified: integration_test/ndm_util/ndm_util.go
- new file: pkg/udev/common_test.go
- new file: pkg/udev/device_test.go
- new file: pkg/udev/enumerate_test.go
- modified: pkg/udev/mockdata.go
- new file: pkg/udev/udev_test.go
- new file: pkg/util/util_test.go
Signed-off-by: Shovan <shovan.cse91@gmail.com>
Changes committed: modified: README.md Signed-off-by: sagarkrsd <sagar.kumar@openebs.io>
… update disk cr (#47) * libudev monitor for detect attach detach disk Node-disk-manager will automate the management of disk attached to the node. To automate management of disks ndm should be notified by udev-event module for two basic event disk attach and disk detach. Node-disk-manager will detect disks attached to the node and create a Disk CR corresponding to the newly detected disk. In case this disk was moved from one node to some other node of same cluster, it should update the host-name associated with the disk. libudev API calls will help to detect attach and detach event and get details info about attached or detached disk node-disk-manager will process event and update disk cr in etcd. This PR also contain Unit tests, some changes of methods name in udev pkg and better-code-hub badge added in README. Signed-off-by: Shovan Maity <shovan.cse91@gmail.com>
This PR removes ndm device list cli command output from pkg/controller. Now it prints using defaultDeviceList template. Changes committed: modified: cmd/app/command/device-list.go modified: cmd/controller/controller.go Signed-off-by: Shovan <shovan.cse91@gmail.com>
Signed-off-by: sagarkrsd <sagar.kumar@openebs.io>
For each disk by default by-id and by-path links are created. by-id, by-path links are like - /dev/disk/by-id/scsi-0Google_PersistentDisk_demo-disk /dev/disk/by-path/virtio-pci-0000:00:03.0-scsi-0:0:4:0 these are added in Details of Disk cr's spec. Signed-off-by: Shovan Maity <shovan.maity@mayadata.io>
This PR arrange them below order- 1. by-id link which contain serail no and bus 2. others links Changes committed: modified: pkg/udev/common.go Signed-off-by: Shovan Maity <shovan.maity@mayadata.io>
Changes committed: modified: Gopkg.lock modified: Gopkg.toml modified: vendor Signed-off-by: Abhishek Kashyap <abhishek.kashyap@mayadata.io> Signed-off-by: Akash Srivastava <akash.srivastava@openebs.io> Co-authored-by: Akash Srivastava <akash.srivastava@openebs.io>
Changes committed: new file: integration_test/common_resource/common_resource.go This package will hold all the common variables and constants so that all other packages can use them. Since this package is intended to be used by all other package, we should keep this on the top of the package hierarchy to avoid cyclic dependency. new file: integration_test/ndm_util/ndm_util_test.go This file contains unit-test of integration_test/ndm_util/ndm_util.go modified: integration_test/ginkgo_integration_test.go modified: integration_test/k8s_util/ndm_specific.go modified: integration_test/ndm_util/ndm_util.go These files are modified to use CITF for specific operations and common_resource package for common thing e.g. CitfInstance. deleted: integration_test/common/k8s_specific.go deleted: integration_test/common/string_operations.go deleted: integration_test/common/system_commands.go deleted: integration_test/k8s_util/k8s_util.go deleted: integration_test/minikube_adm/minikube_adm.go These files are no longer used because the functionality these files used to serve are now served by CITF in even better manner. Signed-off-by: Abhishek Kashyap <abhishek.kashyap@mayadata.io> Signed-off-by: Akash Srivastava <akash.srivastava@openebs.io> Co-authored-by: Akash Srivastava <akash.srivastava@openebs.io>
…tes (#58) This code provides the foundation for working on fetching the SMART parameters from the disks. As part of this initial implementation NDM will use the SCSI/ATA interfaces to fetch basic device attributes like - -vendor,model,serial number,version, logical sector size, firmware revision, capacity, etc. Signed-off-by: sagarkrsd <sagar.kumar@openebs.io>
Implementation steps :- 1. read /proc/self/mounts and find which partition is mounted on / path (ie - sda2) 2. read link /sys/class/block/[partition-name] ie - /sys/class/block/sda2 this link looks like "/sys/devices/pci0000:00/0000:00:1f.2/ata1/host0 /target0:0:0/0:0:0:0/block/sda/sda2" read / seperated entry after "block" that is os disk name 3. we need to mount /proc/self/mounts inside container if we use self it points to currently running process so content of that file getting changed for that we need to mount /proc/1/mounts this is the proc file of process 1 of host/node. 4. one new attribute IgnoreDiskUuid added in Controller struct at the time of event processing if the uuid of the disk is present in IgnoreDiskUuid then that event will not processed by event handler. Changes committed: modified: cmd/app/command/start.go modified: cmd/controller/controller.go new file: cmd/controller/filter.go modified: cmd/controller/probe.go new file: cmd/filter/filter.go new file: cmd/filter/mountutil.go new file: cmd/filter/osdiskfilter.go modified: cmd/probe/eventhandler.go modified: cmd/probe/probe.go modified: cmd/probe/udevprobe.go modified: ndm-operator.yaml modified: pkg/udev/common.go modified: samples/node-disk-manager.yaml Signed-off-by: Shovan <shovan.cse91@gmail.com>
Signed-off-by: Shovan Maity <shovan.maity@mayadata.io>
This PR contains - 1. Probe/Filter Config logic via configmap 2. Filter using vendor name 3. Some typo fix node-disk-manager-config is the configmap which will use to config probes and filters of ndm. If that configmap is not present node-disk-manager loads default config for Probes and Filters. Signed-off-by: Shovan <shovan.cse91@gmail.com>
Signed-off-by: kmova <kiran.mova@openebs.io>
Signed-off-by: kmova <kiran.mova@openebs.io>
Signed-off-by: kmova <kiran.mova@openebs.io>
Signed-off-by: kmova <kiran.mova@openebs.io>
Signed-off-by: kmova <kiran.mova@openebs.io>
Codecov Report
@@ Coverage Diff @@
## master #1 +/- ##
=========================================
Coverage ? 54.48%
=========================================
Files ? 37
Lines ? 1360
Branches ? 0
=========================================
Hits ? 741
Misses ? 566
Partials ? 53
Continue to review full report at Codecov.
|
kmova
pushed a commit
that referenced
this pull request
Aug 7, 2018
Rebased the design doc on latest changes. This PR will replace #1 Updated README with: - Instructions to launch ndm and query for disks via kubectl - Build and test instructions Signed-off-by: kmova <kiran.mova@mayadata.io>
Codecov Report
@@ Coverage Diff @@
## master #1 +/- ##
=======================================
Coverage ? 51%
=======================================
Files ? 55
Lines ? 2637
Branches ? 0
=======================================
Hits ? 1345
Misses ? 1182
Partials ? 110
Continue to review full report at Codecov.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Signed-off-by: kmova kiran.mova@openebs.io
This PR is intended to cover the requirements and high level design of node-disk-manager. The node-disk-manager extends the capabilities of the Kubernetes to manage the storage (disks) attached to a node.
Refer: Design Presentation