Skip to content
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

support setting tls configs in "SetNode" #526

Merged
merged 1 commit into from
Dec 29, 2021

Conversation

DuodenumL
Copy link
Contributor

在SetNode里支持设置ca / cert / key,以及一些其它的小优化。

if client == nil {
return nil
}
if err := validateEngine(ctx, client, config.ConnectionTimeout); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里和L86都是为了真正检查这个engine的可用性,可以一定程度上防止之前http请求连接到https的问题。

addIfNotEmpty(fmt.Sprintf(nodeCaKey, node.Name), node.Ca)
addIfNotEmpty(fmt.Sprintf(nodeCertKey, node.Name), node.Cert)
addIfNotEmpty(fmt.Sprintf(nodeKeyKey, node.Name), node.Key)
enginefactory.RemoveEngineFromCache(node.Endpoint, node.Ca, node.Cert, node.Key)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里提供一个可以清除engine缓存的入口,调用eru-cli node set可以清除对应node的engine缓存。

但是要保证ca / cert / key都跟之前的一致,才能清除掉缓存。例如对于开启了TLS的node,如果只是调用eru-cli node set xxx-node而不传任何参数的话,这里Ca / Cert / Key都是空字符串,自然清除不掉正确的缓存。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些很细节的地方需要好好文档记录 过一个月可能自己都忘了怎么清除缓存..

@@ -180,13 +190,19 @@ func (m *Mercury) UpdateNodeResource(ctx context.Context, node *types.Node, reso
}

func (m *Mercury) makeClient(ctx context.Context, node *types.Node) (client engine.API, err error) {
// try to get from cache without ca/cert/key
if client = enginefactory.GetEngineFromCache(ctx, m.config, node.Endpoint, "", "", ""); client != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

在不知道有没有配置TLS的情况下,先拿空的去试一试,这样可以减轻ETCD的压力。即使配置了TLS,缓存里查不到也不会有网络IO,性能应该还行。

return
nodes = append(nodes, node)
}
wg := &sync.WaitGroup{}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

因为后面GetEngine每次都要调用Info来validate一下,这里还是并发请求比较好。

Bind string `yaml:"bind" required:"true" default:"5001"` // HTTP API address
LockTimeout time.Duration `yaml:"lock_timeout" required:"true" default:"30s"` // timeout for lock (ttl)
GlobalTimeout time.Duration `yaml:"global_timeout" required:"true" default:"300s"` // timeout for remove, run_and_wait and build, in second
ConnectionTimeout time.Duration `yaml:"connection_timeout" default:"10s"` // timeout for connections
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加了一个配置项,“小timeout”

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

做咩的

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

做咩的
GlobalTimeout太大了,用于一些api call之类的地方不太合适,所以放了个“小timeout”。

keyFormats := []string{nodeCaKey, nodeCertKey, nodeKeyKey}
data := []string{"", "", ""}
for i := 0; i < 3; i++ {
ev, err := m.GetOne(ctx, fmt.Sprintf(keyFormats[i], node.Name))
if err != nil {
if !errors.Is(err, types.ErrBadCount) {
log.Warnf(ctx, "[makeClient] Get key failed %v", err)
return nil, err
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

拿不到就不应该继续了,否则用不正确的tls config去连接,可能会出奇怪问题。

This comment was marked as resolved.

cluster/calcium/node.go Outdated Show resolved Hide resolved
engine/factory/factory.go Outdated Show resolved Hide resolved
@@ -153,6 +159,10 @@ func (m *Mercury) UpdateNodes(ctx context.Context, nodes ...*types.Node) error {
d := string(bytes)
data[fmt.Sprintf(nodeInfoKey, node.Name)] = d
data[fmt.Sprintf(nodePodKey, node.Podname, node.Name)] = d
addIfNotEmpty(fmt.Sprintf(nodeCaKey, node.Name), node.Ca)
addIfNotEmpty(fmt.Sprintf(nodeCertKey, node.Name), node.Cert)
addIfNotEmpty(fmt.Sprintf(nodeKeyKey, node.Name), node.Key)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

所以怎么删掉 ca?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

没法通过api删掉,但是可以去etcd里手动把那三个key给删了

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

虽然我觉得不太好 但是也没想到好办法...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

硬要想办法的话,可以在proto里设置一个"message String",区分nil和""和含义...就是很麻烦和繁琐,看着也不好看

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要不把 delta 用起来? node set options 里有个 delta 参数, 默认是 false, 规定只有 true 的时候才改变 ca. 这正好符合使用场景, 因为改变 ca 的时候就是需要 cli node set --delta 才能保持资源不变.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

要不把 delta 用起来? node set options 里有个 delta 参数, 默认是 false, 规定只有 true 的时候才改变 ca. 这正好符合使用场景, 因为改变 ca 的时候就是需要 cli node set --delta 才能保持资源不变.

感觉还是有点奇怪,比如原本这个node是有tls相关的这些东西的,然后cli node set --delta --memory 1G xxx-node,那这个时候要不要把ca置为空?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

还是保持现状吧..

keyFormats := []string{nodeCaKey, nodeCertKey, nodeKeyKey}
data := []string{"", "", ""}
for i := 0; i < 3; i++ {
ev, err := m.GetOne(ctx, fmt.Sprintf(keyFormats[i], node.Name))
if err != nil {
if !errors.Is(err, types.ErrBadCount) {
log.Warnf(ctx, "[makeClient] Get key failed %v", err)
return nil, err

This comment was marked as resolved.

@DuodenumL DuodenumL force-pushed the feature/endpoint branch 2 times, most recently from 93922ad to abe3dba Compare December 28, 2021 10:27
@jschwinger233
Copy link
Member

然后这 PR 先不合, 你接下来去把 cli 也适配上, 然后用 cli 测试一下, 包括缓存更新这些隐藏的功能, 测完之后再合并这个.

@jschwinger233 jschwinger233 changed the title support setting tls configs in "SetNode" [WIP] support setting tls configs in "SetNode" Dec 28, 2021
@DuodenumL
Copy link
Contributor Author

然后这 PR 先不合, 你接下来去把 cli 也适配上, 然后用 cli 测试一下, 包括缓存更新这些隐藏的功能, 测完之后再合并这个.

cli我已经做完测过了,明天push一下

@jschwinger233
Copy link
Member

只是刷新缓存和删除ca这两个操作太 tricky 了, 不过暂时也没什么好办法, 先这样吧

@jschwinger233 jschwinger233 changed the title [WIP] support setting tls configs in "SetNode" [support setting tls configs in "SetNode" Dec 29, 2021
@jschwinger233 jschwinger233 changed the title [support setting tls configs in "SetNode" support setting tls configs in "SetNode" Dec 29, 2021
@CMGS CMGS merged commit d903237 into projecteru2:master Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants