-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add support for IPv6 pod range. #269
Conversation
8e56a82
to
a16ce20
Compare
There was a problem hiding this 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?
aca973a
to
e73fa53
Compare
@jingyuanliang Thanks for suggestions. All applied |
Contributed by Marek Chodor <[email protected]> in GoogleCloudPlatform#269
Contributed by Marek Chodor <[email protected]> in GoogleCloudPlatform#269
Contributed by Marek Chodor <[email protected]> in GoogleCloudPlatform#269
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)" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Exactly.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
d8a9fbc
to
5deca11
Compare
090c957
to
23bf379
Compare
Signed-off-by: Marek Chodor <[email protected]> Change-Id: I93fe2040852e163f8ff1bdf8cbeb8d1c0c89f829
/lgtm |
[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 |
ca4b191
into
GoogleCloudPlatform:master
IPv6 addresses are supported when using a simplified spec template for ipam plugin:
Compatibility of new template spec with other use cases is covered in install-cni.sh tests.