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 usage of pre-existing VPCs via config file #447

Merged
merged 1 commit into from
Jan 18, 2019

Conversation

errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented Jan 17, 2019

Description

Checklist

  • Code compiles correctly (i.e make build)
  • All tests passing (i.e. make test)
  • Added/modified documentation as required (such as the README)

@errordeveloper

This comment has been minimized.

@errordeveloper errordeveloper changed the title Fix usage of pre-existing VPCs via config file WIP: Fix usage of pre-existing VPCs via config file Jan 17, 2019
@errordeveloper errordeveloper force-pushed the fix-vpc-config branch 3 times, most recently from 6d6e860 to edc67bc Compare January 18, 2019 08:27
@errordeveloper errordeveloper changed the title WIP: Fix usage of pre-existing VPCs via config file Fix usage of pre-existing VPCs via config file Jan 18, 2019
eu-north-1c:
id: subnet-c123456
# must provide 'Private' and/or 'Public' subnets by availibility zone as shown
Private:
Copy link
Contributor Author

@errordeveloper errordeveloper Jan 18, 2019

Choose a reason for hiding this comment

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

We should make this lowercase, but that would a bigger change and we'd have to bump the API version. Next time!

@errordeveloper
Copy link
Contributor Author

Integration test looking pass.

- allow VPC ID to be passed via config file
- validate VPC and subnet CIDRs
- improve example config file
- cleanup code, make it more consistent, add comments
Copy link
Contributor

@dlespiau dlespiau left a comment

Choose a reason for hiding this comment

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

\o/

@errordeveloper errordeveloper merged commit 4980695 into master Jan 18, 2019
@errordeveloper errordeveloper deleted the fix-vpc-config branch January 18, 2019 11:37
@pputnik
Copy link

pputnik commented May 11, 2020

I still have the issue, the vpc and subnets already created. The code:

---
apiVersion: eksctl.io/v1alpha5
kind: ClusterConfig

metadata:
  name: eksworkshop-eksctl
  region: eu-west-1

vpc:
  id: vpc-0da60ff028192627e
  subnets:
    Public: 
      eu-west-1b: { id: subnet-08dbc4f966c6410d4 }
      eu-west-1a: { id: subnet-0e0d14242f3472e8e }
      eu-west-1c: { id: subnet-02408be52718f5398 }
    Private: 
      eu-west-1a: { id: subnet-00b9ec0511703c9f6 }

managedNodeGroups:
- name: nodegroup
  instanceType: t3a.mini
  desiredCapacity: 2
  iam:
    withAddonPolicies:
      albIngress: true

The error is in CF template ("Subnet Id is required"):

  ControlPlane:
    Type: 'AWS::EKS::Cluster'
    Properties:
      Name: eksworkshop-eksctl
      ResourcesVpcConfig:
        SecurityGroupIds:
          - !Ref ControlPlaneSecurityGroup
        SubnetIds:
          - ''
          - ''
          - ''
          - ''
          - ''
          - ''
      RoleArn: !GetAtt 
        - ServiceRole
        - Arn
      Version: '1.15'

Am I doing something wrong? I want my nodes be in public subnet.

@martina-if
Copy link
Contributor

Hi @pputnik ,

I think there might be a mistake in the cluster configuration you are using:

vpc:
  subnets:
    Public: 
      eu-west-1b: { id: subnet-08dbc4f966c6410d4 }
      eu-west-1a: { id: subnet-0e0d14242f3472e8e }
      eu-west-1c: { id: subnet-02408be52718f5398 }
    Private: 
      eu-west-1a: { id: subnet-00b9ec0511703c9f6 }

Public and Private should be all lowercase:

vpc:
  subnets:
    public: 
      eu-west-1b: { id: subnet-08dbc4f966c6410d4 }
      eu-west-1a: { id: subnet-0e0d14242f3472e8e }
      eu-west-1c: { id: subnet-02408be52718f5398 }
    private: 
      eu-west-1a: { id: subnet-00b9ec0511703c9f6 }

Let us know if that doesn't help!

@pputnik
Copy link

pputnik commented May 12, 2020

Hi @martina-if , thank you, I took uppercase version it from solution in one of the linked issues.

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.

cannot create cluster on existing VPC with config file
4 participants