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

Add support for IPv6 pod range. #269

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

marqc
Copy link
Member

@marqc marqc commented Feb 7, 2024

IPv6 addresses are supported when using a simplified spec template for ipam plugin:

{
  "name": "gke-pod-network",
  "cniVersion": "0.3.1",
  "plugins": [
    {
      "type": "@cniType",
      "mtu": @mtu,
      "ipam": {
          "type": "host-local",
          "ranges": [
            @subnets
          ],
          "routes": [
            @routes
          ]
      }
    },
    {
      "type": "portmap",
      "capabilities": {
        "portMappings": true
      }
    }@cniBandwidthPlugin@cniCiliumPlugin@cniIstioPlugin
  ]
}

Compatibility of new template spec with other use cases is covered in install-cni.sh tests.

scripts/install-cni.sh Outdated Show resolved Hide resolved
scripts/install-cni.sh Show resolved Hide resolved
scripts/install-cni.sh Outdated Show resolved Hide resolved
Copy link
Member

@jingyuanliang jingyuanliang left a comment

Choose a reason for hiding this comment

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

Could you add a testcase where netd is used to generate calico conf file?

@marqc marqc force-pushed the ipv6-support branch 3 times, most recently from aca973a to e73fa53 Compare February 14, 2024 16:04
@marqc
Copy link
Member Author

marqc commented Feb 20, 2024

@jingyuanliang Thanks for suggestions. All applied

scripts/install-cni.sh Outdated Show resolved Hide resolved
jingyuanliang added a commit to jingyuanliang/netd that referenced this pull request Feb 21, 2024
jingyuanliang added a commit to jingyuanliang/netd that referenced this pull request Feb 21, 2024
jingyuanliang added a commit to jingyuanliang/netd that referenced this pull request Feb 21, 2024
scripts/install-cni.sh Outdated Show resolved Hide resolved
scripts/install-cni.sh Outdated Show resolved Hide resolved
elif is_ipv6_range "${subnet}" ; then
echo "IPv6 subnet detected in .spec.podCIDRs: '${subnet:-}'"
POPULATE_IP6TABLES="true"
echo "ip6tables will be populated because IPv6 podCIDR is configured (from .spec.podCIDRs)"
Copy link
Contributor

Choose a reason for hiding this comment

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

We cannot make this change live till CCM is fixed to not populate the IPv6 address in the podCIDR for single stack IPv4 clusters on dual stack subnets. Today, we do this even when the cluster's stack type is IPv4.

Copy link
Member

Choose a reason for hiding this comment

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

Can we just fix CCM to match the expectation?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we are already working on this. CCM needs this bug fix regardless.

Copy link
Member

Choose a reason for hiding this comment

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

Okay your point is just to hold netd versions with this codepath and don't release it earlier than CCM with the change, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. Exactly.

Copy link
Member

Choose a reason for hiding this comment

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

@sdmodi do you have the ETA or timeline of the fix?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it is ready to merge kubernetes/cloud-provider-gcp#652 , I'm waiting for a last pass from Sudeep

scripts/install-cni.sh Outdated Show resolved Hide resolved
@marqc marqc force-pushed the ipv6-support branch 3 times, most recently from d8a9fbc to 5deca11 Compare March 14, 2024 11:43
@marqc marqc force-pushed the ipv6-support branch 2 times, most recently from 090c957 to 23bf379 Compare March 14, 2024 11:56
@marqc marqc requested review from MrHohn and sdmodi March 14, 2024 11:57
Signed-off-by: Marek Chodor <[email protected]>
Change-Id: I93fe2040852e163f8ff1bdf8cbeb8d1c0c89f829
@jingyuanliang
Copy link
Member

/lgtm
/approve

Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jingyuanliang, marqc

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

@google-oss-prow google-oss-prow bot merged commit ca4b191 into GoogleCloudPlatform:master Apr 11, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants