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.

Sign up for GitHub

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

Jump to bottom

Allocate IPv6 blocks for all subnets #776

Merged
merged 3 commits into from
May 10, 2019
Merged

Allocate IPv6 blocks for all subnets #776

merged 3 commits into from
May 10, 2019

Conversation

errordeveloper
Copy link
Contributor

@errordeveloper errordeveloper commented May 2, 2019 •
edited
Loading

Description

Fixes #566.

Checklist

  • Code compiles correctly (i.e make build)
  • Added tests that cover your change (if possible)
  • All unit tests passing (i.e. make test)
  • All integration tests passing (i.e. make integration-test)
  • Added/modified documentation as required (such as the README.md, and examples directory)

@errordeveloper errordeveloper changed the title WIP: Allocate ipv6 blocks WIP: Allocate IPv6 blocks for all subnets May 2, 2019
@errordeveloper errordeveloper mentioned this pull request May 2, 2019
Closed
@VincentCouzij
Copy link

Feature is working as expected during my testing. The ipv6 CIDR are added to the subnets and the route table also includes the ipv6 outgoing rule.

martina-if
martina-if reviewed May 7, 2019
View reviewed changes
pkg/cfn/builder/vpc.go Outdated Show resolved Hide resolved
pkg/cfn/builder/vpc.go Outdated Show resolved Hide resolved
pkg/vpc/vpc.go Outdated Show resolved Hide resolved
@errordeveloper errordeveloper force-pushed the allocate-ipv6-blocks branch 2 times, most recently from 34b3d8f to 41ba556 Compare May 8, 2019 07:31
@errordeveloper errordeveloper force-pushed the allocate-ipv6-blocks branch 2 times, most recently from 07c859c to c92a5be Compare May 8, 2019 07:44
martina-if
martina-if reviewed May 8, 2019
View reviewed changes
pkg/cfn/builder/vpc.go Outdated Show resolved Hide resolved
@errordeveloper errordeveloper force-pushed the allocate-ipv6-blocks branch 3 times, most recently from 6b4eeb1 to e321c38 Compare May 9, 2019 16:08
@errordeveloper errordeveloper marked this pull request as ready for review May 9, 2019 16:08
@errordeveloper errordeveloper changed the title WIP: Allocate IPv6 blocks for all subnets Allocate IPv6 blocks for all subnets May 9, 2019
@errordeveloper
Copy link
Contributor Author

I added a whole bunch of new unit tests for the cluster stack, it's not complete yet, but at least it now goes as far as testing key aspects of the cluster stack builder.

https://github.com/weaveworks/eksctl/pull/776/files#diff-d5202f6261e88644b8b1715122f47c4c

martina-if
martina-if reviewed May 10, 2019
View reviewed changes
pkg/cfn/builder/vpc.go
// NOTE: this is done inside of CloudFormation using Fn::Cidr,
// we don't slice it here, just construct the JSON expression
// that does slicing at runtime.
refAutoAllocateCIDRv6 := gfn.MakeFnSelect(
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is inside a big loop and these are quite low level details, can we encapsulate lines 57-66 into a function like addCIDR6Selector(subnetIndex)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the puzzling thing is that it's all a bit tied into how the outer function works, don't you think? I'd much rather take all of the conditional IPv6 bits out, but that asks for more refactoring...

Copy link
Contributor Author

@errordeveloper errordeveloper May 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I've tried, but it turns out more complicated for what its worth... I'd rather live it for later.

@errordeveloper errordeveloper merged commit 0f4f8d6 into master May 10, 2019
@errordeveloper errordeveloper deleted the allocate-ipv6-blocks branch May 10, 2019 13:58
@errordeveloper errordeveloper mentioned this pull request May 13, 2019
Closed
6 tasks
@nikimanoledaki nikimanoledaki mentioned this pull request Oct 1, 2021
Closed
torredil pushed a commit to torredil/eksctl that referenced this pull request May 20, 2022
@k8s-ci-robot
Merge pull request eksctl-io#776 from AndyXiangLi/not-tolerate-all-taint
ee3b46c 
add toleration time to NoExecute effect --- Enable in next release
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@martina-if martina-if martina-if approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create ipv6 subnets for created VPC
3 participants
@errordeveloper @VincentCouzij @martina-if