Add a default ingress ip range #10500
Conversation
|
Is this correct? I couldn't see how the 'cloud-provider' argument will be populated in config.KubeMasterConfig, so I'm not sure how the check in MasterArgs.BuildSerializeableMasterConfig() will ever fail. |
marun
force-pushed
the
default-ingress-cidr
branch
from
97a6ede to
07be0a9
Compare
|
[test] |
| // that will be allocated from. For security reasons, you should | ||
| // ensure that this range does not overlap with the CIDRs reserved | ||
| // for external ips, nodes, pods, or services. | ||
| IngressIPNetworkCIDR string |
There was a problem hiding this comment.
You don't need to add this here - we stopped allowing all but "critical" new flags here. Just adding it to the config is sufficient
There was a problem hiding this comment.
My intent was to allow an admin to set 0.0.0.0/32 to avoid having the default be set. From what I can tell, if IngressIPNetworkCIDR can't be set by arg, BuildSerializeableMasterConfig will always set the default ingress cidr (unless it conflicts with arg-provided service and cluster net). Am I missing something, or is that what you intend?
There was a problem hiding this comment.
marun
force-pushed
the
default-ingress-cidr
branch
3 times, most recently
from
a7a3f9c to
79df588
Compare
If no ingress ip range is specified, default to a private range. Ingress ip allocation can be disabled by specifying "0.0.0.0/32" for the range.
marun
force-pushed
the
default-ingress-cidr
branch
from
79df588 to
ee401cc
Compare
|
Evaluated for origin test up to ee401cc |
|
continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8263/) |
|
LGTM [merge] |
|
continuous-integration/openshift-jenkins/merge SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pr_origin/8263/) (Image: devenv-rhel7_4883) |
|
Evaluated for origin merge up to ee401cc |
If no ingress ip range is specified, default to a private range.
Ingress ip allocation can be disabled by specifying "0.0.0.0/32" for the range.
Addresses #10387
cc: @smarterclayton