Skip to content

Conversation

@dustymabe
Copy link
Member

  • Now depending on which directory you are in it will do the right thing.
  • Handles RHCOS versus FCOS gracefully
  • Deletes old provisioning instructions from the README

- Now depending on which directory you are in it will do the right thing.
- Handles RHCOS versus FCOS gracefully
- Deletes old provisioning instructions from the README
Copy link
Member

@jlebon jlebon left a comment

Choose a reason for hiding this comment

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

One comment, but LGTM as is. Will let @ravanelli stamp it.

Copy link
Member

Choose a reason for hiding this comment

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

Does TF support includes? I feel like it might be clearer to rename this file, move it one level up and have the aarch64 and x86_64 bits just define the arch and include this one?

@ravanelli
Copy link
Member

@dustymabe I finally got the change to test it. I did a rebase and only add the IP suggestion, I will let you do the rebase. So we can merge it.

Copy link
Member

@ravanelli ravanelli left a comment

Choose a reason for hiding this comment

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

LGTM, let's rebase it. The new x86 in AWS as deployed using it.

instance_type = "m6g.metal"
instance_type = local.instance_type
vpc_security_group_ids = [aws_security_group.sg.id]
subnet_id = local.aws_subnet_id
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
subnet_id = local.aws_subnet_id
subnet_id = local.aws_subnet_id
private_ip = var.distro == "rhcos" ? (var.arch == "aarch64" ? "10.30.77.147" : "10.30.77.139") : null

Let's add our fixed IPs here, so we do not need to update our othe confgs in case the IPs changing

Copy link
Member

Choose a reason for hiding this comment

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

Or we can also pass them as we are passing the splunk part, maybe that's better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants