-
Notifications
You must be signed in to change notification settings - Fork 116
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
Add OVSNetwork CRD and controller for it #646
Add OVSNetwork CRD and controller for it #646
Conversation
Thanks for your PR,
To skip the vendors CIs use one of:
|
96c6cf8
to
062ceda
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
062ceda
to
7356cf2
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
Pull Request Test Coverage Report for Build 8893889198Details
💛 - Coveralls |
7356cf2
to
ed47b49
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
ed47b49
to
1791638
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
1791638
to
f66feeb
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
f66feeb
to
de0caa8
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
move refactoring commit to a separate PR #663 |
de0caa8
to
b539265
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
b539265
to
fb6bd63
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
fb6bd63
to
13ceb25
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
I removed the note about auto bridge selection. PR to the ovs-cni is not merged and this comment is incorrect at the moment |
Signed-off-by: Yury Kulazhenkov <[email protected]>
Signed-off-by: Yury Kulazhenkov <[email protected]>
13ceb25
to
dd72472
Compare
Thanks for your PR,
To skip the vendors CIs use one of:
|
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.
Great work!
just small nits and we should be able to merge this one
k8s.v1.cni.cncf.io/resourceName: {{.CniResourceName}} | ||
spec: | ||
config: '{ | ||
"cniVersion":"0.3.1", |
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.
please update this one version to also 1.0.0
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.
since yury is on PTO i addressed this in #696
@@ -0,0 +1,7 @@ | |||
apiVersion: sriovnetwork.openshift.io/v1 |
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.
please make a valid example here
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.
since yury is on PTO i addressed this in #696
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.
please note that other samples have the same value. i have aligned this one in the followup pr
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, happy to address additional comments as a followup PR since yury is on PTO
@SchSeba can we merge this one ? |
closing this one |
This PR adds OVSNetwork CRD and controller for it.
Context is #640
Depends on: #663. Last two commits to review