-
Notifications
You must be signed in to change notification settings - Fork 114
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
[software-bridges 5/x] Enable software-bridge implementation #750
[software-bridges 5/x] Enable software-bridge implementation #750
Conversation
Thanks for your PR,
To skip the vendors CIs, Maintainers can use one of:
|
Pull Request Test Coverage Report for Build 10564493259Details
💛 - Coveralls |
4a09222
to
3c99cd8
Compare
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
@SchSeba PTAL on this one. |
@@ -68,8 +69,16 @@ func WithSkipVFConfiguration() Option { | |||
} | |||
} | |||
|
|||
// WithSkipBridgeConfiguration configures generic_plugin to skip configuration of the managed bridges | |||
func WithSkipBridgeConfiguration() Option { |
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.
I don't see this one used.
so at the end we only validate the vars.ManageSoftwareBridges
. on the if state here return vars.ManageSoftwareBridges && !p.skipBridgeConfiguration
also (I know it's not part of this PR) can you please explain me when on prePhase we don't configure the vfs?
sriov-network-operator/cmd/sriov-network-config-daemon/service.go
Lines 209 to 210 in f8cc336
case PhasePre: | |
configPlugin, err = newGenericPluginFunc(hostHelpers, generic.WithSkipVFConfiguration()) |
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.
also (I know it's not part of this PR) can you please explain me when on prePhase we don't configure the vfs?
At the pre
stage we creatr VFs, but unbind them from a driver to let OS Network manager (e.g. NetworkManager, systemd-network) to apply custom configuration for the PF function (e.g. VF-lag). Such configuration can't be when the PF has VFs that are bound to a driver.
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.
The WithSkipBridgeConfiguration function is used in the service command to disable bridge management during the pre
phase
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.
you are right! I miss that sorry
Signed-off-by: Yury Kulazhenkov <[email protected]>
Signed-off-by: Yury Kulazhenkov <[email protected]>
3c99cd8
to
0b9f45c
Compare
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
nice work!
@@ -68,8 +69,16 @@ func WithSkipVFConfiguration() Option { | |||
} | |||
} | |||
|
|||
// WithSkipBridgeConfiguration configures generic_plugin to skip configuration of the managed bridges | |||
func WithSkipBridgeConfiguration() Option { |
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.
you are right! I miss that sorry
Thx for the great work @ykulazhenkov !! 🎉 |
This PR enables all required code for software-bridges features.
I will submit documentation for the feature in the next PR. I created #749 to track documentation update.
How to test the feature:
manageSoftwareBridges
feature gatekubectl patch sriovoperatorconfigs.sriovnetwork.openshift.io -n network-operator default --patch '{ "spec": { "featureGates": { "manageSoftwareBridges": true } } }' --type='merge'