Open mateusoliveira43 opened 5 days ago
// Default BackupImages behavior when nil to true
func (dpa *DataProtectionApplication) BackupImages() bool {
return dpa.Spec.BackupImages == nil || *dpa.Spec.BackupImages
}
These were added because at the time we want to add configurability while hiding that these are defaulted to true since all users essentially had this on but we want to give the option to turn it off.
We didn't want to require DPA update to restore prior behavior.
If we had used CRD marker defaults, it would've added a new line to everyone's dpa, which while clearer was seen as "changing user's dpa", an undesired behavior.
Essentially, What You oc create -f
is what you oc get dpa <name> -oyaml
.
There are enough examples of controllers / CRDs displaying defaults to users' created CR, so the prior behavior was not a hard requirement.
Problem
OADP does not use default value markers for generating its CRDs
The adoption of this would be better for:
developers
Less code to write. Functions like this would not be necessary, as API would ensure a value is present on the field
https://github.com/openshift/oadp-operator/blob/fb2411e2948874a6abe61ca31b1cd9957621ebb6/api/v1alpha1/oadp_types.go#L614-L617
users
Default values would be more visible to users. Easier to see when default values are updated
Examples
If user creates a DPA with this spec
it would have this spec in the cluster
Upgrades would work with these changes. A sample DPA prior to upgrade
The same DPA after upgarde
References
https://book.kubebuilder.io/reference/markers/crd-validation https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#defaulting