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.

Sign up for GitHub

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

Jump to bottom

Rename CRD fields #118

Merged
merged 5 commits into from
Apr 3, 2024
Merged

Rename CRD fields #118

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e6df3b
rename spec.podDisruptionBudget to podDisruptionBudgetTemplate
sircthulhu Apr 3, 2024
38ad902
move spec.podSpec to spec.podTemplate.spec
sircthulhu Apr 3, 2024
fdf808c
move .spec.podTemplate.spec.extraArgs to .spec.options
sircthulhu Apr 3, 2024
d35883d
update samples and crds to match the new structure
sircthulhu Apr 3, 2024
9c10351
move startup probes to spec.podTemplate.spec
sircthulhu Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions api/v1alpha1/etcdcluster_types.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop

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

Loading