Merge pull request kubernetes#6121 from kisieland/prov-req-address-co…

…mments

Address comments from kubernetes#6104

cluster-autoscaler/config/crd/autoscaling.x-k8s.io_provisioningrequests.yaml

@@ -43,13 +43,20 @@ spec:
             type: object
           spec:
             description: 'Spec contains specification of the ProvisioningRequest object.
-              More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status.'
+              More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status.
+              The spec is immutable, to make changes to the request users are expected
+              to delete an existing and create a new object with the corrected fields.'
             properties:
-              additionalParameters:
+              parameters:
                 additionalProperties:
+                  description: Parameter is limited to 255 characters.
+                  maxLength: 255
                   type: string
-                description: AdditionalParameters contains all other parameters custom
-                  classes may require.
+                description: Parameters contains all other parameters classes may
+                  require. 'atomic-scale-up.kubernetes.io' supports 'ValidUntilSeconds'
+                  parameter, which should contain a string denoting duration for which
+                  we should retry (measured since creation fo the CR).
+                maxProperties: 100
                 type: object
                 x-kubernetes-validations:
                 - message: Value is immutable
@@ -65,7 +72,6 @@ spec:
                       description: Count contains the number of pods that will be
                         created with a given template.
                       format: int32
-                      maximum: 16384
                       minimum: 1
                       type: integer
                     podTemplateRef:
@@ -79,6 +85,8 @@ spec:
                         name:
                           description: 'Name of the referenced object. More info:
                             https://kubernetes.io/docs/concepts/overview/working-with-objects/names#names'
+                          maxLength: 253
+                          pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                           type: string
                       type: object
                   required:
@@ -91,32 +99,38 @@ spec:
                 x-kubernetes-validations:
                 - message: Value is immutable
                   rule: self == oldSelf
-              provisioningClass:
-                description: 'ProvisioningClass describes the different modes of provisioning
-                  the resources. Supported values: * check-capacity.kubernetes.io
+              provisioningClassName:
+                description: 'ProvisioningClassName describes the different modes
+                  of provisioning the resources. Currently there is no support for
+                  ''ProvisioningClass'' objects. Supported values: * check-capacity.kubernetes.io
                   - check if current cluster state can fullfil this request, do not
-                  reserve the capacity. * atomic-scale-up.kubernetes.io - provision
-                  the resources in an atomic manner * ... - potential other classes
-                  that are specific to the cloud providers'
+                  reserve the capacity. Users should provide a reference to a valid
+                  PodTemplate object. CA will check if there is enough capacity in
+                  cluster to fulfill the request and put the answer in ''CapacityAvailable''
+                  condition. * atomic-scale-up.kubernetes.io - provision the resources
+                  in an atomic manner. Users should provide a reference to a valid
+                  PodTemplate object. CA will try to create the VMs in an atomic manner,
+                  clean any partially provisioned VMs and re-try the operation in
+                  a exponential back-off manner. Users can configure the timeout duration
+                  after which the request will fail by ''ValidUntilSeconds'' key in
+                  ''Parameters''. CA will set ''Failed=true'' or ''Provisioned=true''
+                  condition according to the outcome. * ... - potential other classes
+                  that are specific to the cloud providers. ''kubernetes.io'' suffix
+                  is reserved for the modes defined in Kubernetes projects.'
+                maxLength: 253
+                pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$
                 type: string
                 x-kubernetes-validations:
                 - message: Value is immutable
                   rule: self == oldSelf
             required:
             - podSets
-            - provisioningClass
+            - provisioningClassName
             type: object
           status:
             description: Status of the ProvisioningRequest. CA constantly reconciles
               this field.
             properties:
-              additionalStatus:
-                additionalProperties:
-                  type: string
-                description: AdditionalStatus contains all other status values custom
-                  provisioning classes may require.
-                minProperties: 64
-                type: object
               conditions:
                 description: Conditions represent the observations of a Provisioning
                   Request's current state. Those will contain information whether
@@ -192,6 +206,15 @@ spec:
                 x-kubernetes-list-map-keys:
                 - type
                 x-kubernetes-list-type: map
