-
Notifications
You must be signed in to change notification settings - Fork 33
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
metallbv2-add-connect-timer #666
base: main
Are you sure you want to change the base?
metallbv2-add-connect-timer #666
Conversation
734fd28
to
e5cff99
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.
several comments
pkg/metallb/bgppeer.go
Outdated
} | ||
|
||
glog.V(100).Infof( | ||
"Creating BGPPeer %s in namespace %s with this holdTime: %s", |
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.
change holdTime to ConnectTime
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.
changed
|
||
builder.errorMsg = "bgppeer 'connectTime' value is not valid" | ||
} | ||
|
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 add
if builder.errorMsg != "" {
return builder
}
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.
added
@@ -397,8 +421,9 @@ func (builder *BGPPeerBuilder) WithNodeSelector(nodeSelector map[string]string) | |||
return builder | |||
} | |||
|
|||
ndSelector := []mlbtypes.NodeSelector{{MatchLabels: nodeSelector}} | |||
builder.Definition.Spec.NodeSelectors = ndSelector | |||
builder.Definition.Spec.NodeSelectors = []metav1.LabelSelector{{ |
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.
What's the idea of this change?
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.
They changed it upstream in mlbtypesv1beta2
pkg/metallb/bgppeer_test.go
Outdated
@@ -291,11 +329,6 @@ func TestBGPPeerWithNodeSelector(t *testing.T) { | |||
testBGPPeer: buildValidBGPPeerBuilder(buildBGPPeerTestClientWithDummyObject()), | |||
nodeSelector: map[string]string{"test": "test1"}, | |||
}, | |||
{ | |||
testBGPPeer: buildInValidBGPPeerBuilder(buildBGPPeerTestClientWithDummyObject()), |
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.
What's the idea of this change?
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.
replaced
f94ecb2
to
08622f3
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.
1 comment
pkg/metallb/bgppeer.go
Outdated
// WithNodeSelector defines the nodeSelector placed in the BGPPeer spec. | ||
func (builder *BGPPeerBuilder) WithNodeSelector(nodeSelector map[string]string) *BGPPeerBuilder { | ||
if valid, _ := builder.validate(); !valid { | ||
return builder | ||
} | ||
|
||
if builder.errorMsg != "" { |
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.
?
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.
removed
08622f3
to
3a98dbf
Compare
/lgtm |
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
Adding metallb connect timer. Connect timer is part of mlbtypesv1beta2