-
Notifications
You must be signed in to change notification settings - Fork 5
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
set default values for Instance Type for AWS CP and Worker nodes #485
Comments
Please checkout d2iq-labs/cluster-api-runtime-extensions-nutanix@main...shalin/default_instance_type for an implementation that we discussed. Currently AWS nodespec variable is shared between CP and worker node, that makes it difficult to set a single default in Json variable schema However the way the current API type is defined and JsonSchema is built it requires code changes in multiple places or need helper functions. I personally prefer some other solution than implemented in this PR. Alternatives:
|
We had a discussion today. We decided to implement this with a mutating webhook which will add defaults to the cluster variables. |
Moved d2iq-labs#51 Addressed the pending review comments **What problem does this PR solve?:** - sets defaults for aws nodes CP: m5.xlarge and Workers: m5.2xlarge - sets PLACEHOLDER value as defaults in AWSMachineTemplates because instanceType field is required by CAPA. - sets instance type as required field in the schema since CAREN will be using the default if not provided by the user. update existing unit tests to use default schema **NOTE** Once this PR is finalized, I will create follow up PR to set instance profile and some other changes in the API types that would ensure that it creates variable schema from the object instead of creating new one. **Which issue(s) this PR fixes:** Fixes # #485 **How Has This Been Tested?:** Tried to test locally using make e2e-test E2E_LABEL='provider:AWS && cni:Cilium' but local environment timed outs during initializations. Looking into fixing local environment (colima +docker +kind on mac) Tested manually be creating AWS cluster. aws-quick-start cluster class has following ``` instanceType: default: m5.xlarge description: The AWS instance type to use for the cluster Machines type: string required: - instanceType ``` and workers ``` instanceType: default: m5.2xlarge description: The AWS instance type to use for the cluster Machines type: string required: - instanceType ``` Created AWS cluster without adding instanceType variable, CAREN patched AWSMachineTemplate with default instanceType. ** Special notes for your reviewer:** The approach taken in this PR will be used as guide to set default for other variables.
InstanceType is a mandatory field in AWSMachineTemplate. Currently we set respective defaults CP: "m5.xlarge" and workers: "m5.2xlarge" in the default cluster class templates.
We should move it to the CAREN API so that the default values are discoverable to the CAREN Clients.
The text was updated successfully, but these errors were encountered: