add hostip and protocol to the hostport predicates #52421
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,9 +17,12 @@ limitations under the License. | |
| package predicates | ||
|
|
||
| import ( | ||
| "strings" | ||
|
|
||
| "k8s.io/api/core/v1" | ||
| metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" | ||
| "k8s.io/apimachinery/pkg/labels" | ||
| schedutil "k8s.io/kubernetes/plugin/pkg/scheduler/util" | ||
| ) | ||
|
|
||
| // FindLabelsInSet gets as many key/value pairs as possible out of a label set. | ||
|
|
@@ -89,3 +92,69 @@ func GetEquivalencePod(pod *v1.Pod) interface{} { | |
| type EquivalencePod struct { | ||
| ControllerRef metav1.OwnerReference | ||
| } | ||
|
|
||
| type hostPortInfo struct { | ||
| protocol string | ||
| hostIP string | ||
| hostPort string | ||
| } | ||
|
There was a problem hiding this comment. The There was a problem hiding this comment. You are far too kind, Now I will move to e2e test |
||
|
|
||
| // decode decodes string ("protocol/hostIP/hostPort") to *hostPortInfo object. | ||
| func decode(info string) *hostPortInfo { | ||
|
There was a problem hiding this comment. Just nitpick. If it was me, I would define it as something like func (h *hostPortInfo) Load(info string) error
h := hostPostInfo{}
err := h.Load(str)
if err != nil {
return err
}There was a problem hiding this comment. I think it is just fine now There was a problem hiding this comment. or at least you call it There was a problem hiding this comment. haha I think we can get what we decode from context, not very important to add it to the function name |
||
| hostPortInfoSlice := strings.Split(info, "/") | ||
|
|
||
| protocol := hostPortInfoSlice[0] | ||
| hostIP := hostPortInfoSlice[1] | ||
| hostPort := hostPortInfoSlice[2] | ||
|
|
||
| return &hostPortInfo{ | ||
| protocol: protocol, | ||
| hostIP: hostIP, | ||
| hostPort: hostPort, | ||
| } | ||
| } | ||
|
|
||
| // specialPortConflictCheck detects whether specailHostPort(whose hostIP is 0.0.0.0) is conflict with otherHostPorts. | ||
| // return true if we have a conflict. | ||
| func specialPortConflictCheck(specialHostPort string, otherHostPorts map[string]bool) bool { | ||
| specialHostPortInfo := decode(specialHostPort) | ||
|
|
||
| if specialHostPortInfo.hostIP == schedutil.DefaultBindAllHostIP { | ||
| // loop through all the otherHostPorts to see if there exists a conflict | ||
| for hostPortItem := range otherHostPorts { | ||
| hostPortInfo := decode(hostPortItem) | ||
|
|
||
| // if there exists one hostPortItem which has the same hostPort and protocol with the specialHostPort, that will cause a conflict | ||
| if specialHostPortInfo.hostPort == hostPortInfo.hostPort && specialHostPortInfo.protocol == hostPortInfo.protocol { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| // portsConflict check whether existingPorts and wantPorts conflict with each other | ||
| // return true if we have a conflict | ||
| func portsConflict(existingPorts, wantPorts map[string]bool) bool { | ||
|
|
||
| for existingPort := range existingPorts { | ||
| if specialPortConflictCheck(existingPort, wantPorts) { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| for wantPort := range wantPorts { | ||
| if specialPortConflictCheck(wantPort, existingPorts) { | ||
| return true | ||
| } | ||
|
|
||
| // general check hostPort conflict procedure for hostIP is not 0.0.0.0 | ||
| if existingPorts[wantPort] { | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 can not remember why we can not just delete the port from map, can you help to check it?
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 quite sure about it either, you wrote it in #42524
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.
@k82cn I can open another pr to change the logic, I think delete the port from map is better
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.
that's ok to me.
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.
ok
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 think this problem is fixed by #54538, so no longer a problem