Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows platform will always fail, because "windows" != "linux" #1410

Closed
brettcave opened this issue May 28, 2021 · 4 comments · Fixed by #1413
Closed

Windows platform will always fail, because "windows" != "linux" #1410

brettcave opened this issue May 28, 2021 · 4 comments · Fixed by #1413

Comments

@brettcave
Copy link

brettcave commented May 28, 2021

Description

If a worker group platform is set to "windows", the eks apply will never pass. This is due how the AMI is resolved in local.

Versions

  • Terraform: 0.13.x
  • Provider(s):
    provider registry.terraform.io/hashicorp/aws v3.42.0
  • Module:
    terraform-aws-eks, v0.16.1 onwards, 0.17

Reproduction

Run this module with var.workers_group_defaults.platform set to "windows". Plan will succeed. Apply will fail:

Error: Cannot fetch root device name for blank AMI ID.
      on .terraform/modules/eks/workers.tf line 1
    198: resource "aws_launch_configuration" "workers" {

Workspaces: fails with or without.

Expected behavior

terraform apply passes, creating a windows worker node group.

Additional context

#1371 introduced the bug, affecting 0.16.1 and 0.17 and beyond.

The fix will be committed shortly.

---   worker_has_windows_ami = length([for x in concat(var.worker_groups, var.worker_groups_launch_template) : x if lookup(x, "platform", "linux") == "windows"]) > 0

+++ worker_has_windows_ami = length([for x in concat(var.worker_groups, var.worker_groups_launch_template) : x if lookup(x, "platform", "windows") == "windows"]) > 0
brettcave added a commit to brettcave/terraform-aws-eks that referenced this issue May 28, 2021
…es not find an AMI, because the local will never resolve a windows AMI from the linux platform.
@brettcave
Copy link
Author

Is there anything else that is needed to apply this to other minor versions? Some upstream modules that reference this one have minor version pinning (e.g. ~> 16.0), what would be needed to apply this to a v16 patch?

@brettcave
Copy link
Author

Is there anything else that is needed to apply this to other minor versions? Some upstream modules that reference this one have minor version pinning (e.g. ~> 16.0), what would be needed to apply this to a v16 patch?

@barryib is a v16 patch release possible? (otherwise we'll need to get other module vendors to bump their version - e.g. https://github.com/spotinst/terraform-spotinst-ocean-eks/blob/master/eks.tf#L3

@barryib
Copy link
Member

barryib commented May 28, 2021

v17.0.3 is released

@github-actions
Copy link

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants