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

Fix/208 windows resize volume #231

Merged
merged 8 commits into from
Feb 11, 2022
Merged

Conversation

ricardo-aspira
Copy link
Contributor

@ricardo-aspira ricardo-aspira commented Feb 7, 2022

What does this PR do?

Motivation

Attend the need from the community of specifying a volume size greater than the default, for Linux or Windows self managed worked nodes.

More

  • Yes, I have tested the PR using my local account setup (Provide any test evidence report under Additional Notes)
  • Yes, I have added a new example under examples to support my PR
  • Yes, I have created another PR for add-ons under add-ons repo (if applicable)
  • Yes, I have updated the docs for this feature
  • Yes, I ran pre-commit run -a with this PR

Note: Not all the PRs required examples and docs except a new pattern or add-on added.

For Moderators

  • E2E Test successfully complete before merge?

Additional Notes

Windows

PS C:\Windows\system32> get-volume

DriveLetter FriendlyName FileSystemType DriveType HealthStatus OperationalStatus SizeRemaining   Size
----------- ------------ -------------- --------- ------------ ----------------- -------------   ----
E                        NTFS           Fixed     Healthy      OK                     149.9 GB 150 GB
D                        NTFS           Fixed     Healthy      OK                     119.9 GB 120 GB
C                        NTFS           Fixed     Healthy      OK                     79.86 GB 100 GB
Input used
...
    self_mg_4 = {
      node_group_name        = "self-managed-ondemand" # Name is used to create a dedicated IAM role for each node group and adds to AWS-AUTH config map
      subnet_ids             = module.aws_vpc.private_subnets
      create_launch_template = true
      launch_template_os     = "amazonlinux2eks"       # amazonlinux2eks  or bottlerocket or windows
      public_ip              = false                   # Enable only for public subnets
      pre_userdata           = <<-EOT
            yum install -y amazon-ssm-agent \
            systemctl enable amazon-ssm-agent && systemctl start amazon-ssm-agent \
        EOT

      block_device_mappings = [
        {
          device_name = "/dev/sda1"
          volume_type = "gp3"
          volume_size = 100
        },
        {
          device_name = "/dev/sdf"
          volume_type = "gp3"
          volume_size = 120
        },
        {
          device_name = "/dev/sdg"
          volume_type = "gp3"
          volume_size = 150
        }
      ]
...

Linux

sh-4.2$ df -h
Filesystem      Size  Used  Avail  Use%  Mounted on
...
/dev/nvme0n1p1  100G  4.1G    96G    5%  /
/dev/nvme1n1    150G  186M   150G    1%  /local1
/dev/nvme2n1    120G  156M   120G    1%  /local2
Input used
...
    self_mg_4 = {
      node_group_name        = "self-managed-ondemand" # Name is used to create a dedicated IAM role for each node group and adds to AWS-AUTH config map
      subnet_ids             = module.aws_vpc.private_subnets
      create_launch_template = true
      launch_template_os     = "amazonlinux2eks"       # amazonlinux2eks  or bottlerocket or windows
      public_ip              = false                   # Enable only for public subnets
      pre_userdata           = <<-EOT
            yum install -y amazon-ssm-agent \
            systemctl enable amazon-ssm-agent && systemctl start amazon-ssm-agent \
        EOT

      block_device_mappings = [
        {
          device_name = "/dev/xvda"
          volume_type = "gp3"
          volume_size = 100
        },
        {
          device_name = "/dev/sdf"
          volume_type = "gp3"
          volume_size = 120
        },
        {
          device_name = "/dev/sdg"
          volume_type = "gp3"
          volume_size = 150
        }
      ]
...

@vara-bonthu vara-bonthu added the enhancement New feature or request label Feb 8, 2022
@vara-bonthu vara-bonthu linked an issue Feb 8, 2022 that may be closed by this pull request
1 task
Copy link
Contributor

@vara-bonthu vara-bonthu left a comment

Choose a reason for hiding this comment

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

Nice work on this PR @ricardosouzamorais ! I left few minor comments

I think we need to check few things before merging

  • E2E test with Windows and Linux nodes using Self managed with Launch Templates

  • Ensure TF Destroy works without any issues. This is to ensure the IAM role dependency on launch templates

I will run some tests over the weekend with your PR and then we can merge it

@ricardo-aspira
Copy link
Contributor Author

@vara-bonthu, I have executed terraform destroy in all scenarios:

  • Linux instances without local NVMe storage
  • Linux instances with local NVMe storage
  • Windows instances without local NVMe storage
  • Windows instances with local NVMe storage

I found no issues with that.

Regarding the E2E, how should I proceed?

Btw, I solved your comments, could you please re-check?

Copy link
Contributor

@vara-bonthu vara-bonthu 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 wait for another Approval.

Copy link
Contributor

@Zvikan Zvikan left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@askulkarni2 askulkarni2 left a comment

Choose a reason for hiding this comment

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

LGTM

@vara-bonthu vara-bonthu merged commit 4dd315c into main Feb 11, 2022
@vara-bonthu vara-bonthu deleted the fix/208-windows-resize-volume branch February 11, 2022 21:57
alidonmez pushed a commit to alidonmez/terraform-aws-eks-blueprints-1 that referenced this pull request Mar 25, 2023
…n/test/util/types/node-18.11.10

Bump @types/node from 18.11.9 to 18.11.10 in /test/util
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: disk_size on Windows nodes
4 participants