Use debian-iptables for k8s-dns-node-cache, bump debian-base version

[champtar]

debian-iptables container transparently select iptables-legacy or iptables-nft since v12.0.0:
kubernetes/kubernetes#82966
This fixes #338

[k8s-ci-robot]

Hi @champtar. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /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 by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[champtar]

$ kubectl logs nodelocaldns-6xmhm -n kube-system 
...
2020/04/10 03:15:39 [INFO] Added back nodelocaldns rule - {raw PREROUTING [-p tcp -d 169.254.25.10 --dport 53 -j NOTRACK]}
2020/04/10 03:15:39 [INFO] Added back nodelocaldns rule - {raw PREROUTING [-p udp -d 169.254.25.10 --dport 53 -j NOTRACK]}
...

[prameshj]

/ok-to-test

[champtar]

iptables-legacy is working as expected with this patch, but when using iptables-nft it's adding the 'raw' rules in loop

[champtar]

/hold

[champtar]

https://bugs.centos.org/view.php?id=17239

[prameshj]

https://bugs.centos.org/view.php?id=17239

Thanks for filing this.. I see you also noted that the issue is happening with debian-buster. So does that mean we cannot use the updated v2.0.0 debian base image either?

[champtar]

@prameshj we can update, but iptables-nft will be broken, so I prefer to wait to reintroduce the change. iptables-legacy seems fine.

[prameshj]

@prameshj we can update, but iptables-nft will be broken, so I prefer to wait to reintroduce the change. iptables-legacy seems fine.

ok. Would you mind submitting another PR to upgrade base image to 2.0.0 and no other change? There are some vulnerability fixes that went into the latest base image which would be good to pick up. I can submit one too if needed.

[champtar]

In that case we can merge this code explicitly stating that nft support is broken ? (I'll change the commit message). So everything will be tested and the only missing piece will be a version bump for iptables image

[champtar]

BTW the iptables bug report: https://bugzilla.netfilter.org/show_bug.cgi?id=1422

[prameshj]

@champtar can you send out another PR, which will use the debian base for node-cache as well? We can merge that PR right away since it is valuable to go to the latest debian base.
The only change left would be to use ARG_FROM_IPT in node-cache Dockerfile. And that can be covered in this PR.

[champtar]

@prameshj ok will do that likely today

[champtar]

to be rebased on top of #370 when merged

[champtar]

@prameshj I don't think Travis is going to work :(

[luigibk]

@prameshj @champtar yes

@luigibk can you check that the ebtables change here is sufficient?

Change looks good to me.

@prameshj yes it looks good and works for me

[luigibk]

@luigibk when deploying node-local-dns you need to change the CNI config to point to the dns to the local IP 169.254.20.10, so why not also add a route ip r add 169.254.25.10 via <nodeip> ?
ebtables definitly works but it's an ugly hack IMO

@champtar No I do NOT need to make any changes to the CNI (Azure in my case) with the ebtables option active. That's all I need and there are for me clear advantages in not having to modify the CNI and being able to control all the setup in a single place.

[luigibk]

/approve

[champtar]

@prameshj Travis is ok now, but it didn't update the PR status

[champtar]

@prameshj LGTM ?

[prameshj]

yup.
/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: champtar, luigibk, prameshj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [prameshj]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[champtar]

@prameshj do you already have a date in mind for the next release ?

[prameshj]

I just pushed tag 1.16.0.

[champtar]

Hi @prameshj,
Any ETA for the build ?

$ podman pull k8s.gcr.io/dns/k8s-dns-node-cache:1.16.0
Trying to pull k8s.gcr.io/dns/k8s-dns-node-cache:1.16.0...
  manifest unknown: Failed to fetch "1.16.0" from request "/v2/dns/k8s-dns-node-cache/manifests/1.16.0".

[prameshj]

They will be available once kubernetes/k8s.io#1427 merges.

[champtar]

Thanks !