Handle Empty clusterCIDR

[mandarjog]

What this PR does / why we need it:
Handles empty clusterCIDR by skipping the corresponding rule.

Which issue this PR fixes
fixes #36652

Special notes for your reviewer:

1. Added test to check for presence/absence of XLB to SVC rule
2. Changed an error statement to log rules along with the error string in case of a failure; This ensures that full debug info is available in case of iptables-restore errors.

Empty clusterCIDR causes invalid rules generation.
Fixes issue #36652

---

This change is

[k8s-ci-robot]

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands will still work. Regular contributors should join the org to skip this step.

If you have questions or suggestions related to this bot's behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.

[bprashanth]

Didn't you mention that you were writing an e2e for this (iptables-restore --test)? or do you want to send a follow up, or file a bug and we'll add it sometime since it sounds useful to detect a bad configuration and explicitly warn

[bprashanth]

can you just check the fp.clusterCIDR and get rid of the last bool arg?

[mandarjog]

Done.

[mandarjog]

I will open an issue and handle it there separately.

I started it but realized it is more work than I thought.

Regards,
Sent from my Ericsson Blaze

Mandar U Jog

On Nov 15, 2016, at 12:04 PM, Prashanth B notifications@github.com wrote:

@bprashanth commented on this pull request.

Didn't you mention that you were writing an e2e for this (iptables-restore --test)? or do you want to send a follow up, or file a bug and we'll add it sometime since it sounds useful to detect a bad configuration and explicitly warn

In pkg/proxy/iptables/proxier_test.go:

func TestOnlyLocalNodePorts(t *testing.T) {
ipt := iptablestest.NewFake()
fp := NewFakeProxier(ipt)

• onlyLocalNodePorts(t, fp, ipt, true)
  +}
  +
  +func onlyLocalNodePorts(t *testing.T, fp *Proxier, ipt *iptablestest.FakeIPTables, shouldLBTOSVCRuleExist bool) {
  can you just check the fp.clusterCIDR and get rid of the last bool arg?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[bprashanth]

LGTM thanks

[mandarjog]

@bprashanth Thanks, Tests passed.

[k8s-github-robot]

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue