🌱 Remove lint ignore for gocritic

[MrDXY]

What this PR does / why we need it:
Remove //nolint:gocritic linter and fix findings.

1. Remove nolint:gocritic for /controllers/controllers_suite_test.go
   1. To properly execute the teardown, I remove the defer keyword and place it before the os.Exit(code) statement.
2. Remove nolint:gocritic for /controllers/vspheremachine_controller.go
   1. To simplify the code, I merge the conditions r.supervisorBased and machineCtx.GetCluster().Status.InfrastructureReady using an else if statement as recommended by gocritic.
3. Remove nolint:gocritic for pkg/services/govmomi/service.go
   1. Reconstruct the switch logic, and use an if to replace it.
   2. I noticed a TODO comment from @akutz that suggests using a switch instead of an if statement to check metadata encoding. However, I have not seen any follow-ups on this. As everything seems to be working well, I have refactored it to an if structure. If anyone has a better solution or suggestion, or, perhaps I made a mistake, please feel free to comment on this PR.

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

[k8s-ci-robot]

Hi @MrDXY. Thanks for your PR.

I'm waiting for a kubernetes-sigs 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.

[killianmuldoon]

/ok-to-test

[codecov]

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (3b7fbc6) 60.79% compared to head (062d607) 60.94%.
Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2399      +/-   ##
==========================================
+ Coverage   60.79%   60.94%   +0.14%     
==========================================
  Files         164      164              
  Lines        9473     9465       -8     
==========================================
+ Hits         5759     5768       +9     
+ Misses       3298     3284      -14     
+ Partials      416      413       -3     

Files	Coverage Δ
controllers/vspheremachine_controller.go	76.47% <40.00%> (-0.09%)	⬇️
pkg/services/govmomi/service.go	1.25% <0.00%> (+0.01%)	⬆️

... and 7 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[killianmuldoon]

Looks good - one idea to add comments.

And you managed to fix what looks like a bad bug while you were at it!

[chrischdi]

One last nit.

:-)

[chrischdi]

/lgtm

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 4dd388b26bb3ddbc8be9c32ace8d18be92292433

[MrDXY]

Hi folks, do we still have any comments/suggestions or concerns on this PR? If everything looks good, please kindly approve at your convenience.

[chrischdi]

I'm good with the change, however merging will currently fail due to issues in CI...

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chrischdi

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 [chrischdi]

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

[killianmuldoon]

I'm good with the change, however merging will currently fail due to issues in CI...

Maybe not - do we re-run test runs when we merge? Let's see 🤔

[chrischdi]

/retest