Revert #64189: Fix Windows CNI for the sandbox case

[feiskyer]

What this PR does / why we need it:

This reverts PR #64189, which breaks DNS for Windows containers.

Refer #64189 (comment)

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #64861

Special notes for your reviewer:

Release note:

NONE

cc @madhanrm @PatrickLang @alinbalutoiu @dineshgovindasamy

[feiskyer]

/kind bug
/sig windows
/sig node
/priority critical-urgent
/milestone v1.11

[feiskyer]

Added to v1.11 milestone since this is a critical urgent bug fix.

/status approved-for-milestone

[feiskyer]

/status approved-for-milestone

[feiskyer]

/assign @Random-Liu @derekwaynecarr

[feiskyer]

/retest

[PatrickLang]

/status approved-for-milestone

[k8s-ci-robot]

You must be a member of the kubernetes/kubernetes-milestone-maintainers github team to add status labels.

[PatrickLang]

Approving per @dineshgovindasamy recommendation

[PatrickLang]

/test pull-kubernetes-kubemark-e2e-gce

[alinbalutoiu]

I would prefer to see another simple commit with more details instead of just reverting this one. I don't expect the new PR to have more than 4-5 lines.

// Do not return any IP, so that we would continue and get the IP of the Sandbox

This is not explained enough, should be something like:

// Workaround for Windows DNS issue, the CNI should call attach the endpoint to every container from the POD and not only the infra container

Would it be possible to just add the few lines in a separate PR with a proper comment and explanation instead of this revert? I can help on that if needed.

[alinbalutoiu]

Also as per microsoft/SDN#222 (comment)
It should mention the Windows versions which are affected.

[feiskyer]

I would prefer to see another simple commit with more details instead of just reverting this one.

@alinbalutoiu Added another clarification comment commit.

Would it be possible to just add the few lines in a separate PR with a proper comment and explanation instead of this revert? I can help on that if needed.

I think an explanation is enough, codes must be changed: getIP() for each containers is required for DNS settings. Let's revert the original PR and make another cleanup PR after code freeze.

[feiskyer]

/test pull-kubernetes-node-e2e

[dims]

/retest

[alinbalutoiu]

@feiskyer thanks, it is better now with that comment.

My question would be the following then, why do we need this PR then? #63905

Since setting it for the endpoint should be enough, what is the purpose of the other PR?

[feiskyer]

My question would be the following then, why do we need this PR then? #63905

@alinbalutoiu This PR is a workaround for DNS settings in CNI plugins. Please notice that the DNS configured by CNI plugins are fixed (from CNI config file), and it doesn't conform to Pod's DNS policy.

While #63905 is to fix dnsPolicy problem. Although there're still issues in HNS, but it's the right way we should move to. And this workaround should be removed after HNS issues resolved.

[feiskyer]

@michmike @PatrickLang @madhanrm @dineshgovindasamy Any comments on this?

[michmike]

the PR looks good to me, but i will defer to @dineshgovindasamy and @alinbalutoiu for approval. it seems that both are good with this, so i will approve it.

[michmike]

/approve

[michmike]

/lgtm

[feiskyer]

ping @derekwaynecarr @dchen1107 for approval

[k8s-github-robot]

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@Random-Liu @dchen1107 @derekwaynecarr @feiskyer @michmike

Pull Request Labels

• sig/node sig/windows: Pull Request will be escalated to these SIGs if needed.
• priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
• kind/bug: Fixes a bug discovered during the current release.

Help

• Additional instructions
• Commands for setting labels

[dchen1107]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dchen1107, feiskyer, michmike

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:

• pkg/kubelet/OWNERS [dchen1107]

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

[k8s-github-robot]

/test all [submit-queue is verifying that this PR is safe to merge]

[k8s-github-robot]

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.