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

Implement ".spec.storage" #49

Closed
kvaps opened this issue Mar 20, 2024 · 6 comments · Fixed by #67
Closed

Implement ".spec.storage" #49

kvaps opened this issue Mar 20, 2024 · 6 comments · Fixed by #67
Milestone

Comments

@kvaps
Copy link
Member

kvaps commented Mar 20, 2024

Internally we agreed to extend spec like this:

 ---
 apiVersion: etcd.aenix.io/v1alpha1
 kind: EtcdCluster
 metadata:
   name: test
   namespace: ns1
 spec:
   image: "quay.io/coreos/etcd:v3.5.12"
   replicas: 3
+  storage:
+    persistence: true # default: true, immutable
+    storageClass: local-path
+    size: 10Gi
 status:
   conditions:
   - lastProbeTime: null
 	lastTransitionTime: "2024-03-06T18:39:45Z"
 	status: "True"
 	type: Ready
@kvaps
Copy link
Member Author

kvaps commented Mar 20, 2024

There were also an idea of reuisng VolumeSource type.

But there were some concerns about security and stability, eg. user can use hostPath volumes and compromise the system.

Thus it's up to discussion.

@kvaps kvaps added this to the v0.0.2 milestone Mar 20, 2024
@Kirill-Garbar
Copy link
Collaborator

Kirill-Garbar commented Mar 20, 2024

I didn't get how it can be our problem if user defines hostPath. Usually k8s admins create policies (old PSP, kyverno, OPA gatekeeper) to prevent k8s users from creation unwanted (compromising) pod settings (privileged, hostNetwork, hostPath, etc
..)
If we want to help with it or we think that some types are not goot to use, we can add it to validation webhook - allow only some of the volume types. But again, I think it is not our decision to make for the user.

@kvaps
Copy link
Member Author

kvaps commented Mar 20, 2024

How about creating custom type, with selecting only specific methods we want to support?

Otherwise there might be potencial problems with supporting resizing for various backends (eg. NFS and iscsi). One more thing is that most of these methods are deprecated in favor CSI plugins

@sircthulhu
Copy link
Member

I think we should use spec.storage field from proposal #62

@gecube
Copy link
Collaborator

gecube commented Mar 22, 2024

I just understood that we can use storage field for zero downtime migration of etcd from one storage to another. @kvaps What do you think?

@kvaps
Copy link
Member Author

kvaps commented Mar 22, 2024

I think we should use spec.storage field from proposal #62

Agree

I just understood that we can use storage field for zero downtime migration of etcd from one storage to another.

Sounds like an idea for additional option.
The same about cleaninng up PVC after EtcdCluster get removed.

Just opened this question here
#67 (comment)

@sircthulhu sircthulhu self-assigned this Mar 23, 2024
sircthulhu added a commit that referenced this issue Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants