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

Implement support for AWS ipv6 prefixes #12112

Merged
merged 8 commits into from
Sep 16, 2021

Conversation

olemarkus
Copy link
Member

@olemarkus olemarkus commented Aug 6, 2021

Using this PR you can provision a cluster using AWS prefixes.

Going forward, I expect it to be possible to enable ipv6 prefixes as part of launch templates so kOps doesn't have to do it. Meanwhile I do believe this is k8s installer territory.

I hope that in the future, the IPAM controller can be a part of CCM, but meanwhile it runs fine as part of kops-controller.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 6, 2021
@olemarkus olemarkus changed the title Implement support for AWS ipv6 prefixes WIP: Implement support for AWS ipv6 prefixes Aug 6, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 6, 2021
@olemarkus
Copy link
Member Author

This is the cluster settings I used.
I would expect this to use with any CNI though.

  kubeDNS:
    upstreamNameservers:
    - 2620:119:53::53
  kubeControllerManager:
    allocateNodeCIDR: false
    controllers:
      - "*"
      - "-nodeipam"
  kubeProxy:
    enabled: false
  networkCIDR: 172.20.0.0/16
  networking:
    cilium:
      version: v1.10.3
      disableMasquerade: true
      tunnel: disabled
  nonMasqueradeCIDR: fd00:10:96::/64

nodeup/pkg/model/sysctls.go Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 12, 2021
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/api and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 22, 2021
@olemarkus
Copy link
Member Author

Since there were actually interest in getting this one merged, I made the code paths configurable.

The commits are still a bit messy, but feedback on the overall semantics would be appreciated.

/cc @justinsb

@k8s-ci-robot k8s-ci-robot requested a review from justinsb August 22, 2021 16:33
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 28, 2021
@olemarkus olemarkus changed the title WIP: Implement support for AWS ipv6 prefixes Implement support for AWS ipv6 prefixes Aug 29, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 29, 2021
@olemarkus olemarkus requested a review from hakman September 1, 2021 18:36
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Sep 3, 2021
@olemarkus
Copy link
Member Author

This is the cluster config I use for this PR now.
I have not tested NodePort, but it should probably work.

...
  kubeProxy:
    enabled: true
  networking:
    cilium:
      enableNodePort: false
  nonMasqueradeCIDR: fd00:10:96::/64
  podCIDRFromCloud: true
...

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: justinsb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 16, 2021
@olemarkus
Copy link
Member Author

@justinsb did you comment the nits? I only see the comment above. Since we need to enable a controller i kops-controller etc we need a flag that shows user's intent. Empty podCIDR implicitly meaning cloud IPAM sounds a bit magical to me.

@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Sep 16, 2021
@rifelpet
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 16, 2021
@olemarkus
Copy link
Member Author

/retest

@k8s-ci-robot k8s-ci-robot merged commit fa29c9a into kubernetes:master Sep 16, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.23 milestone Sep 16, 2021
@justinsb
Copy link
Member

Ooops - I queued them up and forgot to hit send. I actually can't submit the comments now, so I guess I'll open a follow-up PR or two :-) It was mostly nits apart from the podCIDR == "" question, so this way probably works better anyway!

@kubernetes kubernetes deleted a comment from k8s-ci-robot Sep 22, 2021
@kubernetes kubernetes deleted a comment from olemarkus Sep 22, 2021
@kubernetes kubernetes deleted a comment from olemarkus Sep 22, 2021
@kubernetes kubernetes deleted a comment from olemarkus Sep 22, 2021
@kubernetes kubernetes deleted a comment from olemarkus Sep 22, 2021
@kubernetes kubernetes deleted a comment from olemarkus Sep 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/addons area/api area/kops-controller area/nodeup cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants