cloudpods icon indicating copy to clipboard operation
cloudpods copied to clipboard

配置结构体深拷贝bug修复以及修复baremetal agent注册bug

Open nyl1001 opened this issue 1 year ago • 18 comments

What this PR does / why we need it:

Does this PR need to be backport to the previous release branch?:

nyl1001 avatar Jan 17 '24 16:01 nyl1001

比如修改keystone服务configmap中如下默认配置参数 max_group_roles_in_project: 20 max_user_roles_in_project: 20 修改为: max_group_roles_in_project: 200 max_user_roles_in_project: 200 重修发布keystone服务后,在keystone服务中的配置MaxUserRolesInProject依然为默认值20,而不是修改后的200。 新加配置也不生效,只能得到默认值。

nyl1001 avatar Jan 17 '24 16:01 nyl1001

@nyl1001 先看下这个文档 https://www.cloudpods.org/docs/guides/auth_security/serviceconfig/#%E6%9C%8D%E5%8A%A1%E9%85%8D%E7%BD%AE

ioito avatar Jan 18 '24 02:01 ioito

https://www.cloudpods.org/docs/guides/auth_security/serviceconfig/#%E6%9C%8D%E5%8A%A1%E9%85%8D%E7%BD%AE 这个地方的代码昨天都review过了,并不是被数据库中覆盖了,数据库没有的配置也被重置成默认值了,你们原先的那个拷贝函数使用不当

nyl1001 avatar Jan 18 '24 03:01 nyl1001

https://www.cloudpods.org/docs/guides/auth_security/serviceconfig/#%E6%9C%8D%E5%8A%A1%E9%85%8D%E7%BD%AE 这个地方的代码昨天都review过了,并不是被数据库中覆盖了,数据库没有的配置也被重置成默认值了,你们原先的那个拷贝函数使用不当

climc service-config-edit 修改的配置优先级最高,也是推荐使用的改配置的方式

ioito avatar Jan 18 '24 03:01 ioito

https://www.cloudpods.org/docs/guides/auth_security/serviceconfig/#%E6%9C%8D%E5%8A%A1%E9%85%8D%E7%BD%AE 这个地方的代码昨天都review过了,并不是被数据库中覆盖了,数据库没有的配置也被重置成默认值了,你们原先的那个拷贝函数使用不当

climc service-config-edit 修改的配置优先级最高,也是推荐使用的改配置的方式

还没涉及到修改,你这边初始化就有问题了

nyl1001 avatar Jan 18 '24 05:01 nyl1001

climc service-config keystone max_group_roles_in_project=200 会先改数据库中 keystone.whitelisted_config表里面的配置,然后会通知相应的服务动态更改配置,下次服务重启会优先选用service-config里面的配置,你可以改下试试,然后看下数据库中的配置,再加下日志打印下配置

ioito avatar Jan 18 '24 07:01 ioito

climc service-config keystone max_group_roles_in_project=200 会先改数据库中 keystone.whitelisted_config表里面的配置,然后会通知相应的服务动态更改配置,下次服务重启会优先选用service-config里面的配置,你可以改下试试,然后看下数据库中的配置,再加下日志打印下配置 哎,你这些不用跟我强调,我都给你说过,这里的代码我都看过,我感觉你都不明白我的意思。 你初始化配置的时候使用的copyOptions有bug,我指出这个问题,你却谈后面的whitelist补救,这两者之间有关系吗?存在冲突吗?当然whitelist若存在,会覆盖存在的配置。但不存在的呢? 我始终强调初始化配置时的copyOptions函数有bug,这个不能完成深拷贝,我纠正这个问题。

nyl1001 avatar Jan 19 '24 02:01 nyl1001

climc service-config keystone max_group_roles_in_project=200 会先改数据库中 keystone.whitelisted_config表里面的配置,然后会通知相应的服务动态更改配置,下次服务重启会优先选用service-config里面的配置,你可以改下试试,然后看下数据库中的配置,再加下日志打印下配置

