modules,steps: enable existing vpc

[squat]

This commit enables the existing VPC functionality that was already
built into the installer by fixing several bugs related to the
processing of AWS private zone IDs.

Thanks to @yifan-gu's recent changes in #3219.

I tested with ~10 clusters and was able to bring up clusters in existing VPC each time.

cc @yifan-gu @enxebre

[squat]

this resource was never used :/

[alexsomesan]

Does this mean we're not doing split horizon anymore?

[squat]

we still have split horizon, it was just moved somewhere else and this was never cleaned up: https://github.com/coreos/tectonic-installer/blob/master/steps/topology/aws/main.tf#L22

[alexsomesan]

Got it. Makes sense then. All cool!

[enxebre]

I'm curious if you considered to use a ternary here to return either tectonic_aws_external_private_zone or aws_route53_zone.tectonic_int.*.zone_id then it would be transparent for consumers steps, so the private_zone_id input wouldn't need to do the check and could keep just private_zone_id = "${data.terraform_remote_state.topology.private_zone_id}"

[squat]

yes I did, but I decided against it for consistency and clarity. Otherwise, I foresee a near future where someone forgets that this variable is already the result of a ternary and adds another ternary on top. It wouldn't hurt the output but it is muddy.

[enxebre]

Nice! Thanks for putting this back. LGTM once is green.
Thought aside from this PR: In the future we might want to consider to avoid conditionals and to evolve the model to the point where deploying on a pre-existing vpc means just skip the "vpc" step and supply the expected user inputs for the remaining steps

[dcondomitti]

I just tested this and deploying into an existing VPC worked except that my ELBs and masters have public IPs despite being in private subnets (autoassign public IP is off). The cluster seemed healthy (all instances in service according to the ELBs) though so provisioning is working properly. I can open a separate issue for this if needed.

aws:
  etcd:
    ec2Type: m4.large
    iamRoleName: dcondomitti-worker
    rootVolume:
      size: 32
      type: gp2
  master:
    ec2Type: m4.large
    iamRoleName: dcondomitti-master
    rootVolume:
      size: 32
      type: gp2
  vpcCIDRBlock: 10.0.0.0/16
  worker:
    ec2Type: m4.large
    iamRoleName: dcondomitti-worker
    rootVolume:
      size: 32
      type: gp2
  sshKey: dcondomitti
  external:
    masterSubnetIDs:
      - subnet-x # three private subnets in us-west-2a, 2b, 2c
      - subnet-x
      - subnet-x
    privateZone: MYZONEID # records were created properly in the private zone
    vpcID: vpc-x
    workerSubnetIDs:
      - subnet-y # three different private subnets in us-west-2a, 2b, 2c
      - subnet-y
      - subnet-y
  installerRole: arn:aws:iam::fe-aws:role/tectonic-installer
  publicEndpoints: false
  privateEndpoints: true
  profile: coreos
  region: us-west-2
baseDomain: privatezone.example.com
containerLinux:
  channel: stable
ddns:
  key:
iscsi:
etcd:
  nodePools:
    - etcd
master:
  nodePools:
    - master
name: ot-priv
networking:
  podCIDR: 10.2.0.0/16
  serviceCIDR: 10.3.0.0/16
  type: flannel
nodePools:
  - count: 3
    name: etcd
  - count: 2
    name: master
  - count: 2
    name: worker
platform: AWS
licensePath: ~/projects/coreos/secrets/license.txt
pullSecretPath: ~/projects/coreos/secrets/pull-secret.json
worker:
  nodePools:
    - worker
admin:
  email: [email protected]
  password: x

[squat]

@dcondomitti yes please do; that is a separate issue

[trawler]

lgtm