-
Notifications
You must be signed in to change notification settings - Fork 63
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
Network topology API definition #144
Network topology API definition #144
Conversation
eb96d8d
to
038f864
Compare
pkg/client/applyconfiguration/scheduling/v1beta1/networktopologiesspec.go
Outdated
Show resolved
Hide resolved
pkg/client/applyconfiguration/scheduling/v1beta1/podgroupspec.go
Outdated
Show resolved
Hide resolved
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.
Only found a couple of code duplications and empty line changes (see comments in individual files for details). Not sure if those are necessary - please excuse my ignorance if those are actually desired. Other than that, everything looks good!
2010613
to
793b1b3
Compare
What is the milestone for the network topology feature? |
0295bc1
to
0e25160
Compare
Will have a POC release in v2.0. |
pkg/apis/bus/v1alpha1/actions.go
Outdated
// RestartPodAction if this action is set, only the pod will be restarted | ||
// It means that only the pod corresponding to the event will be deleted and recreated. | ||
// This action can just work together with pod level events, e.g. PodFailed | ||
RestartPodAction Action = "RestartPod" |
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.
This change seems to have no relation with network topology.
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.
These commits is synced from master branch, will submit a seperate pr later.
TaskName string `json:"taskName,omitempty"` | ||
Path string `json:"path,omitempty"` | ||
Port int `json:"port,omitempty"` | ||
// +optional |
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.
Why have so many lines unrelated to this project been added?
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.
These commits is synced from master branch, will submit a seperate pr later.
/assign |
0e25160
to
5dac612
Compare
Looks good to me now. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: william-wang 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 |
Signed-off-by: Monokaix <changxuzheng@huawei.com>
5dac612
to
6aa1e55
Compare
/lgtm |
e5d361b
into
volcano-sh:network-topology-dev
API change for volcano-sh/volcano#3850