aws-vpc: add support for multiple route tables #717
Conversation
| } | ||
|
|
||
| func (conf *backendConfig) routeTables() ([]string, error) { | ||
| if table, ok := conf.RouteTableID.(string); ok { |
There was a problem hiding this comment.
wondering if we can just simplify the interface so that RouteTableID is always a slice, even if it has just one route table ID, that way we won't have to do this interface stuff. WDYT?
There was a problem hiding this comment.
Changing to a slice would be a breaking change for existing configurations. This seemed less disruptive to the users but added some complexity. I don't mind either way.
There was a problem hiding this comment.
doh! Right, for some reason I thought this was a new field.
main.go
Outdated
| @@ -144,6 +144,7 @@ func main() { | |||
| log.Error("Failed to create SubnetManager: ", err) | |||
| os.Exit(1) | |||
| } | |||
| log.Info(fmt.Sprintf("Created subnet manager: %+v", sm)) | |||
There was a problem hiding this comment.
nit: can you use log.Infof() instead of Sprintf-ing the string?
There was a problem hiding this comment.
Cleaned up and pushed
There was a problem hiding this comment.
Did you forget this one in main.go? https://github.com/coreos/flannel/pull/717/files#diff-7ddfb3e035b42cd70649cc33393fe32cR147
There was a problem hiding this comment.
I did initially, I caught it and squashed in to the previous commit. Looks ok on the remote branch now.
There was a problem hiding this comment.
weird, I still see the old log format on your remote branch: https://github.com/t0mmyt/flannel/blob/awsvpc-multiaz-rebase/main.go#L147
backend/awsvpc/awsvpc.go
Outdated
|
|
||
| func (conf *backendConfig) routeTableConfigured() bool { | ||
| configured := conf.RouteTableID != nil | ||
| log.Info(fmt.Sprintf("Route table configured: %t", configured)) |
There was a problem hiding this comment.
nit: we can use log.Infof() here
There was a problem hiding this comment.
Cleaned up and pushed
backend/awsvpc/awsvpc.go
Outdated
| return []string{table}, nil | ||
| } | ||
| if rawTables, ok := conf.RouteTableID.([]interface{}); ok { | ||
| log.Info(fmt.Sprintf("RouteTableID configured as slice: %+v", rawTables)) |
There was a problem hiding this comment.
Cleaned up and pushed
t0mmyt
force-pushed
the
awsvpc-multiaz-rebase
branch
from
000ffad to
4f9a009
Compare
|
We've been running this (albeit against an older release) for quite a few months now without any noticeable issue. |
|
@t0mmyt which release/commit? |
|
f6d7239 on Nov 9 2016. We've had the current build (prior to the log changes) running on one cluster since this pull request |
t0mmyt
force-pushed
the
awsvpc-multiaz-rebase
branch
from
4f9a009 to
6afdd5c
Compare
|
can you squash both commits? |
Resolving conflicts for pull request flannel-io#561 and adding documentation.
t0mmyt
force-pushed
the
awsvpc-multiaz-rebase
branch
from
6afdd5c to
2cbd855
Compare
|
Done |
|
If you want, I can deploy this build tomorrow morning (it's late here now) and confirm all still works. |
Resolving conflicts for pull request #561 and adding documentation.
This change adds the option of the RouteTableID being passed as an array instead of a
string, allowing multiple route tables to be specified.
Todos