Rename e2e job names to ci-commitsha-uuid for consistency #246
Conversation
shivakunv
requested review from
ArangoGutierrez,
elezar,
tariq1890 and
cdesiniotis
as code owners
shivakunv
changed the title
tests/common/common.go
Outdated
| // uid is unique for each run | ||
| uid := GenerateUID() | ||
|
|
||
| cfg.Name = fmt.Sprintf("ci%s-%s-%s", attempt, sha, uid) |
There was a problem hiding this comment.
I am not sure if this change is desirable. Won't the names now become a lot longer?
There was a problem hiding this comment.
old code:- opts.cfg.Name = opts.cfg.Name + "-" + common.GenerateUID()
changed to :-
common.SetCfgName(&opts.cfg)
No, it will not be longer, it will align with our e2e tests for the lib/app (e.g., gpu-operator, gpu-driver-container, etc.), which follow the naming format ci-commitid-uuid.
Currently, holodeck e2e tests use names derived from the yaml file's given name plus a UUID (e.g., holodeck-uuid).
As a result, the cleanup script, which only works for instances named in the format ci-commitid-uuid, will not clean up these instances.
This change will ensure that any instances not properly cleaned up will now be handled correctly.
There was a problem hiding this comment.
As a result, the cleanup script, which only works for instances named in the format ci-commitid-uuid, will not clean up these instances.
If the cleanup script looks for VPCs/instances with the holodeck and cicd tags, wouldn't that suffice? Do you really need a particular name format?
There was a problem hiding this comment.
with
--filters "Name=tag:Project,Values=holodeck" "Name=tag:Environment,Values=cicd" in
#244
we dont need this PR . I'll close this PR after verification (deletion of resources )
There was a problem hiding this comment.
have you checked with the AWS API if it allows such long string names?
There was a problem hiding this comment.
it allows
it will not be longer, it will align with our e2e tests for the lib/app (e.g., gpu-operator, gpu-driver-container, etc.), which follow the naming format ci-commitid-uuid.
There was a problem hiding this comment.
pushed suggested code
tests/common/common.go
Outdated
| @@ -28,3 +34,16 @@ func GenerateUID() string { | |||
|
|
|||
| return string(b) | |||
| } | |||
|
|
|||
| func SetCfgName(cfg *v1alpha1.Environment) { | |||
There was a problem hiding this comment.
Two things
-
commonisn't the right package for this method. This method can be moved back to the same package where it is invoked and be made private -
The method should just return a name instead of taking the Environment object pointer as an argument .
cfg.Name := getName()
There was a problem hiding this comment.
This PR is unnecessary as PR :
#244
will clean up AWS resources in the CICD environment with the specified holodeck project name.
There was a problem hiding this comment.
I think, unregardless of #244 I think having a nice standardized way of naming our instances is worth the effort. Agree with @tariq1890
There was a problem hiding this comment.
pushed suggested code
|
@shivakunv please rebase |
|
@tariq1890 please take another look |
tests/common/common.go
Outdated
| @@ -28,3 +34,16 @@ func GenerateUID() string { | |||
|
|
|||
| return string(b) | |||
| } | |||
|
|
|||
| func SetCfgName(cfg *v1alpha1.Environment) { | |||
There was a problem hiding this comment.
I think, unregardless of #244 I think having a nice standardized way of naming our instances is worth the effort. Agree with @tariq1890
tests/common/common.go
Outdated
| // uid is unique for each run | ||
| uid := GenerateUID() | ||
|
|
||
| cfg.Name = fmt.Sprintf("ci%s-%s-%s", attempt, sha, uid) |
There was a problem hiding this comment.
have you checked with the AWS API if it allows such long string names?
Signed-off-by: shiva kumar <[email protected]>
No description provided.