support excluding nodes when creating workloads #365
Merged
Conversation
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
CMGS
reviewed
Mar 19, 2021
cluster/calcium/create.go
Outdated
| return nodenames, nil | ||
| } | ||
|
|
||
| allNodes, err := c.ListPodNodes(context.Background(), podname, map[string]string{}, false) |
There was a problem hiding this comment.
ctx ！ 不要用 background，另外这个函数为啥在这里，不应该在 helper 么
There was a problem hiding this comment.
为了用这个 ListPodNodes 啦
CMGS
reviewed
Mar 19, 2021
cluster/calcium/create.go
Outdated
Comment on lines
60
to
66
| for _, n := range allNodes { | ||
| if _, ok := excludes[n.Name]; ok { | ||
| continue | ||
| } | ||
| rv = append(rv, n.Name) | ||
| } | ||
| return rv, nil |
There was a problem hiding this comment.
我其实甚至觉得 withNodeLocks 都不应该有 podname ... 为了保持一致就改这里了
There was a problem hiding this comment.
那你不如抽出整个 node 筛选的函数，包括 pod，all，黑名单白名单，withNodeLocks 就当纯粹的锁……现在就很分裂
|
updated |
rpc/gen/core.proto
Outdated
| @@ -452,6 +452,7 @@ message DeployOptions { | |||
| repeated string after_create = 22; | |||
| bytes raw_args = 23; | |||
| ResourceOptions resource_opts = 24; | |||
| repeated string excluded_nodenames = 25; | |||
There was a problem hiding this comment.
似乎可以通用化一点, 引入一个类型 NodeFilter, 以后好扩展
|
我就想偷个懒你们... 好吧 |
|
因为我们发现你想偷懒了……
…On Fri, Mar 19, 2021 at 4:44 PM tonic ***@***.***> wrote:
我就想偷个懒你们... 好吧
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#365 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD3S2DMZ3UVTW2I6DKHHSTTEMFGDANCNFSM4ZN6VZMQ>
.
--
- CMGS
|
tonicmuroq
commented
Mar 21, 2021
|
这得问你自己啊，这不是你定的规矩么…
tonic ***@***.***>于2021年3月21日 周日12:21写道：
***@***.**** commented on this pull request.
------------------------------
In rpc/transform.go
<#365 (comment)>:
> @@ -280,6 +280,17 @@ func toCoreDeployOptions(d *pb.DeployOptions) (*types.DeployOptions, error) {
return nil, err
}
+ // FIXME: adapt ... clean this later
+ nf := types.NodeFilter{
+ Podname: d.Podname,
+ Includes: d.Nodenames,
+ Labels: d.Nodelabels,
+ }
+ if d.NodeFilter != nil {
+ nf.Includes = d.NodeFilter.Includes
+ nf.Excludes = d.NodeFilter.Excludes
+ }
+
我总觉得应该可以直接用 core.pb.go 的东西的, 为啥还要包一层来着 = =?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#365 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD3S2HW4OILFPOOXKPIHZLTEVX6JANCNFSM4ZN6VZMQ>
.
--
- CMGS
|
|
updated, 有空看看吧 @CMGS @jschwinger23 |
CMGS
reviewed
Mar 21, 2021
types/options.go
Outdated
| @@ -22,7 +23,7 @@ type DeployOptions struct { | |||
| Debug bool // debug mode, use syslog as log driver | |||
| OpenStdin bool // OpenStdin for workload | |||
| Labels map[string]string // Labels for workloads | |||
| NodeLabels map[string]string // NodeLabels for filter node | |||
| NodeLabels map[string]string // NodeLabels for filter node, DEPRECATED, use NodeFilter instead | |||
There was a problem hiding this comment.
don't care about the compatibility, can remove it!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.