[PATCH v3 compass-ci] fix: sched: add 2 fields into cluster state
[Why] 1) Each node will report it's infos (roles, ip, direct_ips, direct_macs) when running cluster job, so also add these 2 parameters into node state: "direct_ips", "direct_macs". Cluster test scripts needs "direct_server_ips" as server client's ip to connect to server to do a test. When request state is "roles_ip", return clients two variables: server, direct_server_ips. Clients will export them. 2) Alter funtion "get_ip"'s name and it's return value to fit for changes. Signed-off-by: Ren Wen <15991987063@163.com> --- src/lib/sched.cr | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/lib/sched.cr b/src/lib/sched.cr index c32091f..b981876 100644 --- a/src/lib/sched.cr +++ b/src/lib/sched.cr @@ -99,24 +99,31 @@ class Sched when "write_state" node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] + update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end # show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end - # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end end -- 2.23.0
On Wed, Oct 28, 2020 at 07:32:57PM +0800, Ren Wen wrote:
[Why] 1) Each node will report it's infos (roles, ip, direct_ips, direct_macs) when running cluster job, so also add these 2 parameters into node state: "direct_ips", "direct_macs".
Cluster test scripts needs "direct_server_ips" as server client's ip to connect to server to do a test. When request state is "roles_ip", return clients two variables: server, direct_server_ips. Clients will export them.
2) Alter funtion "get_ip"'s name and it's return value to fit for changes.
Signed-off-by: Ren Wen <15991987063@163.com> --- src/lib/sched.cr | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/lib/sched.cr b/src/lib/sched.cr index c32091f..b981876 100644 --- a/src/lib/sched.cr +++ b/src/lib/sched.cr @@ -99,24 +99,31 @@ class Sched when "write_state" node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] + update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role
Why use % in here? Thanks, Xueliang
+ return role_state end end end -- 2.23.0
On Wed, Oct 28, 2020 at 07:32:57PM +0800, Ren Wen wrote:
[Why] 1) Each node will report it's infos (roles, ip, direct_ips, direct_macs) when running cluster job, so also add these 2 parameters into node state: "direct_ips", "direct_macs".
Cluster test scripts needs "direct_server_ips" as server client's ip to connect to server to do a test. When request state is "roles_ip", return clients two variables: server, direct_server_ips. Clients will export them.
2) Alter funtion "get_ip"'s name and it's return value to fit for changes.
Signed-off-by: Ren Wen <15991987063@163.com> --- src/lib/sched.cr | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/lib/sched.cr b/src/lib/sched.cr index c32091f..b981876 100644 --- a/src/lib/sched.cr +++ b/src/lib/sched.cr @@ -99,24 +99,31 @@ class Sched when "write_state" node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] + update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end
We can use write into 1 line: return role_state if %(#{role_state["roles"]}) == role Thanks, Xijian
end end -- 2.23.0
On Wed, Oct 28, 2020 at 07:32:57PM +0800, Ren Wen wrote:
[Why] 1) Each node will report it's infos (roles, ip, direct_ips, direct_macs) when running cluster job, so also add these 2 parameters into node state: "direct_ips", "direct_macs".
Cluster test scripts needs "direct_server_ips" as server client's ip to connect to server to do a test. When request state is "roles_ip", return clients two variables: server, direct_server_ips. Clients will export them.
2) Alter funtion "get_ip"'s name and it's return value to fit for changes.
Signed-off-by: Ren Wen <15991987063@163.com> --- src/lib/sched.cr | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/lib/sched.cr b/src/lib/sched.cr index c32091f..b981876 100644 --- a/src/lib/sched.cr +++ b/src/lib/sched.cr @@ -99,24 +99,31 @@ class Sched when "write_state" node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] + update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state
return role_state if %(#{role_state["roles"]}) == role why do not directly use role_state["roles"] == role Thanks Luan Shengde
end end end -- 2.23.0
On Wed, Oct 28, 2020 at 07:32:57PM +0800, Ren Wen wrote:
[Why] 1) Each node will report it's infos (roles, ip, direct_ips, direct_macs) when running cluster job, so also add these 2 parameters into node state: "direct_ips", "direct_macs".
Cluster test scripts needs "direct_server_ips" as server client's ip to connect to server to do a test. When request state is "roles_ip", return clients two variables: server, direct_server_ips. Clients will export them.
2) Alter funtion "get_ip"'s name and it's return value to fit for changes.
Signed-off-by: Ren Wen <15991987063@163.com> --- src/lib/sched.cr | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/lib/sched.cr b/src/lib/sched.cr index c32091f..b981876 100644 --- a/src/lib/sched.cr +++ b/src/lib/sched.cr @@ -99,24 +99,31 @@ class Sched when "write_state" node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] +
can keep align with "=", like below: node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] direct_ips = env.params.query["direct_ips"] direct_macs = env.params.query["direct_macs"] Thanks, Kaiyi
update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end end -- 2.23.0
node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] +
can keep align with "=", like below:
node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] direct_ips = env.params.query["direct_ips"] direct_macs = env.params.query["direct_macs"]
It's a not recommended style to align variables. Can use the style in containers like Hash. Thanks, RenWen
Thanks, Kaiyi
update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end end -- 2.23.0
On Thu, Oct 29, 2020 at 07:48:11PM +0800, Ren Wen wrote:
node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] +
can keep align with "=", like below:
node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] direct_ips = env.params.query["direct_ips"] direct_macs = env.params.query["direct_macs"]
It's a not recommended style to align variables. Can use the style in containers like Hash.
Did you run crystal-format? Thanks, Xueliang
Thanks, RenWen
Thanks, Kaiyi
update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end end -- 2.23.0
On Thu, Oct 29, 2020 at 07:50:29PM +0800, Cao Xueliang wrote:
On Thu, Oct 29, 2020 at 07:48:11PM +0800, Ren Wen wrote:
node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] +
can keep align with "=", like below:
node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] direct_ips = env.params.query["direct_ips"] direct_macs = env.params.query["direct_macs"]
It's a not recommended style to align variables. Can use the style in containers like Hash.
Did you run crystal-format?
I've tested it. Thanks, RenWen
Thanks, Xueliang
Thanks, RenWen
Thanks, Kaiyi
update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end end -- 2.23.0
On Wed, Oct 28, 2020 at 07:32:57PM +0800, Ren Wen wrote:
[Why] 1) Each node will report it's infos (roles, ip, direct_ips, direct_macs) when running cluster job, so also add these 2 parameters into node state: "direct_ips", "direct_macs".
Cluster test scripts needs "direct_server_ips" as server client's ip to connect to server to do a test. When request state is "roles_ip", return clients two variables: server, direct_server_ips. Clients will export them.
2) Alter funtion "get_ip"'s name and it's return value to fit for changes.
Signed-off-by: Ren Wen <15991987063@163.com> --- src/lib/sched.cr | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/src/lib/sched.cr b/src/lib/sched.cr index c32091f..b981876 100644 --- a/src/lib/sched.cr +++ b/src/lib/sched.cr @@ -99,24 +99,31 @@ class Sched when "write_state" node_roles = env.params.query["node_roles"] node_ip = env.params.query["ip"] + direct_ips = env.params.query["direct_ips"] + direct_macs = env.params.query["direct_macs"] + update_cluster_state(cluster_id, job_id, "roles", node_roles) update_cluster_state(cluster_id, job_id, "ip", node_ip) + update_cluster_state(cluster_id, job_id, "direct_ips", direct_ips) + update_cluster_state(cluster_id, job_id, "direct_macs", direct_macs) when "roles_ip" role = "server" - server_ip = get_ip(cluster_id, role) - return "server=#{server_ip}" + role_state = get_role_state(cluster_id, role) + raise "Missing #{role} state in cluster state" unless role_state + return "server=#{role_state["ip"]}\n" \ + "direct_server_ips=#{role_state["direct_ips"]}" end
# show cluster state return @redis.hash_get("sched/cluster_state", cluster_id) end
- # get the ip of role from cluster_state - def get_ip(cluster_id, role) + # get the node state of role from cluster_state + private def get_role_state(cluster_id, role) cluster_state = get_cluster_state(cluster_id) - cluster_state.each_value do |config| - if %(#{config["roles"]}) == role - return config["ip"] + cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end
if all condition is false, you should also have return value. otherwise, return value will be "cluster_state". Thanks, Liushaofei
end -- 2.23.0
+ cluster_state.each_value do |role_state| + if %(#{role_state["roles"]}) == role + return role_state end end
if all condition is false, you should also have return value. otherwise, return value will be "cluster_state".
Hash[key] will raise an exception if key is nonexistent. Here raising the exception or returning 'role_state'. Thanks, RenWen
Thanks, Liushaofei
end -- 2.23.0
participants (6)
-
Cao Xueliang -
Liu Shaofei -
Lu Kaiyi -
Luan Shengde -
Ren Wen -
Xu Xijian