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 datastore-driven clusterIP updates #272

Merged

Conversation

@caseydavenport
Copy link
Member

caseydavenport commented Sep 28, 2019

@@ -47,18 +47,12 @@ type routeGenerator struct {
svcIndexer, epIndexer cache.Indexer
svcRouteMap map[string]map[string]bool
routeAdvertisementCount map[string]int
clusterCIDR string
clusterCIDRs []string

This comment has been minimized.

Copy link
@caseydavenport

caseydavenport Sep 28, 2019

Author Member

This change to plural isn't strictly necessary, but aligns us with the externalIPs field and will be required when we want IPv6 support.

This comment has been minimized.

Copy link
@neiljerram

neiljerram Sep 30, 2019

Member

Don't think I understand. It is necessary now that the data model allows multiple cluster CIDRs, isn't it?

Copy link
Member

neiljerram left a comment

A few queries here.

pkg/backends/calico/client.go Outdated Show resolved Hide resolved
pkg/backends/calico/client.go Outdated Show resolved Hide resolved
pkg/backends/calico/client.go Show resolved Hide resolved
pkg/backends/calico/client.go Outdated Show resolved Hide resolved
@@ -47,18 +47,12 @@ type routeGenerator struct {
svcIndexer, epIndexer cache.Indexer
svcRouteMap map[string]map[string]bool
routeAdvertisementCount map[string]int
clusterCIDR string
clusterCIDRs []string

This comment has been minimized.

Copy link
@neiljerram

neiljerram Sep 30, 2019

Member

Don't think I understand. It is necessary now that the data model allows multiple cluster CIDRs, isn't it?

pkg/backends/calico/routes.go Show resolved Hide resolved
pkg/backends/calico/routes.go Show resolved Hide resolved
Copy link
Member

lmm left a comment

LGTM just a few small things to change (assuming the libcalico PR is good)

pkg/backends/calico/client.go Outdated Show resolved Hide resolved
pkg/backends/calico/client.go Show resolved Hide resolved
Copy link
Member

neiljerram left a comment

I'm happy to leave remaining changes to your judgement, following our chat just now.

@caseydavenport caseydavenport added this to the Calico v3.10.0 milestone Sep 30, 2019
@caseydavenport caseydavenport merged commit d41026f into projectcalico:master Sep 30, 2019
2 checks passed
2 checks passed
license/cla Contributor License Agreement is signed.
Details
semaphoreci The build passed on Semaphore.
Details
@caseydavenport caseydavenport deleted the caseydavenport:handle-datastore-clusterip branch Sep 30, 2019
@lmm lmm added docs-completed and removed docs-pr-required labels Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.