+              provisioningClassDetails:
+                additionalProperties:
+                  description: Detail is limited to 32768 characters.
+                  maxLength: 32768
+                  type: string
+                description: ProvisioningClassDetails contains all other values custom
+                  provisioning classes may want to pass to end users.
+                maxProperties: 64
+                type: object
             type: object
         required:
         - spec

cluster-autoscaler/hack/update-codegen.sh

@@ -30,8 +30,8 @@ chmod +x "${CODEGEN_PKG}"/generate-groups.sh
 chmod +x "${CODEGEN_PKG}"/generate-internal-groups.sh
 
 bash "${CODEGEN_PKG}"/generate-groups.sh "applyconfiguration,client,deepcopy,informer,lister" \
-  k8s.io/autoscaler/cluster-autoscaler/provisioningrequests/client \
-  k8s.io/autoscaler/cluster-autoscaler/provisioningrequests/apis \
+  k8s.io/autoscaler/cluster-autoscaler/provisioningrequest/client \
+  k8s.io/autoscaler/cluster-autoscaler/provisioningrequest/apis \
   autoscaling.x-k8s.io:v1beta1 \
   --go-header-file "${SCRIPT_ROOT}"/../hack/boilerplate/boilerplate.generatego.txt
 

cluster-autoscaler/proposals/provisioning-request.md

@@ -97,11 +97,11 @@ type ProvisioningRequestSpec struct {
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	ProvisioningClass string `json:"provisioningClass"`
 
-	// AdditionalParameters contains all other parameters custom classes may require.
+	// Parameters contains all other parameters custom classes may require.
 	//
 	// +optional
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	AdditionalParameters map[string]string `json:"additionalParameters"`
+	Parameters map[string]string `json:"Parameters"`
 }
 
 type PodSet struct {
@@ -142,11 +142,11 @@ type ProvisioningRequestStatus struct {
     // +optional
 	Conditions []metav1.Condition `json:"conditions"`
 
-	// AdditionalStatus contains all other status values custom provisioning classes may require.
+	// Statuses contains all other status values custom provisioning classes may require.
 	//
 	// +optional
 	// +kubebuilder:validation:MaxItems=64
-	AdditionalStatus map[string]string `json:"additionalStatus"`
+	Statuses map[string]string `json:"statuses"`
 }
 ```
 
@@ -168,7 +168,7 @@ specified pods in an atomic way. The proposed logic is to:
 1. Try to provision required VMs in one loop.
 2. If it failed, remove the partially provisioned VMs and back-off.
 3. Stop the back-off after a given duration (optional), which would be passed
-   via `AdditionalParameters` field, using `ValidUntilSeconds` key and would contain string
+   via `Parameters` field, using `ValidUntilSeconds` key and would contain string
    denoting duration for which we should retry (measured since creation fo the CR).
 
 Note: that the VMs created in this mode are subject to the scale-down logic.
@@ -318,9 +318,9 @@ type ProvisioningClass struct {
 	// +kubebuilder:validation:Required
 	Name string `json:"name"`
 
-	// AdditionalParameters contains all other parameters custom classes may require.
+	// Parameters contains all other parameters custom classes may require.
 	//
 	// +optional
-	AdditionalParameters map[string]string `json:"additionalParameters"`
+	Parameters map[string]string `json:"Parameters"`
 }
 ```

cluster-autoscaler/provisioningrequests/apis/autoscaling.x-k8s.io/v1beta1/types.go → cluster-autoscaler/provisioningrequest/apis/autoscaling.x-k8s.io/v1beta1/types.go

@@ -46,6 +46,8 @@ type ProvisioningRequest struct {
 	metav1.ObjectMeta `json:"metadata,omitempty"`
 	// Spec contains specification of the ProvisioningRequest object.
 	// More info: https://git.k8s.io/community/contributors/devel/api-conventions.md#spec-and-status.
+	// The spec is immutable, to make changes to the request users are expected to delete an existing
+	// and create a new object with the corrected fields.
 	//
 	// +kubebuilder:validation:Required
 	Spec ProvisioningRequestSpec `json:"spec"`
@@ -81,24 +83,42 @@ type ProvisioningRequestSpec struct {
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
 	PodSets []PodSet `json:"podSets"`
 
-	// ProvisioningClass describes the different modes of provisioning the resources.
+	// ProvisioningClassName describes the different modes of provisioning the resources.
+	// Currently there is no support for 'ProvisioningClass' objects.
 	// Supported values:
 	// * check-capacity.kubernetes.io - check if current cluster state can fullfil this request,
-	//   do not reserve the capacity.
-	// * atomic-scale-up.kubernetes.io - provision the resources in an atomic manner
-	// * ... - potential other classes that are specific to the cloud providers
+	//   do not reserve the capacity. Users should provide a reference to a valid PodTemplate object.
+	//   CA will check if there is enough capacity in cluster to fulfill the request and put
+	//   the answer in 'CapacityAvailable' condition.
+	// * atomic-scale-up.kubernetes.io - provision the resources in an atomic manner.
+	//   Users should provide a reference to a valid PodTemplate object.
+	//   CA will try to create the VMs in an atomic manner, clean any partially provisioned VMs
+	//   and re-try the operation in a exponential back-off manner. Users can configure the timeout
+	//   duration after which the request will fail by 'ValidUntilSeconds' key in 'Parameters'.
+	//   CA will set 'Failed=true' or 'Provisioned=true' condition according to the outcome.
+	// * ... - potential other classes that are specific to the cloud providers.
+	// 'kubernetes.io' suffix is reserved for the modes defined in Kubernetes projects.
 	//
 	// +kubebuilder:validation:Required
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	ProvisioningClass string `json:"provisioningClass"`
+	// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
+	// +kubebuilder:validation:MaxLength=253
+	ProvisioningClassName string `json:"provisioningClassName"`
 
-	// AdditionalParameters contains all other parameters custom classes may require.
+	// Parameters contains all other parameters classes may require.
+	// 'atomic-scale-up.kubernetes.io' supports 'ValidUntilSeconds' parameter, which should contain
+	//  a string denoting duration for which we should retry (measured since creation fo the CR).
 	//
 	// +optional
 	// +kubebuilder:validation:XValidation:rule="self == oldSelf",message="Value is immutable"
-	AdditionalParameters map[string]string `json:"additionalParameters"`
+	// +kubebuilder:validation:MaxProperties=100
+	Parameters map[string]Parameter `json:"parameters"`
 }
 
+// Parameter is limited to 255 characters.
+// +kubebuilder:validation:MaxLength=255
+type Parameter string
+
 // PodSet represents one group of pods for Provisioning Request to provision capacity.
 type PodSet struct {
 	// PodTemplateRef is a reference to a PodTemplate object that is representing pods
@@ -112,7 +132,6 @@ type PodSet struct {
 	// template.
 	//
 	// +kubebuilder:validation:Minimum=1
-	// +kubebuilder:validation:Maximum=16384
 	Count int32 `json:"count"`
 }
 
@@ -122,6 +141,8 @@ type Reference struct {
 	// More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names#names
 	//
 	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*$`
+	// +kubebuilder:validation:MaxLength=253
 	Name string `json:"name,omitempty"`
 }
 
@@ -139,13 +160,18 @@ type ProvisioningRequestStatus struct {
 	// +optional
 	Conditions []metav1.Condition `json:"conditions"`
 
-	// AdditionalStatus contains all other status values custom provisioning classes may require.
+	// ProvisioningClassDetails contains all other values custom provisioning classes may
+	// want to pass to end users.
 	//
 	// +optional
-	// +kubebuilder:validation:MinProperties=64
-	AdditionalStatus map[string]string `json:"additionalStatus"`
+	// +kubebuilder:validation:MaxProperties=64
+	ProvisioningClassDetails map[string]Detail `json:"provisioningClassDetails"`
 }
 
+// Detail is limited to 32768 characters.
+// +kubebuilder:validation:MaxLength=32768
+type Detail string
+
 // The following constants list all currently available Conditions Type values.
 // See: https://pkg.go.dev/k8s.io/apimachinery/pkg/apis/meta/v1#Condition
 const (