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

Support configuring service cluster ip and pod network #220

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@rapenchukd
Copy link

rapenchukd commented Jun 13, 2018

What this PR does / why we need it:
This will allow cluster creators to set/modify kubernetes service and pod networks by modifying their CIDR. Additionally this will allow a user to change the kubedns service IP so it can also reside within a changed network range. The existing values are set as safe defaults

How can this PR be verified?
Build a cluster and verify IPs

Is there any change in kubo-deployment?
kubo-deployment shouldnt need a change since all modifications have defaults. however in order to take advantage of the ability to change, kubo-deployment will need updating.

Is there any change in kubo-ci?
no

Does this affect upgrade, or is there any migration required?
If users change the networks for an existing cluster, it may cause some confustion, but everything should come back up- this will require they apply-specs for kube-dns pods to get updated as well.

Which issue(s) this PR fixes:
This does not have an open github issue, however there should be an existing internal issue tracker for this somewhere.

Release note:

Now supports the modification of both kubernetes pod network and service network by overriding the appropriate CIDRs.
@cfdreddbot

This comment has been minimized.

Copy link

cfdreddbot commented Jun 13, 2018

Hey rapenchukd!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

@cf-gitbot

This comment has been minimized.

Copy link

cf-gitbot commented Jun 13, 2018

We have created an issue in Pivotal Tracker to manage this:

https://www.pivotaltracker.com/story/show/158329092

The labels on this github issue will be updated when the story is started.

@rapenchukd

This comment has been minimized.

Copy link
Author

rapenchukd commented Jun 13, 2018

I believe this is a functional start to the required changes for An existing issue in the pivotal tacker

This will require updated to kubo-deployment as well.

@alex-slynko

This comment has been minimized.

Copy link
Member

alex-slynko commented Jun 13, 2018

Hi @rapenchukd
Thanks you for the PR. It looks good, we will verify and merge it after releasing 0.18.

@@ -15,7 +15,7 @@ processes:
- --leader-elect
- --root-ca-file=/var/vcap/jobs/kube-controller-manager/config/ca.pem
- --service-account-private-key-file=/var/vcap/jobs/kube-controller-manager/config/service-account-private-key.pem
- --service-cluster-ip-range=10.100.200.0/24

This comment has been minimized.

@karampok

karampok Jun 18, 2018

Member

I am no sure if we need that at all.
According to the doc

--cluster-cidr string-
  | CIDR Range for Pods in cluster. Requires --allocate-node-cidrs to be true

and we are not using the allocate-node-cidrs=true

This comment has been minimized.

@rapenchukd

rapenchukd Jun 19, 2018

Author

--service-cluster-ip-range is needed, it sets the address block for hosting kubernetes service IPs, if not set, kubernetes uses a 10.X.X.X default of some sort, which could potentially clash with users pod IP space, or even network routable IP space.

I think the default size is also a /24, which means a kubernetes cluster can only have an absolute max of 254 services.

This comment has been minimized.

@rapenchukd

rapenchukd Jun 19, 2018

Author

Ah, thats for the controller manager and not the apiserver, yeah you are correct, that should be fine to drop for now.

@karampok
Copy link
Member

karampok left a comment

@@ -51,7 +51,7 @@ start_flanneld() {
--cert-file /var/vcap/jobs/flanneld/config/etcd-client.crt \
--key-file /var/vcap/jobs/flanneld/config/etcd-client.key \
--ca-file /var/vcap/jobs/flanneld/config/etcd-ca.crt \
set /coreos.com/network/config '{"Network":"10.200.0.0/16","Backend":{"Type":"vxlan"}}'
set /coreos.com/network/config '{"Network":"<%= pod-network-cidr %>","Backend":{"Type":"vxlan"}}'

This comment has been minimized.

@karampok

karampok Jun 19, 2018

Member

the format is incorrect for properties.
It should be like

 set /coreos.com/network/config '{"Network":"<%= p('pod-network-cidr') %>","Backend":{"Type":"vxlan"}}'

otherwise bosh is complaining

- Error filling in template 'flanneld_ctl.erb' (line 54: undefined local variable or method `pod' for #<Bosh::Template::EvaluationContext:0x000055e7f7843278>)

akshaymankar added a commit that referenced this pull request Jun 19, 2018

Support for service cluster cidr and pod cidr
[#157480131][#220]

Signed-off-by: Konstantinos Karampogias <konstantinos.karampogias@swisscom.com>
@akshaymankar

This comment has been minimized.

Copy link
Member

akshaymankar commented Jun 19, 2018

Hey @rapenchukd, We looked at your PR, most of the things looked alright. There were a few mistakes, we've fixed them on wip/change-cidrs branch. We have some issues regarding allowing changing CIDRs, so we'll keep that work on the branch for now. We'll close this PR once we're done iterating over this.

@rapenchukd

This comment has been minimized.

Copy link
Author

rapenchukd commented Jun 19, 2018

@akshaymankar Sounds good

iainsproat added a commit that referenced this pull request Jul 5, 2018

Support for service cluster cidr and pod cidr
[#157480131][#220]

Signed-off-by: Konstantinos Karampogias <konstantinos.karampogias@swisscom.com>
@iainsproat

This comment has been minimized.

Copy link
Member

iainsproat commented Jul 17, 2018

@rapenchukd Thanks for making this PR.

The requested functionality has now been implemented as part of this story and are in the Master branch. This should be in our upcoming 0.19 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.