哎,你这些不用跟我强调,我都给你说过,这里的代码我都看过,我感觉你都不明白我的意思。 你初始化配置的时候使用的copyOptions有bug,我指出这个问题,你却谈后面的whitelist补救,这两者之间有关系吗?存在冲突吗?当然whitelist若存在,会覆盖存在的配置。但不存在的呢? 我始终强调初始化配置时的copyOptions函数有bug,这个不能完成深拷贝,我纠正这个问题。

nyl1001 avatar Jan 19 '24 02:01 nyl1001

https://github.com/yunionio/cloudpods/blob/b41c0dd1ca46d516bf4c90b2782dd978f5d2080c/pkg/apis/identity/consts.go#L123 configmap里面只有改 ServiceBlacklistOptionMap 里面配置的字段才会生效,本身就是这么设计的

ioito avatar Jan 19 '24 10:01 ioito

加了一个单元测试,实现应该是没问题的:https://github.com/yunionio/cloudpods/pull/19312/files

swordqiu avatar Jan 21 '24 01:01 swordqiu

反复强调,深拷贝有bug,本身这个修改也是针对这个深拷贝的,我一开始就把用例解释得很清楚了,你们要这么拧巴,随你们吧,当我没提

nyl1001 avatar Jan 22 '24 06:01 nyl1001

加了一个单元测试,实现应该是没问题的:https://github.com/yunionio/cloudpods/pull/19312/files

你这用例不够,无法验证出问题,你用实际的keystone的configmap配置作为用例试试吧,深拷贝没有你这么使用的

nyl1001 avatar Jan 22 '24 06:01 nyl1001

加了一个单元测试,实现应该是没问题的:https://github.com/yunionio/cloudpods/pull/19312/files

你这用例不够,无法验证出问题,你用实际的keystone的configmap配置作为用例试试吧,深拷贝没有你这么使用的

我试过你的改动,没有起到作用,改完configmap后,重启服务配置依然是20,本身设计就是需要用climc去改配置,不然删除configmap后会重置配置,没办法持久化到数据库中

ioito avatar Jan 22 '24 09:01 ioito

Collaborator

还在扯其他的,你的这个拷贝函数是不是有bug?首先初始化要保证正确,修改配置用climc没有问题,也不冲突?

nyl1001 avatar Jan 22 '24 09:01 nyl1001

加了一个单元测试,实现应该是没问题的:https://github.com/yunionio/cloudpods/pull/19312/files

你这用例不够,无法验证出问题,你用实际的keystone的configmap配置作为用例试试吧,深拷贝没有你这么使用的

我试过你的改动,没有起到作用,改完configmap后,重启服务配置依然是20,本身设计就是需要用climc去改配置,不然删除configmap后会重置配置,没办法持久化到数据库中

没起作用的原因就是copyOptions不是深拷贝,明白吗?修改配置用climc没有问题,没有反对你这个

nyl1001 avatar Jan 22 '24 09:01 nyl1001

加了一个单元测试,实现应该是没问题的:https://github.com/yunionio/cloudpods/pull/19312/files

你这用例不够,无法验证出问题,你用实际的keystone的configmap配置作为用例试试吧,深拷贝没有你这么使用的

我试过你的改动,没有起到作用,改完configmap后,重启服务配置依然是20,本身设计就是需要用climc去改配置,不然删除configmap后会重置配置,没办法持久化到数据库中

没起作用的原因就是copyOptions不是深拷贝,明白吗?修改配置用climc没有问题,没有反对你这个。你也别谈你的设计就是故意让这个copyOptions不能完成深拷贝,故意让他不起作用的哈,那我真就呵呵了

nyl1001 avatar Jan 22 '24 09:01 nyl1001

你用我这个改动后的函数,就可以正确完成copyOptions,这个我是在我们这边的生产环境验证了的,不然也不会在这里跟你们费劲的解释了

nyl1001 avatar Jan 22 '24 09:01 nyl1001

你用我这个改动后的函数,就可以正确完成copyOptions,这个我是在我们这边的生产环境验证了的,不然也不会在这里跟你们费劲的解释了

目前参数的改法是有歧义,这些参数将会在configmap中移除 https://github.com/yunionio/cloudpods-operator/pull/1042 ,只能通过climc service-config-edit 去更改,另外一个commit可以单独提pr合并

ioito avatar Jan 24 '24 09:01 ioito