From 76b5757cf0e2a6fd7d5ad2ed57ff659a59fda730 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 27 Nov 2018 13:00:10 -0800 Subject: [PATCH 1/3] data/data/aws/variables-aws: Drop tectonic_aws_worker_ec2_type, etc. The last consumers for these were removed by 124ac351 (*: Use machine-api-operator to deploy worker nodes, 2018-09-04, #119). --- data/data/aws/variables-aws.tf | 52 ---------------------------------- pkg/tfvars/aws/aws.go | 15 ++-------- pkg/tfvars/tfvars.go | 6 ---- 3 files changed, 2 insertions(+), 71 deletions(-) diff --git a/data/data/aws/variables-aws.tf b/data/data/aws/variables-aws.tf index 615e9a19045..8d86735f16c 100644 --- a/data/data/aws/variables-aws.tf +++ b/data/data/aws/variables-aws.tf @@ -13,12 +13,6 @@ variable "tectonic_aws_master_ec2_type" { default = "t2.medium" } -variable "tectonic_aws_worker_ec2_type" { - type = "string" - description = "Instance size for the worker node(s). Example: `t2.medium`." - default = "t2.medium" -} - variable "tectonic_aws_ec2_ami_override" { type = "string" description = "(optional) AMI override for all nodes. Example: `ami-foobar123`." @@ -36,17 +30,6 @@ EOF default = [] } -variable "tectonic_aws_worker_extra_sg_ids" { - description = < Date: Tue, 27 Nov 2018 13:20:39 -0800 Subject: [PATCH 2/3] data/data/config: Drop tectonic_worker_count And the related, libvirt-specific, tectonic_libvirt_worker_ips. This simplifies the Terraform logic for AWS and OpenStack, and consistently pushes most worker setup into the cluster-API providers who are creating the workers since 124ac351 (*: Use machine-api-operator to deploy worker nodes, 2018-09-04, #119). --- data/data/config.tf | 10 ---------- data/data/libvirt/main.tf | 7 ------- data/data/libvirt/variables-libvirt.tf | 5 ----- pkg/tfvars/libvirt/libvirt.go | 15 +-------------- pkg/tfvars/tfvars.go | 18 ++++++++---------- 5 files changed, 9 insertions(+), 46 deletions(-) diff --git a/data/data/config.tf b/data/data/config.tf index bbcc55449c2..a5d4f50c93f 100644 --- a/data/data/config.tf +++ b/data/data/config.tf @@ -12,16 +12,6 @@ This applies only to cloud platforms. EOF } -variable "tectonic_worker_count" { - type = "string" - default = "3" - - description = < 0 { - if len(l.WorkerIPs) != workerCount { - return fmt.Errorf("length of WorkerIPs doesn't match worker count") - } - } else { - if ips, err := generateIPs("worker", network, workerCount, 50); err == nil { - l.WorkerIPs = ips - } else { - return err - } - } - return nil } diff --git a/pkg/tfvars/tfvars.go b/pkg/tfvars/tfvars.go index bfbbddebf7d..d73f68be4a8 100644 --- a/pkg/tfvars/tfvars.go +++ b/pkg/tfvars/tfvars.go @@ -19,7 +19,6 @@ type config struct { Name string `json:"tectonic_cluster_name,omitempty"` BaseDomain string `json:"tectonic_base_domain,omitempty"` Masters int `json:"tectonic_master_count,omitempty"` - Workers int `json:"tectonic_worker_count,omitempty"` IgnitionBootstrap string `json:"ignition_bootstrap,omitempty"` IgnitionMaster string `json:"ignition_master,omitempty"` @@ -42,15 +41,15 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e } for _, m := range cfg.Machines { - var replicas int - if m.Replicas == nil { - replicas = 1 - } else { - replicas = int(*m.Replicas) - } - switch m.Name { case "master": + var replicas int + if m.Replicas == nil { + replicas = 1 + } else { + replicas = int(*m.Replicas) + } + config.Masters += replicas if m.Platform.AWS != nil { config.AWS.Master = aws.Master{ @@ -64,7 +63,6 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e } } case "worker": - config.Workers += replicas if m.Platform.AWS != nil { config.AWS.Worker = aws.Worker{ IAMRoleName: m.Platform.AWS.IAMRoleName, @@ -107,7 +105,7 @@ func TFVars(cfg *types.InstallConfig, bootstrapIgn, masterIgn string) ([]byte, e Image: cfg.Platform.Libvirt.DefaultMachinePlatform.Image, MasterIPs: masterIPs, } - if err := config.Libvirt.TFVars(config.Masters, config.Workers); err != nil { + if err := config.Libvirt.TFVars(config.Masters); err != nil { return nil, errors.Wrap(err, "failed to insert libvirt variables") } if err := config.Libvirt.UseCachedImage(); err != nil { From 8ed4d9f0e03e87bd010a59b3c453ec32f18695e2 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Mon, 26 Nov 2018 00:50:18 -0800 Subject: [PATCH 3/3] data/aws: Bump default instance type from t2.medium to t3.medium The T3 instances were launched in 2018-08-21 [1]. The documents stats are fairly similar to the T2 analogs [2,3]: Name t2.medium t3.medium vCPUs 2 2 Memory (GiB) 4.0 4.0 Baseline Performance/vCPU 20% CPU Credits earned/hr 24 24 Network burst bandwidth (Gbps) 5 EBS burst bandwidth (Gbps) 1.50 On-Demand Price/hr* $0.0464 $0.0418 1-yr Reserved Instance Effective Hourly* $0.025 3-yr Reserved Instance Effective Hourly* $0.017 * Prices shown are for Linux/Unix in US East (Northern Virginia) AWS Region. Prices for 1-year and 3-year reserved instances are for "Partial Upfront" payment options or "No upfront" for instances without the Partial Upfront option. For full pricing details, see the Amazon EC2 pricing page [4]. The only obvious difference there is that the T3 instances are 10% cheaper. However, from [1]: If the instance runs at higher CPU utilization for a prolonged period, there will be an additional charge of $0.05 per vCPU-hour. and from [2]: T3 instances start in Unlimited mode by default... If the instance needs to run at higher CPU utilization for a prolonged period, it can do so for an additional charge of $0.05 per vCPU-hour. In Standard mode, a T3 instance can burst until it uses up all its earned credits. The difference vs. T2 seems to be that with T2, Unlimited mode was opt-in while with T3 it is opt-out. You can set a credit specification in Terraform [5] to opt out, but there doesn't seem to be a setting in the cluster-API structures for this. And in any case, we expect: * Most users to be able to run clusters of up to ~15 workers without overextending an Unlimited t3.medium. * To have cluster alerts for when nodes have exhausted their CPU quota, which will tell cluster admins that it's time to rotate in some larger master nodes. We may already have this alerting, but I'm not sure where it lives. * To have CI be able to get through its e2e tests before exhausting the initial CPU credits. But we can monitor this with the above-mentioned alerting as well. The FIXME is because pkg/tfvars is consuming the install-config, so it doesn't know about configuration that landed further downstream in the asset graph's pkg/asset/machines. I have a partial fix for that, but Abhinav wanted to punt that to future work [6]. [1]: https://aws.amazon.com/blogs/aws/new-t3-instances-burstable-cost-effective-performance/ [2]: https://aws.amazon.com/ec2/instance-types/t2/ [3]: https://aws.amazon.com/ec2/instance-types/t3/ [4]: https://aws.amazon.com/ec2/pricing/ [5]: https://www.terraform.io/docs/providers/aws/r/instance.html#credit-specification [6]: https://github.com/openshift/installer/pull/724#issuecomment-442959144 --- data/data/aws/bootstrap/variables.tf | 2 +- data/data/aws/variables-aws.tf | 6 ++++-- pkg/asset/machines/worker.go | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/data/data/aws/bootstrap/variables.tf b/data/data/aws/bootstrap/variables.tf index ab01df1a313..11b23f116b5 100644 --- a/data/data/aws/bootstrap/variables.tf +++ b/data/data/aws/bootstrap/variables.tf @@ -26,7 +26,7 @@ variable "ignition" { variable "instance_type" { type = "string" - default = "t2.medium" + default = "t3.medium" description = "The EC2 instance type for the bootstrap node." } diff --git a/data/data/aws/variables-aws.tf b/data/data/aws/variables-aws.tf index 8d86735f16c..365b038ec7c 100644 --- a/data/data/aws/variables-aws.tf +++ b/data/data/aws/variables-aws.tf @@ -9,8 +9,10 @@ EOF variable "tectonic_aws_master_ec2_type" { type = "string" - description = "Instance size for the master node(s). Example: `t2.medium`." - default = "t2.medium" + description = "Instance size for the master node(s). Example: `t3.medium`." + + # FIXME: get this wired up to the machine default + default = "t3.medium" } variable "tectonic_aws_ec2_ami_override" { diff --git a/pkg/asset/machines/worker.go b/pkg/asset/machines/worker.go index 2c916699bf1..1050f1915eb 100644 --- a/pkg/asset/machines/worker.go +++ b/pkg/asset/machines/worker.go @@ -27,7 +27,7 @@ import ( func defaultAWSMachinePoolPlatform() awstypes.MachinePool { return awstypes.MachinePool{ - InstanceType: "t2.medium", + InstanceType: "t3.medium", } }