Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Return user networking mode as default on macOS and linux & Remove unused flags from goproxy integration test #4380

Merged
merged 2 commits into from
Oct 4, 2024
Merged

Return user networking mode as default on macOS and linux & Remove unused flags from goproxy integration test #4380

merged 2 commits into from
Oct 4, 2024

Conversation

vyasgun
Copy link
Contributor

@vyasgun vyasgun commented Sep 27, 2024

Fixes: Issue #4335

Solution/Idea

This PR addresses two issues I encountered while running integration tests on my local machine

  • The default networking mode returned should be based on the OS
  • goproxy test is defining and parsing unused flags which are causing a post test-run panic

Proposed changes

  1. Removed the unused flags
  2. Modified the function for fetching the default networking mode

Testing

make GINKGO_OPTS="--ginkgo.label-filter='openshift-preset'" BUNDLE_PATH="--bundle-path=~/.crc/cache/crc_vfkit_4.16.7_arm64.crcbundle" PULL_SECRET_PATH="--pull-secret-path=~/Downloads/pull-secret.txt" integration

@anjannath
Copy link
Member

No panics now..

% make GINKGO_OPTS="--ginkgo.label-filter='goproxy'" BUNDLE_PATH="--bundle-path=/Users/anath/.crc/cache/crc_vfkit_4.16.7_arm64.crcbundle" PULL_SECRET_PATH="--pull-secret-path=/Users/anath/Downloads/pull-secret" integration
Running Suite: Integration - /Users/anath/work/github.com/crc-org/crc/test/integration
======================================================================================
Random Seed: 1727768538

Will run 7 of 36 specs
•••••••SSSSSSSSSSSSSSSSSSSSSSSSSSSSS

Ran 7 of 36 Specs in 953.907 seconds
SUCCESS! -- 7 Passed | 0 Failed | 0 Pending | 29 Skipped
PASS
ok      github.com/crc-org/crc/v2/test/integration      955.429s

@vyasgun need to run make fmt; the CI tests are currently failing because of:

pkg/crc/network/types.go:5: File is not `goimports`-ed (goimports)
	"github.com/crc-org/crc/v2/pkg/crc/logging"
make: *** [cross-lint] Error 1

@cfergeau
Copy link
Contributor

cfergeau commented Oct 1, 2024

Looks good to me, though make lintwill need fixing as pointed out by Anjan.

@vyasgun
Return default networking mode based on the operating system
dcb00ed 
- System mode is default for linux
- User mode is default for darwin and windows
@vyasgun
go proxy integration test: remove unused flags from the test
0f9cca3 
- The flags are not being used in the test
- The flags are being parsed inside a goroutine which
  is causing a panic (due to race condition)
@vyasgun
Copy link
Contributor Author

vyasgun commented Oct 3, 2024

Thanks @anjannath ! For some reason, make lint fails on my system with the following panics:

gvyas@Gunjans-MacBook-Pro crc % make lint
"/Users/gvyas/work/crc/tools/bin"/golangci-lint run
ERRO [linters_context/goanalysis] fact_purity: panic during analysis: interface conversion: interface {} is nil, not *buildir.IR, goroutine 12222 [running]:
runtime/debug.Stack()
        /opt/homebrew/Cellar/go/1.23.0/libexec/src/runtime/debug/stack.go:26 +0x64
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyzeSafe.func1()
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:105 +0x4c
panic({0x101b30000?, 0x1400601d1a0?})
        /opt/homebrew/Cellar/go/1.23.0/libexec/src/runtime/panic.go:785 +0x124
honnef.co/go/tools/analysis/facts/purity.purity(0x1400c802380)
        /Users/gvyas/work/crc/tools/vendor/honnef.co/go/tools/analysis/facts/purity/purity.go:109 +0x270
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyze(0x14004249d10)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:191 +0x8ac
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyzeSafe.func2()
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:113 +0x20
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0x1400282c2d0, {0x10173ebc3, 0xb}, 0x1400370af30)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x44
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyzeSafe(0x1010948e0?)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:112 +0x70
github.com/golangci/golangci-lint/pkg/goanalysis.(*loadingPackage).analyze.func2(0x14004249d10)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_loadingpackage.go:80 +0xac
created by github.com/golangci/golangci-lint/pkg/goanalysis.(*loadingPackage).analyze in goroutine 3677
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_loadingpackage.go:75 +0x174 
ERRO [linters_context/goanalysis] nilness: panic during analysis: interface conversion: interface {} is nil, not *buildir.IR, goroutine 12218 [running]:
runtime/debug.Stack()
        /opt/homebrew/Cellar/go/1.23.0/libexec/src/runtime/debug/stack.go:26 +0x64
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyzeSafe.func1()
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:105 +0x4c
panic({0x101b30000?, 0x140060441e0?})
        /opt/homebrew/Cellar/go/1.23.0/libexec/src/runtime/panic.go:785 +0x124
honnef.co/go/tools/analysis/facts/nilness.run(0x1400c98a0e0)
        /Users/gvyas/work/crc/tools/vendor/honnef.co/go/tools/analysis/facts/nilness/nilness.go:65 +0x244
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyze(0x140043b65a0)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:191 +0x8ac
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyzeSafe.func2()
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:113 +0x20
github.com/golangci/golangci-lint/pkg/timeutils.(*Stopwatch).TrackStage(0x1400282c2d0, {0x1016ecf00, 0x7}, 0x1400326f730)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/timeutils/stopwatch.go:111 +0x44
github.com/golangci/golangci-lint/pkg/goanalysis.(*action).analyzeSafe(0x1010948e0?)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_action.go:112 +0x70
github.com/golangci/golangci-lint/pkg/goanalysis.(*loadingPackage).analyze.func2(0x140043b65a0)
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_loadingpackage.go:80 +0xac
created by github.com/golangci/golangci-lint/pkg/goanalysis.(*loadingPackage).analyze in goroutine 3677
        /Users/gvyas/work/crc/tools/vendor/github.com/golangci/golangci-lint/pkg/goanalysis/runner_loadingpackage.go:75 +0x174 
ERRO [linters_context/goanalysis] typedness: panic during analysis: interface conversion: interface {} is nil, not *buildir.IR, goroutine 12220 [running]:
runtime/debug.Stack()

Figuring out what's wrong. I've updated the PR with the change that was mentioned in the logs so hope the CI tests pass

@cfergeau
Copy link
Contributor

cfergeau commented Oct 3, 2024

Thanks @anjannath ! For some reason, make lint fails on my system with the following panics:

This can happen when golangci-lint is built with a go version older than the one installed on your system. You can try removing ./tools/bin/golangci-lint and running make lint again.

@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 3, 2024

@vyasgun: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/integration-crc 0f9cca3 link true /test integration-crc
ci/prow/e2e-crc 0f9cca3 link true /test e2e-crc

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@openshift-ci openshift-ci bot added the lgtm label Oct 4, 2024
@openshift-ci OpenShift CI
Copy link

openshift-ci bot commented Oct 4, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: anjannath

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:

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

@openshift-ci openshift-ci bot added the approved label Oct 4, 2024
@anjannath anjannath merged commit 053da06 into crc-org:main Oct 4, 2024
26 of 32 checks passed
@anjannath anjannath mentioned this pull request Oct 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anjannath anjannath anjannath approved these changes

@adrianriobo adrianriobo Awaiting requested review from adrianriobo

@gbraad gbraad Awaiting requested review from gbraad

Assignees

@anjannath anjannath

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vyasgun @anjannath @cfergeau