Skip to content

Commit

Permalink
Rename CRD fields (#118)
Browse files Browse the repository at this point in the history
  • Loading branch information
@sircthulhu
sircthulhu committed Apr 3, 2024
2 parents 659291d + 9c10351 commit 81c12c3
Show file tree
Hide file tree
Showing 12 changed files with 3,376 additions and 2,821 deletions.
30 changes: 19 additions & 11 deletions api/v1alpha1/etcdcluster_types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@ type EtcdClusterSpec struct {
// +kubebuilder:default:=3
// +kubebuilder:validation:Minimum:=0
Replicas *int32 `json:"replicas,omitempty"`
// PodSpec defines the desired state of PodSpec for etcd members. If not specified, default values will be used.
PodSpec PodSpec `json:"podSpec,omitempty"`
// PodDisruptionBudget describes PDB resource to create for etcd cluster members. Nil to disable.
// Options are the extra arguments to pass to the etcd container.
// +optional
// +kubebuilder:example:={enable-v2: "false", debug: "true"}
Options map[string]string `json:"options,omitempty"`
// PodTemplate defines the desired state of PodSpec for etcd members. If not specified, default values will be used.
PodTemplate PodTemplate `json:"podTemplate,omitempty"`
// PodDisruptionBudgetTemplate describes PDB resource to create for etcd cluster members. Nil to disable.
//+optional
PodDisruptionBudget *EmbeddedPodDisruptionBudget `json:"podDisruptionBudget,omitempty"`
Storage StorageSpec `json:"storage"`
PodDisruptionBudgetTemplate *EmbeddedPodDisruptionBudget `json:"podDisruptionBudgetTemplate,omitempty"`
Storage StorageSpec `json:"storage"`
}

const (
Expand Down Expand Up @@ -121,6 +125,16 @@ type EmbeddedObjectMetadata struct {
Annotations map[string]string `json:"annotations,omitempty" protobuf:"bytes,12,rep,name=annotations"`
}

type PodTemplate struct {
// EmbeddedObjectMetadata contains metadata relevant to an EmbeddedResource
// +optional
EmbeddedObjectMetadata `json:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"`

// Spec defines the desired state of spec for etcd members. If not specified, default values will be used.
// +optional
Spec PodSpec `json:"spec,omitempty"`
}

// PodSpec defines the desired state of PodSpec for etcd members.
// +k8s:openapi-gen=true
type PodSpec struct {
Expand All @@ -136,9 +150,6 @@ type PodSpec struct {
// see https://kubernetes.io/docs/concepts/containers/images/#referring-to-an-imagepullsecrets-on-a-pod
// +optional
ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty"`
// PodMetadata contains metadata relevant to a PodSpec.
// +optional
PodMetadata *EmbeddedObjectMetadata `json:"metadata,omitempty"`
// Resources describes the compute resource requirements.
// +optional
Resources corev1.ResourceRequirements `json:"resources,omitempty"`
Expand Down Expand Up @@ -169,9 +180,6 @@ type PodSpec struct {
// RuntimeClassName refers to a RuntimeClass object in the node.k8s.io group, which should be used to run this pod.
// +optional
RuntimeClassName *string `json:"runtimeClassName,omitempty"`
// ExtraArgs are the extra arguments to pass to the etcd container.
// +optional
ExtraArgs map[string]string `json:"extraArgs,omitempty"`
// ExtraEnv are the extra environment variables to pass to the etcd container.
// +optional
ExtraEnv []corev1.EnvVar `json:"extraEnv,omitempty"`
Expand Down
37 changes: 18 additions & 19 deletions api/v1alpha1/etcdcluster_webhook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ var _ webhook.Defaulter = &EtcdCluster{}
// Default implements webhook.Defaulter so a webhook will be registered for the type
func (r *EtcdCluster) Default() {
etcdclusterlog.Info("default", "name", r.Name)
if len(r.Spec.PodSpec.Image) == 0 {
r.Spec.PodSpec.Image = defaultEtcdImage
if len(r.Spec.PodTemplate.Spec.Image) == 0 {
r.Spec.PodTemplate.Spec.Image = defaultEtcdImage
}
if r.Spec.Storage.EmptyDir == nil {
if len(r.Spec.Storage.VolumeClaimTemplate.Spec.AccessModes) == 0 {
Expand Down Expand Up @@ -83,11 +83,11 @@ func (r *EtcdCluster) ValidateCreate() (admission.Warnings, error) {
allErrors = append(allErrors, pdbErr...)
}

if errExtraArgs := validateExtraArgs(r); errExtraArgs != nil {
if errOptions := validateOptions(r); errOptions != nil {
allErrors = append(allErrors, field.Invalid(
field.NewPath("spec", "podSpec", "extraArgs"),
r.Spec.PodSpec.ExtraArgs,
errExtraArgs.Error()))
field.NewPath("spec", "options"),
r.Spec.Options,
errOptions.Error()))
}

if len(allErrors) > 0 {
Expand Down Expand Up @@ -127,11 +127,11 @@ func (r *EtcdCluster) ValidateUpdate(old runtime.Object) (admission.Warnings, er
warnings = append(warnings, pdbWarnings...)
}

if errExtraArgs := validateExtraArgs(r); errExtraArgs != nil {
if errOptions := validateOptions(r); errOptions != nil {
allErrors = append(allErrors, field.Invalid(
field.NewPath("spec", "podSpec", "extraArgs"),
r.Spec.PodSpec.ExtraArgs,
errExtraArgs.Error()))
field.NewPath("spec", "options"),
r.Spec.Options,
errOptions.Error()))
}

if len(allErrors) > 0 {
Expand All @@ -152,10 +152,10 @@ func (r *EtcdCluster) ValidateDelete() (admission.Warnings, error) {

// validatePdb validates PDB fields
func (r *EtcdCluster) validatePdb() (admission.Warnings, field.ErrorList) {
if r.Spec.PodDisruptionBudget == nil {
if r.Spec.PodDisruptionBudgetTemplate == nil {
return nil, nil
}
pdb := r.Spec.PodDisruptionBudget
pdb := r.Spec.PodDisruptionBudgetTemplate
var warnings admission.Warnings
var allErrors field.ErrorList

Expand Down Expand Up @@ -244,8 +244,8 @@ func (r *EtcdCluster) validatePdb() (admission.Warnings, field.ErrorList) {
return warnings, nil
}

func validateExtraArgs(cluster *EtcdCluster) error {
if len(cluster.Spec.PodSpec.ExtraArgs) == 0 {
func validateOptions(cluster *EtcdCluster) error {
if len(cluster.Spec.Options) == 0 {
return nil
}

Expand All @@ -262,17 +262,16 @@ func validateExtraArgs(cluster *EtcdCluster) error {

errlist := []error{}

for name := range cluster.Spec.PodSpec.ExtraArgs {
for name := range cluster.Spec.Options {
if strings.HasPrefix(name, "-") || strings.HasSuffix(name, "-") {
errlist = append(errlist, fmt.Errorf("Extra arg should not start with dash and have trailing dash, "+
"but it can contain dashes in the middle. flag: %s", name))
errlist = append(errlist, fmt.Errorf("options should not start with dashes or have trailing dashes. Flag: %s", name))

continue
}

if _, exists := systemflags[name]; exists {
errlist = append(errlist, fmt.Errorf("can't use extra argument '%s' in .Spec.PodSpec.ExtraArgs "+
"because it generated by operator for cluster setup", name))
errlist = append(errlist, fmt.Errorf("can't use extra argument '%s' in .spec.options "+
"because it is generated by the operator for cluster setup", name))
}
}

Expand Down
38 changes: 20 additions & 18 deletions api/v1alpha1/etcdcluster_webhook_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var _ = Describe("EtcdCluster Webhook", func() {
It("Should fill in the default value if a required field is empty", func() {
etcdCluster := &EtcdCluster{}
etcdCluster.Default()
Expect(etcdCluster.Spec.PodSpec.Image).To(Equal(defaultEtcdImage))
Expect(etcdCluster.Spec.PodTemplate.Spec.Image).To(Equal(defaultEtcdImage))
Expect(etcdCluster.Spec.Replicas).To(BeNil(), "User should have an opportunity to create cluster with 0 replicas")
Expect(etcdCluster.Spec.Storage.EmptyDir).To(BeNil())
storage := etcdCluster.Spec.Storage.VolumeClaimTemplate.Spec.Resources.Requests.Storage()
Expand All @@ -46,10 +46,12 @@ var _ = Describe("EtcdCluster Webhook", func() {
etcdCluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Replicas: ptr.To(int32(5)),
PodSpec: PodSpec{
Image: "myregistry.local/etcd:v1.1.1",
PodTemplate: PodTemplate{
Spec: PodSpec{
Image: "myregistry.local/etcd:v1.1.1",
},
},
PodDisruptionBudget: &EmbeddedPodDisruptionBudget{
PodDisruptionBudgetTemplate: &EmbeddedPodDisruptionBudget{
Spec: PodDisruptionBudgetSpec{
MaxUnavailable: ptr.To(intstr.FromInt32(int32(2))),
},
Expand All @@ -71,9 +73,9 @@ var _ = Describe("EtcdCluster Webhook", func() {
}
etcdCluster.Default()
Expect(*etcdCluster.Spec.Replicas).To(Equal(int32(5)))
Expect(etcdCluster.Spec.PodDisruptionBudget).NotTo(BeNil())
Expect(etcdCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable.IntValue()).To(Equal(2))
Expect(etcdCluster.Spec.PodSpec.Image).To(Equal("myregistry.local/etcd:v1.1.1"))
Expect(etcdCluster.Spec.PodDisruptionBudgetTemplate).NotTo(BeNil())
Expect(etcdCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable.IntValue()).To(Equal(2))
Expect(etcdCluster.Spec.PodTemplate.Spec.Image).To(Equal("myregistry.local/etcd:v1.1.1"))
Expect(etcdCluster.Spec.Storage.EmptyDir).To(BeNil())
storage := etcdCluster.Spec.Storage.VolumeClaimTemplate.Spec.Resources.Requests.Storage()
if Expect(storage).NotTo(BeNil()) {
Expand Down Expand Up @@ -137,8 +139,8 @@ var _ = Describe("EtcdCluster Webhook", func() {
Context("Validate PDB", func() {
etcdCluster := &EtcdCluster{
Spec: EtcdClusterSpec{
Replicas: ptr.To(int32(3)),
PodDisruptionBudget: &EmbeddedPodDisruptionBudget{},
Replicas: ptr.To(int32(3)),
PodDisruptionBudgetTemplate: &EmbeddedPodDisruptionBudget{},
},
}
It("Should admit enabled empty PDB", func() {
Expand All @@ -149,7 +151,7 @@ var _ = Describe("EtcdCluster Webhook", func() {
})
It("Should reject if negative spec.podDisruptionBudget.minAvailable", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.PodDisruptionBudget.Spec.MinAvailable = ptr.To(intstr.FromInt32(int32(-1)))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MinAvailable = ptr.To(intstr.FromInt32(int32(-1)))
_, err := localCluster.validatePdb()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
Expand All @@ -164,7 +166,7 @@ var _ = Describe("EtcdCluster Webhook", func() {
})
It("Should reject if negative spec.podDisruptionBudget.maxUnavailable", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable = ptr.To(intstr.FromInt32(int32(-1)))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable = ptr.To(intstr.FromInt32(int32(-1)))
_, err := localCluster.validatePdb()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
Expand All @@ -180,7 +182,7 @@ var _ = Describe("EtcdCluster Webhook", func() {
It("Should reject if min available field larger than replicas", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.Replicas = ptr.To(int32(1))
localCluster.Spec.PodDisruptionBudget.Spec.MinAvailable = ptr.To(intstr.FromInt32(int32(2)))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MinAvailable = ptr.To(intstr.FromInt32(int32(2)))
_, err := localCluster.validatePdb()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
Expand All @@ -196,7 +198,7 @@ var _ = Describe("EtcdCluster Webhook", func() {
It("Should reject if max unavailable field larger than replicas", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.Replicas = ptr.To(int32(1))
localCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable = ptr.To(intstr.FromInt32(int32(2)))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable = ptr.To(intstr.FromInt32(int32(2)))
_, err := localCluster.validatePdb()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
Expand All @@ -212,21 +214,21 @@ var _ = Describe("EtcdCluster Webhook", func() {
It("should accept correct percentage value for minAvailable", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.Replicas = ptr.To(int32(4))
localCluster.Spec.PodDisruptionBudget.Spec.MinAvailable = ptr.To(intstr.FromString("50%"))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MinAvailable = ptr.To(intstr.FromString("50%"))
warnings, err := localCluster.validatePdb()
Expect(err).To(BeNil())
Expect(warnings).To(ContainElement("current number of spec.podDisruptionBudget.minAvailable can lead to loss of quorum"))
})
It("should accept correct percentage value for maxUnavailable", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable = ptr.To(intstr.FromString("50%"))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable = ptr.To(intstr.FromString("50%"))
warnings, err := localCluster.validatePdb()
Expect(err).To(BeNil())
Expect(warnings).To(ContainElement("current number of spec.podDisruptionBudget.maxUnavailable can lead to loss of quorum"))
})
It("Should reject incorrect value for maxUnavailable", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable = ptr.To(intstr.FromString("50$"))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable = ptr.To(intstr.FromString("50$"))
_, err := localCluster.validatePdb()
if Expect(err).NotTo(BeNil()) {
expectedFieldErr := field.Invalid(
Expand All @@ -241,13 +243,13 @@ var _ = Describe("EtcdCluster Webhook", func() {
})
It("should correctly use zero numeric value for maxUnavailable PDB", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable = ptr.To(intstr.FromInt32(int32(0)))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable = ptr.To(intstr.FromInt32(int32(0)))
_, err := localCluster.validatePdb()
Expect(err).To(BeNil())
})
It("should correctly use zero string value for PDB", func() {
localCluster := etcdCluster.DeepCopy()
localCluster.Spec.PodDisruptionBudget.Spec.MaxUnavailable = ptr.To(intstr.FromString("0"))
localCluster.Spec.PodDisruptionBudgetTemplate.Spec.MaxUnavailable = ptr.To(intstr.FromString("0"))
_, err := localCluster.validatePdb()
Expect(err).To(BeNil())
})
Expand Down
42 changes: 27 additions & 15 deletions api/v1alpha1/zz_generated.deepcopy.go
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 81c12c3

Please sign in to comment.