Revert "fix: handle socket file detection on Windows"

[jsturtevant]

This reverts commit 4060ee6.

What type of PR is this?

/kind bug
/kind regression

What this PR does / why we need it:

It looks like fix in golang for golang/go#33357 isn't working properly.

Which issue(s) this PR fixes:

Fixes #126965

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Fixes a 1.31 regression starting kubelet on Windows: Revert "fix: handle socket file detection on Windows"

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

NONE

[jsturtevant]

/assign @marosset @kannon92

fyi @fakecore

[marosset]

/lgtm

Should we add a test to make sure we don't accidently regress this in the future?

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 4f1cc94500bc6d8cb5cb3161fa834abac57aa31b

[kannon92]

/priority critical

[k8s-ci-robot]

@kannon92: The label(s) priority/critical cannot be applied, because the repository doesn't have them.

In response to this:

/priority critical

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-sigs/prow repository.

[ffromani]

/triage accepted
/priority critical-urgent

I think this will be backported to 1.31, right?

[ffromani]

/lgtm

Should we add a test to make sure we don't accidently regress this in the future?

yes. We do have already device manager tests though, and some of them do cause a kubelet restart (which leads to flakes, but that's another story for another day). It should be sufficient to make sure these tests run on windows lane(s).

[jsturtevant]

I can take a look at what it would take to enable those tests, I don't believe we currently run them in anyway.

[jsturtevant]

It looks like these are the tests https://github.com/kubernetes/kubernetes/blob/master/test/e2e_node/device_manager_test.go?

Unfortunately, we don't currently have ability to run e2e_node tests on Windows. It might be something that could be enabled but not in a timely fashion, mostly since api-server isn't built/run on windows, so I don't know what the effort would be to enable that. etcd currently produces windows builds but they are no longer running ci against it

I can add a test into e2e test suite, that will restart kubelet which will cover more than just device manager scenarios.

[randomvariable]

Can confirm the reboot test would catch this

[jsturtevant]

/test pull-kubernetes-e2e-capz-windows-serial-slow
I've added the reboot test into this job. I tested it locally with and without the branch and it does confirm rolling this back works.

[jsturtevant]

Test passed!

Kubernetes e2e suite: [It] [sig-windows] [Feature:Windows] [Excluded:WindowsDocker] [MinimumKubeletVersion:1.22] RebootHost containers [Serial] [Disruptive] [Slow] should run as a reboot process on the host/node

/assign @kannon92 @ffromani

[jsturtevant]

/assign @marosset

[kannon92]

@jsturtevant a presubmit is helpful but is there a periodic that would catch this?

Otherwise, this looks good to me.

[jsturtevant]

yes, opened kubernetes/test-infra#33395 to enable it after this merges

[ffromani]

/lgtm

the revert looks good. Bummer that we can't run the device manager tests on windows, but it seems we got some coverate thanks to @jsturtevant

[kannon92]

/assign @mrunalp @dchen1107 @derekwaynecarr

[kannon92]

/assign @yujuhong @klueska

[kannon92]

/unassign @mrunalp @derekwaynecarr

[dims]

/approve

this is a priority-critical revert.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, jsturtevant

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/cm/OWNERS [dims]

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

[kannon92]

@jsturtevant can we get a cherry-pick for this for v1.31?

[tt-kkaiser]

@jsturtevant This pull request can be reverted, as of pull request #127076 kubernetes uses go 1.23, so the previous code is functional again.

[ffromani]

@jsturtevant This pull request can be reverted, as of pull request #127076 kubernetes uses go 1.23, so the previous code is functional again.

Thanks for the update!
the process is usually to commit fixes on master branch and then backport, and this is what we need to do to unbreak 1.31. Once that is fixed, we can re-introduce the original fix in a separate PR, perhaps with more tests. I'd do that rather than a re-revert.