remove hard-coding of AWS instance type

[trawler]

resolves #85

[enxebre]

is the installer passing this value? what happens if this is empty?

[trawler]

/test e2e

[enxebre]

@trawler this is probably a legit fail based on my comment above as the test config don't set that value

[smarterclayton]

/test e2e-aws

[trawler]

/hold

[enxebre]

@trawler The installer is currently not setting this value, we'll want a counter part PR to set this from the installer config.
How everr looking at the actuator https://github.com/openshift/cluster-api-provider-aws/blob/master/cloud/aws/actuators/machine/actuator.go#L381
type RunInstancesInput struct would use a default value

// The instance type. For more information, see Instance Types (http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html)
	// in the Amazon Elastic Compute Cloud User Guide.
	//
	// Default: m1.small
	InstanceType *string `type:"string" enum:"InstanceType"

and the e2e-aws shows green, so not sure why our e2e was failing in your previews commit when the value was not set in the config, can you please verify this to ensure there's no gaps from installer->actuator?

[trawler]

@trawler The installer is currently not setting this value, we'll want a counter part PR to set this from the installer config.
How everr looking at the actuator https://github.com/openshift/cluster-api-provider-aws/blob/master/cloud/aws/actuators/machine/actuator.go#L381
type RunInstancesInput struct would use a default value

// The instance type. For more information, see Instance Types (http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/instance-types.html)
	// in the Amazon Elastic Compute Cloud User Guide.
	//
	// Default: m1.small
	InstanceType *string `type:"string" enum:"InstanceType"

and the e2e-aws shows green, so not sure why our e2e was failing in your previews commit when the value was not set in the config, can you please verify this to ensure there's no gaps from installer->actuator?

@enxebre I can confirm that when the instanceType is not being set, installation fails with this reply from the server:

time="2018-10-11T12:55:03Z" level=error msg="error creating EC2 instance: InvalidParameterValue: Invalid value '' for InstanceType.\n\tstatus code: 400, request id: 7b516eca-f3f4
-40cb-b6c7-5d3b87057ccb" controller=awsMachine machine=openshift-cluster-api/worker-xkpbr

I'm not sure why the comment you quoted claims to set a default value, but actually, the aws-sdk will fail the install for an empty InstanceType with an InvalidParams error:
https://github.com/aws/aws-sdk-go/blob/master/service/ec2/api.go#L23606

// Validate inspects the fields of the type to determine if they are valid.
func (s *AllocateHostsInput) Validate() error {
  invalidParams := request.ErrInvalidParams{Context: "AllocateHostsInput"}
  if s.AvailabilityZone == nil {
    invalidParams.Add(request.NewErrParamRequired("AvailabilityZone"))
  }
  if s.InstanceType == nil {
    invalidParams.Add(request.NewErrParamRequired("InstanceType"))
  }

[trawler]

/test e2e-aws

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: trawler

If they are not already assigned, you can assign the PR to them by writing /assign @trawler in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[trawler]

/test e2e-aws

[trawler]

e2e-aws test failing for:

* aws_eip.nat_eip.3: Error creating EIP: AddressLimitExceeded: The maximum number of addresses has been reached.
	status code: 400, request id: 2f81367d-af16-4462-b41e-84b7fc3ba5a2

[paulfantom]

/test e2e-aws
/test images

[openshift-ci-robot]

@trawler: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
ci/prow/images	9c94eee	link	/test images
ci/prow/e2e-aws	9c94eee	link	/test e2e-aws

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[wking]

is the installer passing this value? what happens if this is empty?

Where do we want the default value to live? If it's here, that's great. If it's in the installer, what happens if a user later bumps the config to remove the property (in an attempt to revert to the default value)? Or will that sort of adjustment be blocked somehow?

[wking]

Also, it may not be worth sinking time into this if the installer starts creating worker machine-sets directly.

[enxebre]

closing as specs are now moved to the installer