Skip to content
This repository has been archived by the owner on Jun 23, 2021. It is now read-only.

Add verification tasks, always tag NAT gateways, and fix typos #57

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

johnsimcall
Copy link
Collaborator

This PR resolves pre-run verification issues #4 #7 #55 and bug #56

@jaredhocutt would you please take a look at this when you get back from PTO?

This resolves issues jaredhocutt#4 (keypair_path), jaredhocutt#7 (route53_hosted_zone), and jaredhocutt#55 (rhcos_ami)

Edit: Commit amended to include the removal of TODO comments from aws_create.yml
@@ -6,7 +6,7 @@ cluster_domain: "{{ cluster_name }}.{{ base_domain }}"

vpc_cidr: 172.31.0.0/16
vpc_subnet_bits: 24
route53_hosted_zone_name: "{{ cluster_domain }}"
route53_hosted_zone_name: "{{ cluster_domain }}" #TODO: Does this need to have a trailing period?
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No trailing period needed

@@ -21,7 +21,7 @@ ec2_instance_type_bootstrap: i3.large
ec2_instance_type_controller: m5.xlarge
ec2_instance_type_worker: m5.large

root_volume_size_bastion: 100 # +80GB to mirror the OLM images
root_volume_size_bastion: 20 # 20GB base + 100GB to mirror OperatorHub
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean for this to be 120 instead of 20?

Comment on lines +9 to +20
- name: Fail when subnet_ids are undefined
fail:
msg: |
ERROR: When vpc_id is provided a list of public and private subnet_ids
must also be provided. For example:
public_subnet_ids:
- subnet-0123456789abcdef0
private_subnet_ids:
- subnet-1234567890abcdef1
- subnet-234567890abcdef12
- subnet-34567890abcdef123
when: (public_subnet_ids is undefined) or (private_subnet_ids is undefined)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need a bit more complex check because for a disconnected environment, there will not be any public_subnet_ids. Since we don't have the full logic for disconnected yet, this is fine to leave as is. Just commenting so we have a reminder for the future.

Comment on lines +90 to +109
- block:
- name: Create NAT gateways
ec2_vpc_nat_gateway:
subnet_id: "{{ item }}"
if_exist_do_not_create: yes
loop: "{{ public_subnet_ids }}"
register: r_create_nat_gateways

# The ec2_vpc_nat_gateway doesn't allow you to add tags during creation, so
# let's tag things after the fact (even if not all of the NGWs were created)
always:
- name: Add NAT gateway tags
ec2_tag:
resource: "{{ item.0.nat_gateway_id }}"
tags:
Name: "{{ cluster_id }}-{{ item.1 }}"
OpenShiftCluster: "{{ cluster_domain }}"
OpenShiftClusterId: "{{ cluster_id }}"
loop: "{{ r_create_nat_gateways.results | zip(availability_zone_names) | list }}"
when: item.0.success
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty clever. I like it!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants