From 66d2e4791de67985e382478220aeca4606ea5ce8 Mon Sep 17 00:00:00 2001 From: alok87 Date: Sun, 4 Dec 2016 16:31:03 +0530 Subject: [PATCH] IdleTimeout configurable from editcluster --- cmd/kops/create_cluster.go | 1 + docs/bastion.md | 29 ++++++++++++------- pkg/apis/kops/bastion.go | 2 ++ pkg/apis/kops/cluster.go | 3 ++ pkg/apis/kops/v1alpha1/bastion.go | 2 ++ pkg/apis/kops/v1alpha1/cluster.go | 3 ++ pkg/apis/kops/validation.go | 3 ++ .../topologies/_topology_private/bastion.yaml | 2 +- .../fi/cloudup/populate_instancegroup_spec.go | 17 +++++++++++ upup/pkg/fi/cloudup/populatecluster_test.go | 20 +++++++++++++ upup/pkg/fi/cloudup/template_functions.go | 9 ++++++ 11 files changed, 80 insertions(+), 11 deletions(-) diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go index c05f63733e..c8a62fb7e6 100644 --- a/cmd/kops/create_cluster.go +++ b/cmd/kops/create_cluster.go @@ -417,6 +417,7 @@ func RunCreateCluster(f *util.Factory, cmd *cobra.Command, args []string, out io return fmt.Errorf("Invalid topology %s.", c.Topology) } cluster.Spec.Topology.Bastion.MachineType = cloudup.DefaultBastionMachineType(cluster) + cluster.Spec.Topology.Bastion.IdleTimeout = cloudup.DefaultBastionIdleTimeout(cluster) sshPublicKeys := make(map[string][]byte) if c.SSHPublicKey != "" { diff --git a/docs/bastion.md b/docs/bastion.md index 18d5117f7d..18fbe1bdb5 100644 --- a/docs/bastion.md +++ b/docs/bastion.md @@ -13,11 +13,12 @@ Note: Bastion will get setup for the cluster(by default) only when `--topology=" Instance types in AWS comprise varying combinations of CPU, memory, storage, and networking capacity and give you the flexibility to choose the appropriate mix of resources for your applications. -- Bastion Instance type can be modified using `kops edit cluster` -- Defaults to `t2.medium` -#### TODO: add the example below for configuration +- **Defaults** to `t2.medium` +- **Configure:** Bastion Instance type can be modified using `kops edit cluster` ``` - +topology: + bastion: + MachineType: c4.large ``` [More information](https://aws.amazon.com/ec2/instance-types/) @@ -27,18 +28,25 @@ Instance types in AWS comprise varying combinations of CPU, memory, storage, and To turn on/off bastion host setup completely. - **Defaults** to `false` if the topology selected is `public` - **Defaults** to `true` if the topology selected is `private` - +- **Configure:** ``` - kops create cluster --bastion=[true|false] +kops create cluster --bastion=[true|false] +``` +OR using `kops edit cluster` +``` +topology: + bastion: + Enable: true ``` ### Reach bastion from outside of vpc using a name - **Default:** CNAME for the bastion is only created when the user explicitly define it using `kops edit cluster` - **Configure:** Bastion friendly CNAME can be configured using `kops edit cluster` -#### TODO: add the example below for configuration ``` - +topology: + bastion: + PublicName: jumper ``` ### High idle timeout for bastion ASG's ELB. (Configurable LoadBalancer Attributes) @@ -46,9 +54,10 @@ To turn on/off bastion host setup completely. By default, elastic load balancing sets the idle timeout to `60` seconds. - **Default:** Bastion ELB in kops will have `120` seconds as their default timeout. - **Configure:** This value can be configured using `kops edit cluster` -#### TODO: add the example below for configuration ``` - +topology: + bastion: + IdleTimeOut: 75 ``` [More information](http://docs.aws.amazon.com/elasticloadbalancing/latest/classic/config-idle-timeout.html) diff --git a/pkg/apis/kops/bastion.go b/pkg/apis/kops/bastion.go index 16f0916aef..5335aec373 100644 --- a/pkg/apis/kops/bastion.go +++ b/pkg/apis/kops/bastion.go @@ -23,4 +23,6 @@ type BastionSpec struct { Enable bool `json:"Enable,omitempty"` MachineType string `json:"MachineType,omitempty"` PublicName string `json:"Name,omitempty"` + // Bastion's Loadbalancer idle timeout + IdleTimeout int `json:"IdleTimeout,omitempty"` } diff --git a/pkg/apis/kops/cluster.go b/pkg/apis/kops/cluster.go index ca48461be2..aac2af7d40 100644 --- a/pkg/apis/kops/cluster.go +++ b/pkg/apis/kops/cluster.go @@ -554,3 +554,6 @@ func (c *Cluster) GetBastionMachineType() string { func (c *Cluster) GetBastionPublicName() string { return c.Spec.Topology.Bastion.PublicName } +func (c *Cluster) GetBastionIdleTimeout() int { + return c.Spec.Topology.Bastion.IdleTimeout +} diff --git a/pkg/apis/kops/v1alpha1/bastion.go b/pkg/apis/kops/v1alpha1/bastion.go index 22cfcd07a1..65ef66c14b 100644 --- a/pkg/apis/kops/v1alpha1/bastion.go +++ b/pkg/apis/kops/v1alpha1/bastion.go @@ -23,4 +23,6 @@ type BastionSpec struct { Enable bool `json:"Enable,omitempty"` MachineType string `json:"MachineType,omitempty"` PublicName string `json:"Name,omitempty"` + // Bastion's Loadbalancer idle timeout + IdleTimeout int `json:"IdleTimeout,omitempty"` } diff --git a/pkg/apis/kops/v1alpha1/cluster.go b/pkg/apis/kops/v1alpha1/cluster.go index 755b5ebed6..3ef94b2fda 100644 --- a/pkg/apis/kops/v1alpha1/cluster.go +++ b/pkg/apis/kops/v1alpha1/cluster.go @@ -307,3 +307,6 @@ func (c *Cluster) GetBastionMachineType() string { func (c *Cluster) GetBastionPublicName() string { return c.Spec.Topology.Bastion.PublicName } +func (c *Cluster) GetBastionIdleTimeout() int { + return c.Spec.Topology.Bastion.IdleTimeout +} diff --git a/pkg/apis/kops/validation.go b/pkg/apis/kops/validation.go index 761859e816..6daacb1e6f 100644 --- a/pkg/apis/kops/validation.go +++ b/pkg/apis/kops/validation.go @@ -327,6 +327,9 @@ func (c *Cluster) Validate(strict bool) error { if c.Spec.Topology.Bastion.MachineType == "" { return fmt.Errorf("Bastion MachineType can not be empty") } + if c.Spec.Topology.Bastion.IdleTimeout <= 0 { + return fmt.Errorf("Bastion IdleTimeout should be greater than zero") + } } // Etcd diff --git a/upup/models/cloudup/_aws/topologies/_topology_private/bastion.yaml b/upup/models/cloudup/_aws/topologies/_topology_private/bastion.yaml index d3d9f737e8..e68ca23c32 100644 --- a/upup/models/cloudup/_aws/topologies/_topology_private/bastion.yaml +++ b/upup/models/cloudup/_aws/topologies/_topology_private/bastion.yaml @@ -93,7 +93,7 @@ loadBalancerAttributes/bastion.{{ ClusterName }}: connectionSettings: loadBalancerConnectionSettings/bastion.{{ ClusterName }} loadBalancerConnectionSettings/bastion.{{ ClusterName }}: loadBalancer: loadBalancer/bastion.{{ ClusterName }} - idleTimeout: 120 + idleTimeout: {{ GetBastionIdleTimeout }} # --------------------------------------------------------------- # ASG - The Bastion itself # diff --git a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go index 29f2abee46..766cc66bae 100644 --- a/upup/pkg/fi/cloudup/populate_instancegroup_spec.go +++ b/upup/pkg/fi/cloudup/populate_instancegroup_spec.go @@ -40,6 +40,10 @@ const DefaultMasterMachineTypeGCE = "n1-standard-1" const DefaultBastionMachineTypeAWS = "t2.medium" const DefaultBastionMasterMachineTypeGCE = "n1-standard-1" +// Default LoadBalancing IdleTimeout for bastion hosts +const DefaultBastionIdleTimeoutAWS = 120 +const DefaultBastionIdleTimeoutGCE = 120 + // PopulateInstanceGroupSpec sets default values in the InstanceGroup // The InstanceGroup is simpler than the cluster spec, so we just populate in place (like the rest of k8s) func PopulateInstanceGroupSpec(cluster *api.Cluster, input *api.InstanceGroup, channel *api.Channel) (*api.InstanceGroup, error) { @@ -172,6 +176,19 @@ func DefaultBastionMachineType(cluster *api.Cluster) string { } } +// defaultIdleTimeout returns the default Idletimeout for bastion loadbalancer, based on the cloudprovider +func DefaultBastionIdleTimeout(cluster *api.Cluster) int { + switch fi.CloudProviderID(cluster.Spec.CloudProvider) { + case fi.CloudProviderAWS: + return DefaultBastionIdleTimeoutAWS + case fi.CloudProviderGCE: + return DefaultBastionIdleTimeoutGCE + default: + glog.V(2).Infof("Cannot set default IdleTimeout for CloudProvider=%q", cluster.Spec.CloudProvider) + return 0 + } +} + // defaultImage returns the default Image, based on the cloudprovider func defaultImage(cluster *api.Cluster, channel *api.Channel) string { if channel != nil { diff --git a/upup/pkg/fi/cloudup/populatecluster_test.go b/upup/pkg/fi/cloudup/populatecluster_test.go index 209ce36da1..6517e80e63 100644 --- a/upup/pkg/fi/cloudup/populatecluster_test.go +++ b/upup/pkg/fi/cloudup/populatecluster_test.go @@ -336,6 +336,26 @@ func TestPopulateCluster_BastionMachineTypeInvalidNil_Required(t *testing.T) { expectErrorFromPopulateCluster(t, c, "Bastion") } +func TestPopulateCluster_BastionIdleTimeoutInvalidNil_Required(t *testing.T) { + c := buildMinimalCluster() + c.Spec.Topology.Masters = api.TopologyPrivate + c.Spec.Topology.Nodes = api.TopologyPrivate + c.Spec.Topology.Bastion.Enable = true + c.Spec.Topology.Bastion.MachineType = "t2.small" + c.Spec.Topology.Bastion.IdleTimeout = 0 + expectErrorFromPopulateCluster(t, c, "Bastion") +} + +func TestPopulateCluster_BastionIdleTimeoutInvalidNegative_Required(t *testing.T) { + c := buildMinimalCluster() + c.Spec.Topology.Masters = api.TopologyPrivate + c.Spec.Topology.Nodes = api.TopologyPrivate + c.Spec.Topology.Bastion.Enable = true + c.Spec.Topology.Bastion.MachineType = "t2.small" + c.Spec.Topology.Bastion.IdleTimeout = -1 + expectErrorFromPopulateCluster(t, c, "Bastion") +} + func expectErrorFromPopulateCluster(t *testing.T, c *api.Cluster, message string) { _, err := PopulateClusterSpec(c) if err == nil { diff --git a/upup/pkg/fi/cloudup/template_functions.go b/upup/pkg/fi/cloudup/template_functions.go index 787391b3ad..2c963f8ae2 100644 --- a/upup/pkg/fi/cloudup/template_functions.go +++ b/upup/pkg/fi/cloudup/template_functions.go @@ -98,6 +98,7 @@ func (tf *TemplateFunctions) AddTo(dest template.FuncMap) { dest["WithBastion"] = tf.WithBastion dest["GetBastionImageId"] = tf.GetBastionImageId dest["GetBastionMachineType"] = tf.GetBastionMachineType + dest["GetBastionIdleTimeout"] = tf.GetBastionIdleTimeout dest["GetBastionZone"] = tf.GetBastionZone dest["GetELBName32"] = tf.GetELBName32 dest["IsBastionDNS"] = tf.IsBastionDNS @@ -222,6 +223,14 @@ func (tf *TemplateFunctions) GetBastionMachineType() (string, error) { return defaultMachineType, nil } +func (tf *TemplateFunctions) GetBastionIdleTimeout() (int, error) { + timeout := tf.cluster.GetBastionIdleTimeout() + if timeout <= 0 { + return 0, fmt.Errorf("IdleTimeout for Bastion can not be negative") + } + return timeout, nil +} + // Will attempt to calculate a meaningful name for an ELB given a prefix // Will never return a string longer than 32 chars func (tf *TemplateFunctions) GetELBName32(prefix string) (string, error) {