-
Notifications
You must be signed in to change notification settings - Fork 15
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
Basic EtcdCluster custom resource (#9) #15
Conversation
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.
LGTM. Let's allow for future updates to the storage spec, but for v1alpha1 this should be fine.
@sircthulhu What is the target artifact? Did you try to build it? What is the result? |
} | ||
|
||
// EtcdClusterSpec defines the desired state of EtcdCluster | ||
type EtcdClusterSpec struct { |
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.
I think about renaming this type EtcdClusterSpec ->Spec. I read package name and understand, that this code has context about EtcdCluster.
I see:
type EtcdCluster struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`
Spec Spec `json:"spec,omitempty"`
Status Status `json:"status,omitempty"`
}
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.
Package name is v1alpha1
. After adding other CRs there will be name conflicts.
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.
@Allliksey4 sorry, would it be convenient for you to write in English (no worries if errors/typos, nobody will blame)
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.
Thank you! I edited it
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
) | ||
|
||
// EDIT THIS FILE! THIS IS SCAFFOLDING FOR YOU TO OWN! |
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.
I guess we can remove this comment
|
||
var ( | ||
// GroupVersion is group version used to register these objects | ||
GroupVersion = schema.GroupVersion{Group: "etcd.aenix.io.etcd.aenix.io", Version: "v1alpha1"} |
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.
Stange group here. Maybe it should be just etcd.aenix.io
?
controller-gen.kubebuilder.io/version: v0.14.0 | ||
name: etcdclusters.etcd.aenix.io.etcd.aenix.io | ||
spec: | ||
group: etcd.aenix.io.etcd.aenix.io |
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.
Same strange group. It will be fixed after fix in source code
Fixes from comments #15 - Fix API group name for custom resources - remove unnecessary comment lines
Generated and wrote the most basic EtcdCluster custom resource.
See #9