-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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.
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
diagnostics: replace usage of brctl with /sbin/ip #19929
Conversation
@@ -23,7 +23,7 @@ func (l *LogInterface) LogNode(kubeClient kclientset.Interface) { | |||
l.LogSystem() | |||
l.LogServices() | |||
|
|||
l.Run("brctl show", "bridges") | |||
l.Run("ip link", "interfaces") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need bridge-utils
package in node image? If not, we can remove this package from images/dind/node/Dockerfile
and images/node/Dockerfile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hack/dind-cluster.sh uses brctl too, so that would need to be fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that we don't use linux bridges for anything at all any more, I think we can just drop this line
brctl (and its bridge-utils package) has been unmaintained for years and it's functionality replaced by the 'ip' tool from the iproute project. brctl may not be available on systems, and we don't need it anyway (since none of the openshift-sdn interfaces are bridge devices) so just remove it.
@danwinship PTAL thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dcbw, pravisankar 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
brctl (and its bridge-utils package) has been unmaintained
for years and it's functionality replaced by the 'ip' tool
from the iproute project. brctl may not be available on
systems so just use 'ip link' instead.
@knobunc @openshift/networking @pravisankar