Change linux default network-mode to user #4514
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
lilyLuLiu
force-pushed
the
linux-networkmode
branch
from
d4d23de to
42bccc9
Compare
|
@lilyLuLiu : Hello, Could you please provide some context on why this change is needed? |
|
/retest |
|
@lilyLuLiu About integration failure I think https://github.com/crc-org/crc/blob/main/pkg/crc/network/types.go#L65 is culprit because settings only apply if we build binary for non installer, we need to change the default here also |
lilyLuLiu
force-pushed
the
linux-networkmode
branch
6 times, most recently
from
8c8f346 to
1c56660
Compare
lilyLuLiu
force-pushed
the
linux-networkmode
branch
2 times, most recently
from
92bdef8 to
cfb029d
Compare
test/e2e/features/proxy.feature
Outdated
| @@ -4,7 +4,7 @@ Feature: Behind proxy test | |||
| Check CRC use behind proxy | |||
|
|
|||
| # inherits @proxy tag from Feature | |||
| @cleanup | |||
| @cleanup @system_network | |||
There was a problem hiding this comment.
@lilyLuLiu Does this mean to run the proxy tet on linux the network mode should be system?
There was a problem hiding this comment.
@praveenkumar You are right, this test case also applied to user mode. I have removed the tag.
| // start container with squid proxy | ||
| err := util.ExecuteCommand("podman run --name squid -d -p 3128:3128 quay.io/crcont/squid") | ||
| err := util.ExecuteCommand("sudo dnf install podman -y;podman run --name squid -d -p 3128:3128 quay.io/crcont/squid") |
There was a problem hiding this comment.
why we need to install podman now but it was not the case before?
There was a problem hiding this comment.
Previous Linux tests are on our test machines, the podman is already installed. The new added pr checks on linux-arm, machines are reserved from TF, no pre-install podman.
There was a problem hiding this comment.
install podman shouldn't be part of test but it should be machine provision.
lilyLuLiu
force-pushed
the
linux-networkmode
branch
5 times, most recently
from
b383170 to
99a0ff7
Compare
test/e2e/testsuite/testsuite.go
Outdated
| if err != nil { | ||
| fmt.Println(err) | ||
| os.Exit(1) | ||
| } | ||
|
|
||
| err = util.ExecuteCommand("crc config set http-proxy http://192.168.130.1:3128") | ||
| err = util.ExecuteCommand("crc config set http-proxy http://127.0.0.1:8888") |
There was a problem hiding this comment.
this will not work, I think this test should be same as windows/mac.
There was a problem hiding this comment.
Change to http://host.crc.testing:3128 has we discussed.
lilyLuLiu
force-pushed
the
linux-networkmode
branch
from
99a0ff7 to
c89cf80
Compare
|
@lilyLuLiu: The following test failed, say
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. |
Currently, the Linux default network mode is system. But many issues happen due to different system situations.
For most user, user network mode already satisfied their demands. And user network mode no need to deal with complex system situations.
So after discuss, it's better to set user as default network-mode, making sure CRC works well in default.
If the user requires more functions, they can switch to system network mode.
@gbraad @praveenkumar @anjannath have agreed on this being needed.
It needs to start testing early during December.
In the worst case, we can revert before the new release in January.