-
Notifications
You must be signed in to change notification settings - Fork 47
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
Added requirements analysis and design proposal for Helm deployment for eSDK #38
base: master
Are you sure you want to change the base?
Conversation
- ClusterRoleBinding | ||
|
||
Sample: | ||
```text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cosmetic: text --> yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
High availability support of controller plugin will be part of future work. | ||
|
||
Sample: | ||
```text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cosmetic: text --> yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
- Keep username and password secure | ||
|
||
##### Other Non Functional Requirements (Scalability, HA etc…) | ||
- Image size should be be large. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "be" -> "not"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
- This folder contains samples with some examples of basic deployments | ||
|
||
### Helm repository | ||
<!-- - If time permits and required --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, If helm repository is not planned to support, Remove the section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
### Configuration Management | ||
Helm uses a packaging format called charts. A chart is a collection of files that describe a related set of Kubernetes | ||
resources. Charts are created as files laid out in a particular directory tree. They can be packaged into versioned | ||
archives to be deployed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cosmetic comment: "They can be packaged into versioned archives to be deployed." --> "The same directory tree can be archived and used for deployment"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
#### Charts.yaml | ||
|
||
```text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cosmetic: text --> yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
#### values.yaml | ||
The file maintains default values of different configurable attributes in templates files. | ||
|
||
```text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cosmetic: text --> yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
mountPath: /registration | ||
|
||
- name: huawei-csi-driver | ||
image: {{ required "Must provide the CSI controller service node image." .Values.images.huaweiCsiNodeService }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: "Must provide the CSI controller service node image." --> ""Must provide the CSI node service image.""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
``` | ||
|
||
##### huawei.csi.configmap.yaml | ||
```text |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cosmetic: text --> yaml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
https://rafay.co/the-kubernetes-current/helm-chart-hooks-tutorial/ | ||
|
||
|
||
**Open Points** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, open points can be discussed and resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the scratchpad section. The open points do need to be discussed and resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed sections which are not relevant.
//All raw inputs or discussion points or etc can be added here | ||
https://github.com/jsafrane/community/blob/master/contributors/design-proposals/storage/container-storage-interface.md#recommended-mechanism-for-deploying-csi-drivers-on-kubernetes | ||
|
||
#### Multiarchitecture images with docker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the section
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove. this is the scratchpad though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
|
||
https://medium.com/@artur.klauser/building-multi-architecture-docker-images-with-buildx-27d80f7e2408 | ||
|
||
#### Dell powermax CSI Helm deployment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, The section can be removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the comments.
|
||
- This document assumes Kubernetes as the orchestrator. | ||
- For the intitial design of the Helm deployment, the current state of the eSDK plugin will be considered. | ||
- Adapting the deployments of new features should be taken care by the feature owners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change :
"should be taken care" -> "should be taken care of"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
- This document assumes Kubernetes as the orchestrator. | ||
- For the intitial design of the Helm deployment, the current state of the eSDK plugin will be considered. | ||
- Adapting the deployments of new features should be taken care by the feature owners. | ||
- eSDK container images should be made available through image repository (local or remote) or should be available locally on the node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change :
"or should be available locally on the node" -> "locally on the node"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
- In this release no support for HA of controller plugin. | ||
- The design for the same will be proposed in a different design document. | ||
- Secret configuration management of backend storage | ||
- Currently the username and password are sensitive information for storage backend and are visible in plain text. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is it stored as of now in plain text?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was incorrect. The username and password are stored using the Kubernetes secret resource.
This has been fixed.
home: https://github.com/Huawei/eSDK_K8S_Plugin | ||
sources: | ||
- https://github.com/Huawei/eSDK_K8S_Plugin | ||
appVersion: "2.2.13"# The version of the app that this contains (optional). Quotes recommended. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will be configurable basically taken from ENV or input, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the moment we have considered this input manually. Every time there is a new release we need to updated the chart with the latest release number of the application, in this case eSDK release version.
We've already developed a Helm chart for deploying the Huawei CSI plugin in the past: https://github.com/adfinis-sygroup/helm-charts/tree/master/charts/huawei-csi-plugin Feel free to take whatever is required to improve your chart from there. I'll look into the current proposal in this PR and provide some feedback from our experience up to now. |
Thank you @tongpu .I will check it out. |
Added the requirements analysis and design for Helm deployment of eSDK plugins.
The implementation can be checked using #35