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

SKS-2193: Support setting the owner of ElfCluster and virtual machines #162

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

haijianyang
Copy link
Contributor

Issue SKS-2194

支持指定集群和节点虚拟机的所属用户。

Change

ElfCluster 支持设置 CreatedByAnnotation 注解,在创建虚拟机的时候使用 CreatedByAnnotation 设置 Owner。

CreatedByAnnotation 支持以下两种方式的值:

  1. ${Tower username}@${Tower auth_config_id} 例如 caas.smartx@7e98ecbb-779e-43f6-8330-1bc1d29fffc7.
  2. ${Tower username} 例如 root

Test

  1. 没有设置 CreatedByAnnotation
    ksc 和 elfCluster 没有设置 CreatedByAnnotation,观察到创建出来的虚拟机也没有设置 Owner
    image (11)

  2. 设置 CreatedByAnnotation,且值为 ${Tower username}@${Tower auth_config_id} 格式
    ksc 和 elfCluster 没有设置 CreatedByAnnotation 为 haijian.yang@d8dc20fc-e197-41da-83b6-c903c88663fd,观察到创建出来的虚拟机带上了指定的 Owner

image (9)

  1. 设置 CreatedByAnnotation,且值为 ${Tower username} 格式
    image (12)

@haijianyang haijianyang requested a review from jessehu December 6, 2023 08:15
Copy link

codecov bot commented Dec 6, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (2fe52eb) 59.31% compared to head (69b23b8) 59.36%.
Report is 2 commits behind head on master.

Files Patch % Lines
pkg/service/vm.go 0.00% 9 Missing ⚠️
controllers/elfmachine_controller.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #162      +/-   ##
==========================================
+ Coverage   59.31%   59.36%   +0.04%     
==========================================
  Files          19       20       +1     
  Lines        3520     3546      +26     
==========================================
+ Hits         2088     2105      +17     
- Misses       1283     1292       +9     
  Partials      149      149              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -241,11 +243,21 @@ func (svr *TowerVMService) Clone(
hostID = *host.ID
}

var owner *models.VMOwnerParams
if createdBy := annotationsutil.GetCreatedBy(elfCluster); createdBy != "" {
creator := strings.Replace(createdBy, "@", "_", 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里考虑下将来 username中可能会带'@',比如使用email作为username

@jessehu jessehu changed the title SKS-2193: Support setting cluster and virtual machines owner SKS-2193: Support setting the owner of ElfCluster and virtual machines Dec 6, 2023
return createdBy
}

// last `@` replaced with `_`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

这个替换补充下原因

Copy link
Contributor Author

@haijianyang haijianyang Dec 7, 2023

Choose a reason for hiding this comment

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

func 已经有说明使用 ${Tower username}_${Tower auth_config_id} 这种格式。Annotation 也有说明使用 @ 间隔。

Copy link
Collaborator

Choose a reason for hiding this comment

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

// last @ replaced with _. 只说了 做什么,没说为什么要做

Copy link
Collaborator

Choose a reason for hiding this comment

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

或者你在Func comment中补充下 createdBy string是什么格式

// The owner can be in one of the following two formats:
// 1. ${Tower username}_${Tower auth_config_id}, e.g. caas.smartx_7e98ecbb-779e-43f6-8330-1bc1d29fffc7.
// 2. ${Tower username}, e.g. root. If auth_config_id is not set, it means it is a LOCAL user.
func parseOwnerFromCreatedBy(createdBy string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

改个名称:parseOwnerFromCreatedByAnnotation()

@haijianyang haijianyang merged commit b2fb22f into smartxworks:master Dec 7, 2023
3 checks passed
@haijianyang haijianyang deleted the vm-owner branch December 12, 2023 08:29
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.

2 participants